On 2026/10/15 16:14, [email protected] wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential issue(s)
> to consider:
> - [High] trace: Transient NULL pointer deref in tracing_set_tracer() due to
> non-atomic update
> --
>
> commit 8e34d0e2d1517b818cf24f9067ddfaa0e4b47cd2
> Author: Hao Ge <[email protected]>
>
> tracing: Fix crash in print_graph_function_event()
>
> This commit attempts to fix a NULL pointer dereference in
> print_graph_function_event() by ensuring tr->current_trace_flags is never
> NULL. It provides a fallback to a static empty no_tracer_flags when a
> selected tracer has no flags defined.
>
> v1 Link: https://lore.kernel.org/all/[email protected]/
>
>> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
>> index e4a490d3d08c8..fba9521d25115 100644
>> --- a/kernel/trace/trace.c
>> +++ b/kernel/trace/trace.c
> [ ... ]
>> @@ -1319,6 +1325,8 @@ static int run_tracer_selftest(struct tracer *type)
>>
>> tr->current_trace = type;
>> tr->current_trace_flags = type->flags ? : type->default_flags;
>> + if (!tr->current_trace_flags)
>> + tr->current_trace_flags = &no_tracer_flags;
>
> [Severity: High]
> Does this non-atomic double assignment expose a transient NULL pointer to
> concurrent lockless readers?
>
> A concurrent reader might fetch tr->current_trace_flags during the brief
> window between the first store (which writes NULL if the tracer lacks flags)
> and the second store.
>
> Could we compute the new flags pointer into a local variable first, and then
> assign it to tr->current_trace_flags in a single store, perhaps using
> WRITE_ONCE()?
>
> [ ... ]
>> @@ -5090,6 +5098,8 @@ int tracing_set_tracer(struct trace_array *tr, const
>> char *buf)
>> }
>>
>> tr->current_trace_flags = t->flags ? : t->tracer->flags;
>> + if (!tr->current_trace_flags)
>> + tr->current_trace_flags = &no_tracer_flags;
>
> [Severity: High]
> Can this similarly result in a regression with a transient NULL pointer
> dereference?
>
> While tracing_set_tracer() holds trace_types_lock, readers accessing the
> trace file (via seq_read() -> s_show() -> print_trace_line() ->
> print_graph_function_event()) iterate locklessly.
>
> If a concurrent reader observes the intermediate NULL value, it will
> dereference it (e.g., tr->current_trace_flags->val), leading to the exact
> kernel panic this patch is meant to fix.
>
Thanks, V3 will fix this.
https://lore.kernel.org/all/[email protected]/
Best Regards
Hao
