tracing: Fix crash in print_graph_function_event()
Subsystems (2)
PATCH v3 1/1 tracing: Fix crash in print_graph_function_event()
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.
---
Changes in v3:
- Sasha pointed out the original two-step assignment had a transient NULL window. Switched both sites to a single if/else if/else.
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/[email protected]/
--- kernel/trace/trace.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-)
kernel/trace/trace.c +18 -2
@@ -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
@@ -1318,7 +1324,12 @@ static int run_tracer_selftest(struct tracer *type)tracing_reset_online_cpus (&tr->array_buffer); tr->current_trace = type; - tr->current_trace_flags = type->flags ? : type->default_flags; +if (type->flags) + tr->current_trace_flags = type->flags; +else if (type->default_flags) + tr->current_trace_flags = type->default_flags; +else + tr->current_trace_flags = &no_tracer_flags;#ifdef CONFIG_TRACER_MAX_TRACE if (tracer_uses_snapshot (type)) {
@@ -5089,7 +5100,12 @@ int tracing_set_tracer(struct trace_array *tr, const char *buf)return ret; } - tr->current_trace_flags = t->flags ? : t->tracer->flags; +if (t->flags) + tr->current_trace_flags = t->flags; +else if (t->tracer->flags) + tr->current_trace_flags = t->tracer->flags; +else + tr->current_trace_flags = &no_tracer_flags;if (trace->init) { ret =tracer_init (trace, tr);
Signature · 1 line
2.25.1