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

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

[Hao Ge]

tracing_set_tracer() assigns the flags of the newly selected tracer
with:

	tr->current_trace_flags = t->flags ? : t->tracer->flags;

If the new tracer defines neither static flags nor default flags, like
hwlat, irqsoff or wakeup, add_tracer() has no per instance flags copy
for it, and both operands are NULL, so tr->current_trace_flags ends up
being NULL as well.

The ring buffer is not cleared when switching tracers: it is reset
inside tracer_init(), after current_trace_flags was already assigned.
If the buffer still holds function_graph entries of the previous
tracer, printing one of them dereferences the NULL pointer. This was
hit by cat trace during an ftrace stress test:

  BUG: kernel NULL pointer dereference, address: 0000000000000000
  #PF: supervisor read access in kernel mode
  RIP: 0010:print_graph_function_event+0xe/0x20
  Call Trace:
   s_show+0x4d/0x110
   seq_read_iter+0x2d4/0x490
   seq_read+0x130/0x170
   vfs_read+0xb5/0x370
   ksys_read+0x6d/0xe0
   do_syscall_64+0xbc/0x450

The snapshot buffer is not reset at all, so this can be reproduced
without a race:

  echo function_graph > /sys/kernel/tracing/current_tracer
  echo 1 > /sys/kernel/tracing/snapshot
  echo hwlat > /sys/kernel/tracing/current_tracer
  cat /sys/kernel/tracing/snapshot

Rather than teaching every user of tr->current_trace_flags about this
corner case, make sure it is never NULL: when the newly selected
tracer has no flags at all, tracing_set_tracer() falls back to a
static empty no_tracer_flags. run_tracer_selftest() can hit the same
case, so the same fallback is added there too. The existing users
already handle opts == NULL, so these tracers keep behaving as before.

Fixes: e29aa918a928 ("tracing: Have function graph tracer define options per instance")
Cc: stable@vger.kernel.org
Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Signed-off-by: Hao Ge <hao.ge@linux.dev>
---
Changes in v2:
- Rework per Steve's suggestion. v1 could trip the WARN_ON_ONCE() in
  add_tracer() at boot, when an instance was created before the tracer
  registers.

v1 Link: https://lore.kernel.org/all/20260920042139.70736-1-hao.ge@linux.dev/
---
 kernel/trace/trace.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index e4a490d3d08c..fba9521d2511 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -99,6 +99,12 @@ struct tracers {
 	struct tracer_flags	*flags;
 };
 
+/*
+ * Tracers that do not define their own flags use these empty flags,
+ * as tr->current_trace_flags must never be NULL.
+ */
+static struct tracer_flags no_tracer_flags;
+
 /*
  * To prevent the comm cache from being overwritten when no
  * tracing is active, only save the comm when a trace event
@@ -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;
 
 #ifdef CONFIG_TRACER_MAX_TRACE
 	if (tracer_uses_snapshot(type)) {
@@ -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;
 
 	if (trace->init) {
 		ret = tracer_init(trace, tr);
-- 
2.25.1

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

[sashiko-bot]

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.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261015080351.121316-1-hao.ge@linux.dev?part=1

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

[Hao Ge]

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