diff options
| author | Donggeun Yoo <donggeunyoo.kernel@gmail.com> | 2026-09-07 18:14:15 +0900 |
|---|---|---|
| committer | Steven Rostedt <rostedt@goodmis.org> | 2026-09-11 13:54:36 -0400 |
| commit | 6ede78d0563a2a3ae3e46f9c07cedb5d79645429 (patch) | |
| tree | 048d2840572376578436ca9f37c131038da8276d | |
| parent | 0999d3e16d13b6299fd7cc7a7fb2825c18e90dd0 (diff) | |
| download | linux-6ede78d0563a2a3ae3e46f9c07cedb5d79645429.tar.gz linux-6ede78d0563a2a3ae3e46f9c07cedb5d79645429.zip | |
tracing: Set the trace clock before registering the histogram trigger
hist_register_trigger() puts the trigger on the global named_triggers
list in cmd_ops->init(), and only then sets the trace clock:
if (data->cmd_ops->init) {
ret = data->cmd_ops->init(data);
if (ret < 0)
goto out;
}
if (hist_data->enable_timestamps) {
ret = tracing_set_clock(file->tr, hist_data->attrs->clock);
if (ret) {
hist_err(tr, HIST_ERR_SET_CLOCK_FAIL, errpos(clock));
goto out;
}
The clock string is not checked anywhere before that call, so a named
trigger using common_timestamp with an unknown clock fails after it has
already become findable. event_hist_trigger_parse() then frees it
without taking it off the list, and the next lookup by name reads the
freed object:
~# cd /sys/kernel/tracing/events/sched/sched_switch
~# echo 'hist:name=foo:keys=common_pid:ts=common_timestamp:clock=bogus' > trigger
bash: echo: write error: Invalid argument
~# echo 'hist:name=foo:keys=common_pid' > trigger
BUG: KASAN: slab-use-after-free in find_named_trigger+0xac/0xc0
Read of size 8 at addr ffff88800915d760 by task init/1
find_named_trigger+0xac/0xc0
hist_register_trigger+0xc1/0x900
event_hist_trigger_parse+0x3146/0x6af0
event_trigger_write+0xce/0x160
Freed by task 63:
kfree+0x154/0x420
trigger_kthread_fn+0xfd/0x160
Set the clock before the trigger is registered, so that nothing which
can fail runs after it is published, the way commit 6f86bdeab633
("tracing: Fix bad hist from corrupting named_triggers list") moved the
registration below the rest of the setup.
tracing_set_filter_buffering() is reference counted, so the init failure
path has to drop the reference that the clock block now takes first.
Cc: stable@vger.kernel.org
Fixes: a4072fe85ba3 ("tracing: Add a clock attribute for hist triggers")
Link: https://patch.msgid.link/20260907091415.554535-1-donggeunyoo.kernel@gmail.com
Signed-off-by: Donggeun Yoo <donggeunyoo.kernel@gmail.com>
Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
| -rw-r--r-- | kernel/trace/trace_events_hist.c | 15 |
1 files changed, 9 insertions, 6 deletions
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c index 5e00da2d5b1a..1889e310b73c 100644 --- a/kernel/trace/trace_events_hist.c +++ b/kernel/trace/trace_events_hist.c @@ -6631,12 +6631,6 @@ static int hist_register_trigger(char *glob, data->cmd_ops = cmd_ops; } - if (data->cmd_ops->init) { - ret = data->cmd_ops->init(data); - if (ret < 0) - goto out; - } - if (hist_data->enable_timestamps) { char *clock = hist_data->attrs->clock; @@ -6649,6 +6643,15 @@ static int hist_register_trigger(char *glob, tracing_set_filter_buffering(file->tr, true); } + if (data->cmd_ops->init) { + ret = data->cmd_ops->init(data); + if (ret < 0) { + if (hist_data->enable_timestamps) + tracing_set_filter_buffering(file->tr, false); + goto out; + } + } + if (named_data) { remove_hist_vars(hist_data); destroy_hist_data(hist_data); |
