linux-kernel - Re: [PATCH v2] tracing: Add down_write(trace_event

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <20250719133634.f424cba37241e73b0f774a7f@kernel.org>
Date: Sat, 19 Jul 2025 13:36:34 +0900
From: Masami Hiramatsu (Google) <mhiramat@...nel.org>
To: Steven Rostedt <rostedt@...dmis.org>
Cc: LKML <linux-kernel@...r.kernel.org>, Linux trace kernel
 <linux-trace-kernel@...r.kernel.org>, Masami Hiramatsu
 <mhiramat@...nel.org>, Mathieu Desnoyers <mathieu.desnoyers@...icios.com>,
 "Fusheng Huang( 黄富生) "
 <Fusheng.Huang@...share-ict.com>
Subject: Re: [PATCH v2] tracing: Add down_write(trace_event_sem) when adding
 trace event

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

> From: Steven Rostedt <rostedt@...dmis.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@...nel.org>

Thanks!

> 
> Cc: stable@...r.kernel.org
> Reported-by: Fusheng Huang(黄富生)  <Fusheng.Huang@...share-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@...dmis.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@...nel.org>