On Sun, 20 Sep 2026 12:21:39 +0800
Hao Ge <[email protected]> wrote:
> 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 every tracer has a set of flags:
> register_tracer() assigns the empty no_tracer_flags to tracers that
> define none. The existing users already handle opts == NULL, so these
> tracers keep behaving as before.
Nice catch, but unfortunately this patch causes this:
[ 13.937660] WARNING: kernel/trace/trace.c:8123 at
add_tracer.part.0+0x495/0x930, CPU#3: swapper/0/1
[ 13.947844] Modules linked in:
[ 13.949520] CPU: 3 UID: 0 PID: 1 Comm: swapper/0 Not tainted
7.3.0-rc7-test-00017-g26bd9362561c #543 PREEMPT(lazy)
[ 13.953831] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS
1.17.0-debian-1.17.0-1 04/01/2014
[ 13.957723] RIP: 0010:add_tracer.part.0+0x495/0x930
[ 13.959852] Code: 85 73 03 00 00 49 3b 6d 10 74 26 4c 89 e8 48 c1 e8 03 42
80 3c 38 00 0f 85 3c 03 00 00 4d 8b 6d 00 49 81 fd 28 5b d8 85 75 c5 <0f> 0b e9
73 ff ff ff 48 b8 00 00 00 00 00 fc ff df 49 8d 7d 18 48
[ 13.967362] RSP: 0000:ffff888100e67bf0 EFLAGS: 00010246
[ 13.969686] RAX: 1ffff11020012790 RBX: ffff888109fc62c0 RCX: ffff888109fc62d0
[ 13.972635] RDX: ffff888109fc63c0 RSI: 1ffff1102013c3f2 RDI: ffff888100093c90
[ 13.975626] RBP: ffffffff865c0640 R08: 1ffffffff1006101 R09: 1ffffffff0cb7da4
[ 13.980293] R10: 0000000000000003 R11: fffffbfff0cb7da5 R12: ffff8881009e0000
[ 13.983285] R13: ffffffff85d85b28 R14: fffffbfff0cb80db R15: dffffc0000000000
[ 13.986297] FS: 0000000000000000(0000) GS:ffff8882b4cbf000(0000)
knlGS:0000000000000000
[ 13.989781] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 13.992213] CR2: 0000000000000000 CR3: 0000000005aec001 CR4: 0000000000172ef0
[ 13.995221] Call Trace:
[ 13.996469] <TASK>
[ 13.997680] ? __pfx_init_hwlat_tracer+0x10/0x10
[ 13.999725] register_tracer+0x30f/0x470
[ 14.001594] ? __pfx_init_hwlat_tracer+0x10/0x10
[ 14.003632] init_hwlat_tracer+0x2c/0x120
[ 14.005539] ? __pfx_init_hwlat_tracer+0x10/0x10
[ 14.007604] do_one_initcall+0x11e/0x540
[ 14.011129] ? __pfx_do_one_initcall+0x10/0x10
[ 14.013124] ? __pfx_parse_args+0x10/0x10
[ 14.015006] ? kernel_init_freeable+0x4cb/0x710
[ 14.017032] ? rcu_is_watching+0x16/0xb0
[ 14.018887] kernel_init_freeable+0x65c/0x710
[ 14.020853] ? __pfx_kernel_init_freeable+0x10/0x10
[ 14.023036] ? _raw_spin_unlock_irq+0x38/0x60
[ 14.024995] ? __pfx_do_raw_spin_lock+0x10/0x10
[ 14.027045] ? __pfx_kernel_init+0x10/0x10
[ 14.028894] ? rcu_is_watching+0x16/0xb0
[ 14.030718] ? __pfx_kernel_init+0x10/0x10
[ 14.032555] kernel_init+0x21/0x150
[ 14.034249] ? __pfx_kernel_init+0x10/0x10
[ 14.036086] ret_from_fork+0x65c/0x9e0
[ 14.037893] ? __pfx_ret_from_fork+0x10/0x10
[ 14.041417] ? __switch_to+0x3ad/0xda0
[ 14.043154] ? __switch_to_asm+0x39/0x70
[ 14.044953] ? __switch_to_asm+0x33/0x70
[ 14.046792] ? __pfx_kernel_init+0x10/0x10
[ 14.048646] ret_from_fork_asm+0x1a/0x30
>
> Fixes: e29aa918a928 ("tracing: Have function graph tracer define options per
> instance")
> Cc: [email protected]
> Signed-off-by: Hao Ge <[email protected]>
> ---
> kernel/trace/trace.c | 13 +++++++++----
> 1 file changed, 9 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index e4a490d3d08c..1bd757b65be4 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
> @@ -1485,9 +1491,11 @@ int __init register_tracer(struct tracer *type)
> }
> }
>
> - /* store the tracer for __set_tracer_option */
> + /* All tracers must have flags, use the empty flags if none are defined
> */
> if (type->flags)
> type->flags->trace = type;
> + else if (!type->default_flags)
> + type->default_flags = &no_tracer_flags;
>
> ret = do_run_tracer_selftest(type);
> if (ret < 0)
> @@ -8096,9 +8104,6 @@ static int add_tracer(struct trace_array *tr, struct
> tracer *tracer)
>
> flags = tracer->flags;
> if (!flags) {
> - if (!tracer->default_flags)
> - return 0;
> -
> /*
> * If the tracer defines default flags, it means the flags are
> * per trace instance.
Instead of assigning to the tracer, how about just updating
tr->current_trace_flags?
Like this:
diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
index 3b22e70c6016..b6b7d86866ef 100644
--- a/kernel/trace/trace.c
+++ b/kernel/trace/trace.c
@@ -5001,6 +5001,12 @@ static void tracing_set_nop(struct trace_array *tr)
static bool tracer_options_updated;
+/*
+ * 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;
+
int tracing_set_tracer(struct trace_array *tr, const char *buf)
{
struct tracer *trace = NULL;
@@ -5090,6 +5096,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);
It appears to fix the issue you found without any other side effects.
Care to send a v2?
Thanks,
-- Steve
