Re: [PATCH v2] tracing: Fix crash in print_graph_function_event()

[Hao Ge <hao.ge@linux.dev>]

On 2026/10/15 16:14, sashiko-bot@kernel.org 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 <hao.ge@linux.dev>
> 
> 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/20260920042139.70736-1-hao.ge@linux.dev/
> 
>> 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/20261015130658.221198-1-hao.ge@linux.dev/

Best Regards
Hao