Commit 387501f
committed
tracing: Fix bad hist from corrupting named_triggers list
JIRA: https://issues.redhat.com/browse/RHEL-80060
commit 6f86bde
Author: Steven Rostedt <rostedt@goodmis.org>
Date: Thu Feb 27 16:39:44 2025 -0500
tracing: Fix bad hist from corrupting named_triggers list
The following commands causes a crash:
~# cd /sys/kernel/tracing/events/rcu/rcu_callback
~# echo 'hist:name=bad:keys=common_pid:onmax(bogus).save(common_pid)' > trigger
bash: echo: write error: Invalid argument
~# echo 'hist:name=bad:keys=common_pid' > trigger
Because the following occurs:
event_trigger_write() {
trigger_process_regex() {
event_hist_trigger_parse() {
data = event_trigger_alloc(..);
event_trigger_register(.., data) {
cmd_ops->reg(.., data, ..) [hist_register_trigger()] {
data->ops->init() [event_hist_trigger_init()] {
save_named_trigger(name, data) {
list_add(&data->named_list, &named_triggers);
}
}
}
}
ret = create_actions(); (return -EINVAL)
if (ret)
goto out_unreg;
[..]
ret = hist_trigger_enable(data, ...) {
list_add_tail_rcu(&data->list, &file->triggers); <<<---- SKIPPED!!! (this is important!)
[..]
out_unreg:
event_hist_unregister(.., data) {
cmd_ops->unreg(.., data, ..) [hist_unregister_trigger()] {
list_for_each_entry(iter, &file->triggers, list) {
if (!hist_trigger_match(data, iter, named_data, false)) <- never matches
continue;
[..]
test = iter;
}
if (test && test->ops->free) <<<-- test is NULL
test->ops->free(test) [event_hist_trigger_free()] {
[..]
if (data->name)
del_named_trigger(data) {
list_del(&data->named_list); <<<<-- NEVER gets removed!
}
}
}
}
[..]
kfree(data); <<<-- frees item but it is still on list
The next time a hist with name is registered, it causes an u-a-f bug and
the kernel can crash.
Move the code around such that if event_trigger_register() succeeds, the
next thing called is hist_trigger_enable() which adds it to the list.
A bunch of actions is called if get_named_trigger_data() returns false.
But that doesn't need to be called after event_trigger_register(), so it
can be moved up, allowing event_trigger_register() to be called just
before hist_trigger_enable() keeping them together and allowing the
file->triggers to be properly populated.
Cc: stable@vger.kernel.org
Cc: Masami Hiramatsu <mhiramat@kernel.org>
Cc: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>
Link: https://lore.kernel.org/20250227163944.1c37f85f@gandalf.local.home
Fixes: 067fe03 ("tracing: Add variable reference handling to hist triggers")
Reported-by: Tomas Glozar <tglozar@redhat.com>
Tested-by: Tomas Glozar <tglozar@redhat.com>
Reviewed-by: Tom Zanussi <zanussi@kernel.org>
Closes: https://lore.kernel.org/all/CAP4=nvTsxjckSBTz=Oe_UYh8keD9_sZC4i++4h72mJLic4_W4A@mail.gmail.com/
Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
Signed-off-by: Tomas Glozar <tglozar@redhat.com>1 parent e1027aa commit 387501f
1 file changed
+15
-15
lines changed| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
6652 | 6652 | | |
6653 | 6653 | | |
6654 | 6654 | | |
6655 | | - | |
6656 | | - | |
6657 | | - | |
| 6655 | + | |
6658 | 6656 | | |
6659 | | - | |
6660 | | - | |
| 6657 | + | |
| 6658 | + | |
| 6659 | + | |
6661 | 6660 | | |
6662 | | - | |
6663 | | - | |
6664 | | - | |
| 6661 | + | |
| 6662 | + | |
| 6663 | + | |
| 6664 | + | |
| 6665 | + | |
6665 | 6666 | | |
6666 | | - | |
6667 | | - | |
| 6667 | + | |
6668 | 6668 | | |
6669 | | - | |
| 6669 | + | |
6670 | 6670 | | |
6671 | 6671 | | |
6672 | | - | |
6673 | | - | |
6674 | | - | |
6675 | | - | |
| 6672 | + | |
| 6673 | + | |
| 6674 | + | |
| 6675 | + | |
6676 | 6676 | | |
6677 | 6677 | | |
6678 | 6678 | | |
| |||
0 commit comments