Re: [PATCH v3] ftrace: Fix possible use-after-free issue in ftrace_location()
From: Steven Rostedt <rostedt@goodmis.org>
Date: 2024-05-02 21:06:58
Also in:
lkml
On Wed, 17 Apr 2024 11:28:30 +0800 Zheng Yejian [off-list ref] wrote:
quoted hunk ↗ jump to hunk
diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c index da1710499698..e05d3e3dc06a 100644 --- a/kernel/trace/ftrace.c +++ b/kernel/trace/ftrace.c@@ -1581,7 +1581,7 @@ static struct dyn_ftrace *lookup_rec(unsigned long start, unsigned long end) } /** - * ftrace_location_range - return the first address of a traced location + * ftrace_location_range_rcu - return the first address of a traced location
kerneldoc comments are for external functions. You need to move this down
to ftrace_location_range() as here you are commenting a local static function.
But I have to ask, why did you create this static function anyway? There's
only one user of it (the ftrace_location_range()). Why didn't you just
simply add the rcu locking there?
unsigned long ftrace_location_range(unsigned long start, unsigned long end)
{
struct dyn_ftrace *rec;
unsigned long ip = 0;
rcu_read_lock();
rec = lookup_rec(start, end);
if (rec)
ip = rec->ip;
rcu_read_unlock();
return ip;
}
-- Steve
quoted hunk ↗ jump to hunk
* if it touches the given ip range * @start: start of range to search. * @end: end of range to search (inclusive). @end points to the last byte@@ -1592,7 +1592,7 @@ static struct dyn_ftrace *lookup_rec(unsigned long start, unsigned long end) * that is either a NOP or call to the function tracer. It checks the ftrace * internal tables to determine if the address belongs or not. */ -unsigned long ftrace_location_range(unsigned long start, unsigned long end) +static unsigned long ftrace_location_range_rcu(unsigned long start, unsigned long end) { struct dyn_ftrace *rec;@@ -1603,6 +1603,16 @@ unsigned long ftrace_location_range(unsigned long start, unsigned long end) return 0; } +unsigned long ftrace_location_range(unsigned long start, unsigned long end) +{ + unsigned long loc; + + rcu_read_lock(); + loc = ftrace_location_range_rcu(start, end); + rcu_read_unlock(); + return loc; +}