Re: [PATCH] tracing: Add mutex to trace_parser to fix concurrent write races

[Steven Rostedt <rostedt@goodmis.org>]

On Mon, 13 Jul 2026 13:46:40 +0000
Tengda Wu <wutengda@huaweicloud.com> wrote:

> The trace_parser structure is allocated and initialized when a trace
> file is opened, and is subsequently used in the write handler to parse
> user input. If userspace opens a trace file descriptor and shares it
> across multiple threads, concurrent write calls will race on the
> parser's internal state, specifically the idx, cont, and buffer fields,
> leading to corrupted input or undefined behavior.
> 
> Fix this by embedding a mutex directly in struct trace_parser. The mutex
> is initialized in trace_parser_get_init() and destroyed in
> trace_parser_put(). All write-side users that access parser state
> (trace_get_user() followed by checking trace_parser_loaded() /
> trace_parser_cont() against the buffer) now hold the mutex across the
> full critical section, avoiding any TOCTOU gap between the parse and the
> subsequent consumption of parser->buffer.
> 
> Affected write paths:
>   - ftrace_graph_write / ftrace_graph_release
>   - ftrace_regex_write / ftrace_regex_release
> 
> Fixes: e704eff3ff51 ("ftrace: Have set_graph_function handle multiple functions in one write")
> Fixes: 689fd8b65d66 ("tracing: trace parser support for function and graph")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tengda Wu <wutengda@huaweicloud.com>
> ---
>  kernel/trace/ftrace.c | 7 +++++++
>  kernel/trace/trace.c  | 2 ++
>  kernel/trace/trace.h  | 1 +
>  3 files changed, 10 insertions(+)
> 
> diff --git a/kernel/trace/ftrace.c b/kernel/trace/ftrace.c
> index f93e34dd2328..ef47e5659283 100644
> --- a/kernel/trace/ftrace.c
> +++ b/kernel/trace/ftrace.c
> @@ -5842,6 +5842,8 @@ ftrace_regex_write(struct file *file, const char __user *ubuf,
>  	/* iter->hash is a local copy, so we don't need regex_lock */
>  
>  	parser = &iter->parser;
> +
> +	guard(mutex)(&parser->lock);
>  	read = trace_get_user(parser, ubuf, cnt, ppos);

Why are the other users of trace_get_user() not a problem? If
anything, trace_get_user() should have a lockdep assert to make sure
the lock is held.

I think we need to add a lockdep assertion in all the callers that use
the parser and we need to make sure it's taken by every user.

-- Steve