Messages in this thread Patch in this message | | | Date | Thu, 2 May 2024 17:07:43 -0400 | From | Steven Rostedt <> | Subject | Re: [PATCH v3] ftrace: Fix possible use-after-free issue in ftrace_location() |
| |
On Wed, 17 Apr 2024 11:28:30 +0800 Zheng Yejian <zhengyejian1@huawei.com> wrote:
> 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
> * 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; > +}
| |