On Fri, 18 Jul 2025 22:31:58 -0400
Steven Rostedt <[email protected]> wrote:

> From: Steven Rostedt <[email protected]>
> 
> When a module is loaded, it adds trace events defined by the module. It
> may also need to modify the modules trace printk formats to replace enum
> names with their values.
> 
> If two modules are loaded at the same time, the adding of the event to the
> ftrace_events list can corrupt the walking of the list in the code that is
> modifying the printk format strings and crash the kernel.
> 
> The addition of the event should take the trace_event_sem for write while
> it adds the new event.
> 
> Also add a lockdep_assert_held() on that semaphore in
> __trace_add_event_dirs() as it iterates the list.

Looks good to me.

Acked-by: Masami Hiramatsu (Google) <[email protected]>

Thanks!

> 
> Cc: [email protected]
> Reported-by: Fusheng Huang(黄富生)  <[email protected]>
> Closes: https://lore.kernel.org/all/[email protected]/
> Fixes: 110bf2b764eb6 ("tracing: add protection around module events unload")
> Signed-off-by: Steven Rostedt (Google) <[email protected]>
> ---
> Changes since v1: 
> https://lore.kernel.org/[email protected]
> 
> - Replace lockdep_assert_held_read() with lockdep_assert_held() as it is
>   actually held with down_write() when it is called.
> 
>  kernel/trace/trace_events.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/kernel/trace/trace_events.c b/kernel/trace/trace_events.c
> index 120531268abf..d01e5c910ce1 100644
> --- a/kernel/trace/trace_events.c
> +++ b/kernel/trace/trace_events.c
> @@ -3136,7 +3136,10 @@ __register_event(struct trace_event_call *call, struct 
> module *mod)
>       if (ret < 0)
>               return ret;
>  
> +     down_write(&trace_event_sem);
>       list_add(&call->list, &ftrace_events);
> +     up_write(&trace_event_sem);
> +
>       if (call->flags & TRACE_EVENT_FL_DYNAMIC)
>               atomic_set(&call->refcnt, 0);
>       else
> @@ -3750,6 +3753,8 @@ __trace_add_event_dirs(struct trace_array *tr)
>       struct trace_event_call *call;
>       int ret;
>  
> +     lockdep_assert_held(&trace_event_sem);
> +
>       list_for_each_entry(call, &ftrace_events, list) {
>               ret = __trace_add_new_event(call, tr);
>               if (ret < 0)
> -- 
> 2.47.2
> 


-- 
Masami Hiramatsu (Google) <[email protected]>

Reply via email to