Re: [PATCH 1/1] tracing: Fix memory leak when reading set_event file
From: Steven Rostedt
Date: Wed Feb 19 2025 - 11:05:59 EST
On Wed, 19 Feb 2025 18:42:30 +0800
Adrian Huang <adrianhuang0701@xxxxxxxxx> wrote:
> The root cause is that s_next() returns NULL when nothing is found.
> This results in s_stop() attempting to free a NULL pointer because its
> parameter p is NULL.
>
> Fix the issue by freeing the memory appropriately when s_next() fails
> to find anything.
>
> Fixes: b355247df104 ("tracing: Cache ":mod:" events for modules not loaded yet")
> Signed-off-by: Adrian Huang <ahuang12@xxxxxxxxxx>
> ---
> kernel/trace/trace_events.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 4cb275316e51..c76353ad0a4e 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -1591,6 +1591,7 @@ s_next(struct seq_file *m, void *v, loff_t *pos)
> return iter;
> #endif
>
> + kfree(iter);
> return NULL;
> }
>
This most definitely needs a comment, as it will look like a bug otherwise.
Please add:
/*
* The iter is allocated in s_start() and passed via the 'v'
* parameter. To stop the iterator, NULL must be returned. But
* the return value is what the 'v' parameter in s_stop() receives
* and frees. Free iter here as it will no longer be used.
*/
kfree(iter);
I would also change the variable 'p' in s_stop() to 'v' to be consistent.
Thanks,
-- Steve