On Mon, 13 Jul 2026 13:46:40 +0000
Tengda Wu <[email protected]> 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: [email protected]
> Signed-off-by: Tengda Wu <[email protected]>
> ---
> 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
