[PATCH v2] tracing: Add down_write(trace_event_sem) when adding trace event

[PATCH v2] tracing: Add down_write(trace_event_sem) when adding trace event

[Steven Rostedt]

From: Steven Rostedt <rostedt@goodmis.org>

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.

Cc: stable@vger.kernel.org
Reported-by: Fusheng Huang(黄富生)  <Fusheng.Huang@luxshare-ict.com>
Closes: https://lore.kernel.org/all/20250717105007.46ccd18f@batman.local.home/
Fixes: 110bf2b764eb6 ("tracing: add protection around module events unload")
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
---
Changes since v1: https://lore.kernel.org/20250717131204.74559b88@batman.local.home

- 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

Re: [PATCH v2] tracing: Add down_write(trace_event_sem) when adding trace event

[Masami Hiramatsu]

On Fri, 18 Jul 2025 22:31:58 -0400
Steven Rostedt <rostedt@goodmis.org> wrote:

> From: Steven Rostedt <rostedt@goodmis.org>
> 
> 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) <mhiramat@kernel.org>

Thanks!

> 
> Cc: stable@vger.kernel.org
> Reported-by: Fusheng Huang(黄富生)  <Fusheng.Huang@luxshare-ict.com>
> Closes: https://lore.kernel.org/all/20250717105007.46ccd18f@batman.local.home/
> Fixes: 110bf2b764eb6 ("tracing: add protection around module events unload")
> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
> ---
> Changes since v1: https://lore.kernel.org/20250717131204.74559b88@batman.local.home
> 
> - 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) <mhiramat@kernel.org>