FAILED: patch "[PATCH] tracing: Undo the registration when enabling the histogram" failed to apply to 6.6-stable tree
Subsystems (2)
[email protected]
maintainerThe patch below does not apply to the 6.6-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.
To reproduce the conflict and resubmit, you may use the following commands:
git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.6.y
git checkout FETCH_HEAD
git cherry-pick -x 92383cef66791a0c63a2f27755cadbdb2fbf270b
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026091601-uncurled-paddle-226d@gregkh' --subject-prefix 'PATCH 6.6.y' 'HEAD^..'
Possible dependencies:
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 92383cef66791a0c63a2f27755cadbdb2fbf270b Mon Sep 17 00:00:00 2001
From: Donggeun Yoo <[email protected]>
Date: Mon, 7 Sep 2026 21:44:20 +0900
Subject: [PATCH] tracing: Undo the registration when enabling the histogram
trigger fails
Commit 6f86bdeab633 ("tracing: Fix bad hist from corrupting named_triggers list") described how a trigger that is registered but not on file->triggers ends up freed while still on the global named_triggers list, and moved the registration down so that hist_trigger_enable() follows it immediately. One path still gets there. hist_trigger_enable() adds the trigger and takes it straight back out when the event cannot be enabled:
list_add_tail_rcu(&data->list, &file->triggers);
update_cond_flag(file);
if (trace_event_trigger_enable_disable(file, 1) < 0) {
list_del_rcu(&data->list);
update_cond_flag(file);
ret--;
}so the list walk in hist_unregister_trigger() matches nothing, test stays NULL, and the ->free() that would call del_named_trigger() is skipped. out_unreg falls through to out_free, which frees the trigger anyway:
BUG: KASAN: slab-use-after-free in find_named_trigger+0xac/0xc0 Read of size 8 at addr ffff8880091d3160 by task init/1 find_named_trigger+0xac/0xc0 hist_register_trigger+0xc1/0xa00 event_hist_trigger_parse+0x3146/0x6af0 event_trigger_write+0xce/0x160 Freed by task 69: kfree+0x154/0x420 trigger_kthread_fn+0xfd/0x160
Leave the trigger where hist_unregister_trigger() can find it and let that undo the registration, which is the only code that knows all of what cmd_ops->init() took: the named list entry, the hist_pad reference, the reference on the trigger a named histogram is shared with, and the copied cmd_ops. It also pairs the failed trace_event_trigger_enable_disable(), whose sm_ref and buffered event reference are otherwise left behind.
Since ->free() releases trigger_data and, for a trigger that does not share its histogram, hist_data with it, out_unreg can no longer fall through to out_free. For a trigger that does share, hist_register_trigger() has already destroyed the caller's hist_data, so the fall-through was reading freed memory there as well.
Move the enable_timestamps check in hist_unregister_trigger() above the ->free() call for the same reason: hist_data does not outlive it once the trigger being removed is the one that owns it.
kernel/trace/trace_events_hist.c +10 -7
@@ -6670,11 +6670,12 @@ static int hist_trigger_enable(struct event_trigger_data *data,update_cond_flag (file); -if (trace_event_trigger_enable_disable (file,1 ) <0 ) { -list_del_rcu (&data->list); -update_cond_flag (file); +/* +* On failure the caller undoes the registration, and +* hist_unregister_trigger() can only find the trigger here. +*/ +if (trace_event_trigger_enable_disable (file,1 ) <0 ) ret--; - }return ret; }
@@ -6752,13 +6753,13 @@ static void hist_unregister_trigger(char *glob, } } -if (test && test->cmd_ops->free) - test->cmd_ops->free (test); -if (hist_data->enable_timestamps) {if (!hist_data->remove || test)tracing_set_filter_buffering (file->tr, false); } + +if (test && test->cmd_ops->free) + test->cmd_ops->free (test); }static bool hist_file_check_refs (struct trace_event_file *file)
@@ -6963,6 +6964,8 @@ static int event_hist_trigger_parse(struct event_command *cmd_ops,return ret; out_unreg:event_trigger_unregister (cmd_ops, file, glob+1 , trigger_data); +/* The unregister frees trigger_data, skip out_free */ + goto out; out_free:remove_hist_vars (hist_data);
Sasha Levin
maintainerPATCH 1/3 tracing: Constify struct event_trigger_ops
From: Christophe JAILLET <[email protected]>
[ Upstream commit 502d2e71a89fa706843fa9277f4d6de1947072c8 ]
'event_trigger_ops mwifiex_if_ops' are not modified in these drivers.
Constifying these structures moves some data to a read-only section, so increase overall security, especially when the structure holds some function pointers.
On a x86_64, with allmodconfig, as an example:
Before:
======
text data bss dec hex filename
31368 9024 6200 46592 b600 kernel/trace/trace_events_trigger.o
After:
=====
text data bss dec hex filename
31752 8608 6200 46560 b5e0 kernel/trace/trace_events_trigger.o
Stable-dep-of: 92383cef6679 ("tracing: Undo the registration when enabling the histogram trigger fails")
--- kernel/trace/trace.h | 4 +-- kernel/trace/trace_eprobe.c | 6 ++--- kernel/trace/trace_events_hist.c | 20 +++++++-------- kernel/trace/trace_events_trigger.c | 38 ++++++++++++++--------------- 4 files changed, 34 insertions(+), 34 deletions(-)
kernel/trace/trace.h +2 -2
@@ -1614,7 +1614,7 @@ struct event_trigger_data {unsigned long count;int ref;int flags; -struct event_trigger_ops *ops; +const struct event_trigger_ops *ops;struct event_command *cmd_ops;struct event_filter __rcu *filter;char *filter_str;
@@ -1857,7 +1857,7 @@ struct event_command {int (*set_filter)(char *filter_str,struct event_trigger_data *data,struct trace_event_file *file); -struct event_trigger_ops *(*get_trigger_ops)(char *cmd,char *param); +const struct event_trigger_ops *(*get_trigger_ops)(char *cmd,char *param); };/**
kernel/trace/trace_eprobe.c +3 -3
@@ -479,7 +479,7 @@ static void eprobe_trigger_func(struct event_trigger_data *data,__eprobe_trace_func (edata, rec); } -static struct event_trigger_ops eprobe_trigger_ops = { +static const struct event_trigger_ops eprobe_trigger_ops = { .trigger = eprobe_trigger_func, .print = eprobe_trigger_print, .init = eprobe_trigger_init,
@@ -508,8 +508,8 @@ static void eprobe_trigger_unreg_func(char *glob, } -static struct event_trigger_ops *eprobe_trigger_get_ops (char *cmd, -char *param) +static const struct event_trigger_ops *eprobe_trigger_get_ops (char *cmd, +char *param) {return &eprobe_trigger_ops; }
kernel/trace/trace_events_hist.c +10 -10
@@ -6334,7 +6334,7 @@ static void event_hist_trigger_free(struct event_trigger_data *data)free_hist_pad (); } -static struct event_trigger_ops event_hist_trigger_ops = { +static const struct event_trigger_ops event_hist_trigger_ops = { .trigger = event_hist_trigger, .print = event_hist_trigger_print, .init = event_hist_trigger_init,
@@ -6364,15 +6364,15 @@ static void event_hist_trigger_named_free(struct event_trigger_data *data) } } -static struct event_trigger_ops event_hist_trigger_named_ops = { +static const struct event_trigger_ops event_hist_trigger_named_ops = { .trigger = event_hist_trigger, .print = event_hist_trigger_print, .init = event_hist_trigger_named_init, .free = event_hist_trigger_named_free, }; -static struct event_trigger_ops *event_hist_get_trigger_ops (char *cmd, -char *param) +static const struct event_trigger_ops *event_hist_get_trigger_ops (char *cmd, +char *param) {return &event_hist_trigger_ops; }
@@ -6969,38 +6969,38 @@ hist_enable_count_trigger(struct event_trigger_data *data,hist_enable_trigger (data, buffer, rec, event); } -static struct event_trigger_ops hist_enable_trigger_ops = { +static const struct event_trigger_ops hist_enable_trigger_ops = { .trigger = hist_enable_trigger, .print = event_enable_trigger_print, .init = event_trigger_init, .free = event_enable_trigger_free, }; -static struct event_trigger_ops hist_enable_count_trigger_ops = { +static const struct event_trigger_ops hist_enable_count_trigger_ops = { .trigger = hist_enable_count_trigger, .print = event_enable_trigger_print, .init = event_trigger_init, .free = event_enable_trigger_free, }; -static struct event_trigger_ops hist_disable_trigger_ops = { +static const struct event_trigger_ops hist_disable_trigger_ops = { .trigger = hist_enable_trigger, .print = event_enable_trigger_print, .init = event_trigger_init, .free = event_enable_trigger_free, }; -static struct event_trigger_ops hist_disable_count_trigger_ops = { +static const struct event_trigger_ops hist_disable_count_trigger_ops = { .trigger = hist_enable_count_trigger, .print = event_enable_trigger_print, .init = event_trigger_init, .free = event_enable_trigger_free, }; -static struct event_trigger_ops * +static const struct event_trigger_ops *hist_enable_get_trigger_ops (char *cmd,char *param) { -struct event_trigger_ops *ops; +const struct event_trigger_ops *ops;bool enable; enable = (strcmp (cmd,ENABLE_HIST_STR ) ==0 );
kernel/trace/trace_events_trigger.c +19 -19
@@ -833,7 +833,7 @@ struct event_trigger_data *trigger_data_alloc(struct event_command *cmd_ops, void *private_data) {struct event_trigger_data *trigger_data; -struct event_trigger_ops *trigger_ops; +const struct event_trigger_ops *trigger_ops; trigger_ops = cmd_ops->get_trigger_ops (cmd, param);
@@ -1375,38 +1375,38 @@ traceoff_trigger_print(struct seq_file *m, struct event_trigger_data *data) data->filter_str); } -static struct event_trigger_ops traceon_trigger_ops = { +static const struct event_trigger_ops traceon_trigger_ops = { .trigger = traceon_trigger, .print = traceon_trigger_print, .init = event_trigger_init, .free = event_trigger_free, }; -static struct event_trigger_ops traceon_count_trigger_ops = { +static const struct event_trigger_ops traceon_count_trigger_ops = { .trigger = traceon_count_trigger, .print = traceon_trigger_print, .init = event_trigger_init, .free = event_trigger_free, }; -static struct event_trigger_ops traceoff_trigger_ops = { +static const struct event_trigger_ops traceoff_trigger_ops = { .trigger = traceoff_trigger, .print = traceoff_trigger_print, .init = event_trigger_init, .free = event_trigger_free, }; -static struct event_trigger_ops traceoff_count_trigger_ops = { +static const struct event_trigger_ops traceoff_count_trigger_ops = { .trigger = traceoff_count_trigger, .print = traceoff_trigger_print, .init = event_trigger_init, .free = event_trigger_free, }; -static struct event_trigger_ops * +static const struct event_trigger_ops *onoff_get_trigger_ops (char *cmd,char *param) { -struct event_trigger_ops *ops; +const struct event_trigger_ops *ops;/* we register both traceon and traceoff to this callback */ if (strcmp (cmd,"traceon" ) ==0 )
@@ -1488,21 +1488,21 @@ snapshot_trigger_print(struct seq_file *m, struct event_trigger_data *data) data->filter_str); } -static struct event_trigger_ops snapshot_trigger_ops = { +static const struct event_trigger_ops snapshot_trigger_ops = { .trigger = snapshot_trigger, .print = snapshot_trigger_print, .init = event_trigger_init, .free = event_trigger_free, }; -static struct event_trigger_ops snapshot_count_trigger_ops = { +static const struct event_trigger_ops snapshot_count_trigger_ops = { .trigger = snapshot_count_trigger, .print = snapshot_trigger_print, .init = event_trigger_init, .free = event_trigger_free, }; -static struct event_trigger_ops * +static const struct event_trigger_ops *snapshot_get_trigger_ops (char *cmd,char *param) {return param ? &snapshot_count_trigger_ops : &snapshot_trigger_ops;
@@ -1583,21 +1583,21 @@ stacktrace_trigger_print(struct seq_file *m, struct event_trigger_data *data) data->filter_str); } -static struct event_trigger_ops stacktrace_trigger_ops = { +static const struct event_trigger_ops stacktrace_trigger_ops = { .trigger = stacktrace_trigger, .print = stacktrace_trigger_print, .init = event_trigger_init, .free = event_trigger_free, }; -static struct event_trigger_ops stacktrace_count_trigger_ops = { +static const struct event_trigger_ops stacktrace_count_trigger_ops = { .trigger = stacktrace_count_trigger, .print = stacktrace_trigger_print, .init = event_trigger_init, .free = event_trigger_free, }; -static struct event_trigger_ops * +static const struct event_trigger_ops *stacktrace_get_trigger_ops (char *cmd,char *param) {return param ? &stacktrace_count_trigger_ops : &stacktrace_trigger_ops;
@@ -1708,28 +1708,28 @@ void event_enable_trigger_free(struct event_trigger_data *data) } } -static struct event_trigger_ops event_enable_trigger_ops = { +static const struct event_trigger_ops event_enable_trigger_ops = { .trigger = event_enable_trigger, .print = event_enable_trigger_print, .init = event_trigger_init, .free = event_enable_trigger_free, }; -static struct event_trigger_ops event_enable_count_trigger_ops = { +static const struct event_trigger_ops event_enable_count_trigger_ops = { .trigger = event_enable_count_trigger, .print = event_enable_trigger_print, .init = event_trigger_init, .free = event_enable_trigger_free, }; -static struct event_trigger_ops event_disable_trigger_ops = { +static const struct event_trigger_ops event_disable_trigger_ops = { .trigger = event_enable_trigger, .print = event_enable_trigger_print, .init = event_trigger_init, .free = event_enable_trigger_free, }; -static struct event_trigger_ops event_disable_count_trigger_ops = { +static const struct event_trigger_ops event_disable_count_trigger_ops = { .trigger = event_enable_count_trigger, .print = event_enable_trigger_print, .init = event_trigger_init,
@@ -1913,10 +1913,10 @@ void event_enable_unregister_trigger(char *glob, data->ops->free (data); } -static struct event_trigger_ops * +static const struct event_trigger_ops *event_enable_get_trigger_ops (char *cmd,char *param) { -struct event_trigger_ops *ops; +const struct event_trigger_ops *ops;bool enable;#ifdef CONFIG_HIST_TRIGGERS
Signature · 1 line
2.53.0
Sasha Levin
maintainerPATCH 2/3 tracing: Merge struct event_trigger_ops into struct event_command
From: Steven Rostedt <[email protected]>
[ Upstream commit b052d70f7c9c156409a70e65c10d83b5650e7e78 ]
Now that there's pretty much a one to one mapping between the struct event_trigger_ops and struct event_command, there's no reason to have two different structures. Merge the function pointers of event_trigger_ops into event_command.
There's one exception in trace_events_hist.c for the event_hist_trigger_named_ops. This has special logic for the init and free function pointers for "named histograms". In this case, allocate the cmd_ops of the event_trigger_data and set it to the proper init and free functions, which are used to initialize and free the event_trigger_data respectively. Have the free function and the init function (on failure) free the cmd_ops of the data element.
Backport adaptation for 6.6:
The stable tree still selects separate counted-trigger callbacks through get_trigger_ops(), and lacks the upstream count_func and trigger dispatch refactors. Merge the operation tables into event_command while preserving those existing counted callbacks as count_trigger. Select and cache the appropriate callback in event_trigger_data at allocation time, including the separately allocated eprobe triggers. No functions are added.
Keep the upstream named-histogram command copy and its init/free lifetime handling. Do not import data_ops_trigger(), try_unregister_trigger(), or other helpers absent from this stable tree.
Remove the redundant histogram out_free filter reset, since trigger_data_free() already resets the filter. This also provides the unchanged context needed by 92383cef6679. All three target hunks apply without modification.
Stable-dep-of: 92383cef6679 ("tracing: Undo the registration when enabling the histogram trigger fails")
--- kernel/trace/trace.h | 119 +++++++-------- kernel/trace/trace_eprobe.c | 20 +-- kernel/trace/trace_events_hist.c | 131 +++++++---------- kernel/trace/trace_events_trigger.c | 216 +++++++--------------------- 4 files changed, 155 insertions(+), 331 deletions(-)
kernel/trace/trace.h +50 -69
@@ -1614,7 +1614,10 @@ struct event_trigger_data {unsigned long count;int ref;int flags; -const struct event_trigger_ops *ops; +void (*trigger)(struct event_trigger_data *data, +struct trace_buffer *buffer, +void *rec, +struct ring_buffer_event *rbe);struct event_command *cmd_ops;struct event_filter __rcu *filter;char *filter_str;
@@ -1701,64 +1704,6 @@ extern void event_trigger_unregister(struct event_command *cmd_ops,extern void event_file_get (struct trace_event_file *file);extern void event_file_put (struct trace_event_file *file); -/** -* struct event_trigger_ops - callbacks for trace event triggers -* -* The methods in this structure provide per-event trigger hooks for -* various trigger operations. -* -* The @init and @free methods are used during trigger setup and -* teardown, typically called from an event_command's @parse() -* function implementation. -* -* The @print method is used to print the trigger spec. -* -* The @trigger method is the function that actually implements the -* trigger and is called in the context of the triggering event -* whenever that event occurs. -* -* All the methods below, except for @init() and @free(), must be -* implemented. -* -* @trigger: The trigger 'probe' function called when the triggering -* event occurs. The data passed into this callback is the data -* that was supplied to the event_command @reg() function that -* registered the trigger (see struct event_command) along with -* the trace record, rec. -* -* @init: An optional initialization function called for the trigger -* when the trigger is registered (via the event_command reg() -* function). This can be used to perform per-trigger -* initialization such as incrementing a per-trigger reference -* count, for instance. This is usually implemented by the -* generic utility function @event_trigger_init() (see -* trace_event_triggers.c). -* -* @free: An optional de-initialization function called for the -* trigger when the trigger is unregistered (via the -* event_command @reg() function). This can be used to perform -* per-trigger de-initialization such as decrementing a -* per-trigger reference count and freeing corresponding trigger -* data, for instance. This is usually implemented by the -* generic utility function @event_trigger_free() (see -* trace_event_triggers.c). -* -* @print: The callback function invoked to have the trigger print -* itself. This is usually implemented by a wrapper function -* that calls the generic utility function @event_trigger_print() -* (see trace_event_triggers.c). -*/ -struct event_trigger_ops { -void (*trigger)(struct event_trigger_data *data, -struct trace_buffer *buffer, -void *rec, -struct ring_buffer_event *rbe); -int (*init)(struct event_trigger_data *data); -void (*free)(struct event_trigger_data *data); -int (*print)(struct seq_file *m, -struct event_trigger_data *data); -}; -/** * struct event_command - callbacks and data members for event commands *
@@ -1808,7 +1753,7 @@ struct event_trigger_ops {* * @reg: Adds the trigger to the list of triggers associated with the * event, and enables the event trigger itself, after -* initializing it (via the event_trigger_ops @init() function). +* initializing it (via the event_command @init() function). * This is also where commands can use the @trigger_type value to * make the decision as to whether or not multiple instances of * the trigger should be allowed. This is usually implemented by
@@ -1817,7 +1762,7 @@ struct event_trigger_ops {* * @unreg: Removes the trigger from the list of triggers associated * with the event, and disables the event trigger itself, after -* initializing it (via the event_trigger_ops @free() function). +* initializing it (via the event_command @free() function). * This is usually implemented by the generic utility function * @unregister_trigger() (see trace_event_triggers.c). *
@@ -1831,12 +1776,37 @@ struct event_trigger_ops {* ignored. This is usually implemented by the generic utility * function @set_trigger_filter() (see trace_event_triggers.c). * -* @get_trigger_ops: The callback function invoked to retrieve the -* event_trigger_ops implementation associated with the command. -* This callback function allows a single event_command to -* support multiple trigger implementations via different sets of -* event_trigger_ops, depending on the value of the @param -* string. +* @trigger: The trigger 'probe' function called when the triggering +* event occurs. The data passed into this callback is the data +* that was supplied to the event_command @reg() function that +* registered the trigger (see struct event_command) along with +* the trace record, rec. +* +* @count_trigger: An optional trigger callback used instead of @trigger +* when a count parameter is supplied. This callback implements both +* the count handling and the trigger action. +* +* @init: An optional initialization function called for the trigger +* when the trigger is registered (via the event_command reg() +* function). This can be used to perform per-trigger +* initialization such as incrementing a per-trigger reference +* count, for instance. This is usually implemented by the +* generic utility function @event_trigger_init() (see +* trace_event_triggers.c). +* +* @free: An optional de-initialization function called for the +* trigger when the trigger is unregistered (via the +* event_command @reg() function). This can be used to perform +* per-trigger de-initialization such as decrementing a +* per-trigger reference count and freeing corresponding trigger +* data, for instance. This is usually implemented by the +* generic utility function @event_trigger_free() (see +* trace_event_triggers.c). +* +* @print: The callback function invoked to have the trigger print +* itself. This is usually implemented by a wrapper function +* that calls the generic utility function @event_trigger_print() +* (see trace_event_triggers.c). */ struct event_command {struct list_head list;
@@ -1857,7 +1827,18 @@ struct event_command {int (*set_filter)(char *filter_str,struct event_trigger_data *data,struct trace_event_file *file); -const struct event_trigger_ops *(*get_trigger_ops)(char *cmd,char *param); +void (*trigger)(struct event_trigger_data *data, +struct trace_buffer *buffer, +void *rec, +struct ring_buffer_event *rbe); +int (*init)(struct event_trigger_data *data); +void (*free)(struct event_trigger_data *data); +int (*print)(struct seq_file *m, +struct event_trigger_data *data); +void (*count_trigger)(struct event_trigger_data *data, +struct trace_buffer *buffer, +void *rec, +struct ring_buffer_event *rbe); };/**
@@ -1878,7 +1859,7 @@ struct event_command {* either committed or discarded. At that point, if any commands * have deferred their triggers, those commands are finally * invoked following the close of the current event. In other -* words, if the event_trigger_ops @func() probe implementation +* words, if the event_command @func() probe implementation * itself logs to the trace buffer, this flag should be set, * otherwise it can be left unspecified. *
kernel/trace/trace_eprobe.c +5 -15
@@ -479,13 +479,6 @@ static void eprobe_trigger_func(struct event_trigger_data *data,__eprobe_trace_func (edata, rec); } -static const struct event_trigger_ops eprobe_trigger_ops = { - .trigger = eprobe_trigger_func, - .print = eprobe_trigger_print, - .init = eprobe_trigger_init, - .free = eprobe_trigger_free, -}; - static int eprobe_trigger_cmd_parse(struct event_command *cmd_ops, struct trace_event_file *file, char *glob, char *cmd,
@@ -508,12 +501,6 @@ static void eprobe_trigger_unreg_func(char *glob, } -static const struct event_trigger_ops *eprobe_trigger_get_ops (char *cmd, -char *param) -{ -return &eprobe_trigger_ops; -} -static struct event_command event_trigger_cmd = { .name ="eprobe" , .trigger_type =ETT_EVENT_EPROBE ,
@@ -522,8 +509,11 @@ static struct event_command event_trigger_cmd = { .reg = eprobe_trigger_reg_func, .unreg = eprobe_trigger_unreg_func, .unreg_all = NULL, - .get_trigger_ops = eprobe_trigger_get_ops, .set_filter = NULL, + .trigger = eprobe_trigger_func, + .print = eprobe_trigger_print, + .init = eprobe_trigger_init, + .free = eprobe_trigger_free, }; static struct event_trigger_data *
@@ -543,7 +533,7 @@ new_eprobe_trigger(struct trace_eprobe *ep, struct trace_event_file *file) trigger->flags =EVENT_TRIGGER_FL_PROBE ; trigger->count =-1 ; - trigger->ops = &eprobe_trigger_ops; + trigger->trigger = eprobe_trigger_func;/* * EVENT PROBE triggers are not registered as commands with
kernel/trace/trace_events_hist.c +48 -83
227 lines not on this page · raw message
kernel/trace/trace_events_trigger.c +52 -164
372 lines not on this page · raw message
Signature · 1 line
2.53.0
Sasha Levin
maintainerPATCH 3/3 tracing: Undo the registration when enabling the histogram trigger fails
In reply to Sasha LevinFrom: Donggeun Yoo <[email protected]>
[ Upstream commit 92383cef66791a0c63a2f27755cadbdb2fbf270b ]
Commit 6f86bdeab633 ("tracing: Fix bad hist from corrupting named_triggers list") described how a trigger that is registered but not on file->triggers ends up freed while still on the global named_triggers list, and moved the registration down so that hist_trigger_enable() follows it immediately. One path still gets there. hist_trigger_enable() adds the trigger and takes it straight back out when the event cannot be enabled:
list_add_tail_rcu(&data->list, &file->triggers);
update_cond_flag(file);
if (trace_event_trigger_enable_disable(file, 1) < 0) {
list_del_rcu(&data->list);
update_cond_flag(file);
ret--;
}so the list walk in hist_unregister_trigger() matches nothing, test stays NULL, and the ->free() that would call del_named_trigger() is skipped. out_unreg falls through to out_free, which frees the trigger anyway:
BUG: KASAN: slab-use-after-free in find_named_trigger+0xac/0xc0 Read of size 8 at addr ffff8880091d3160 by task init/1 find_named_trigger+0xac/0xc0 hist_register_trigger+0xc1/0xa00 event_hist_trigger_parse+0x3146/0x6af0 event_trigger_write+0xce/0x160 Freed by task 69: kfree+0x154/0x420 trigger_kthread_fn+0xfd/0x160
Leave the trigger where hist_unregister_trigger() can find it and let that undo the registration, which is the only code that knows all of what cmd_ops->init() took: the named list entry, the hist_pad reference, the reference on the trigger a named histogram is shared with, and the copied cmd_ops. It also pairs the failed trace_event_trigger_enable_disable(), whose sm_ref and buffered event reference are otherwise left behind.
Since ->free() releases trigger_data and, for a trigger that does not share its histogram, hist_data with it, out_unreg can no longer fall through to out_free. For a trigger that does share, hist_register_trigger() has already destroyed the caller's hist_data, so the fall-through was reading freed memory there as well.
Move the enable_timestamps check in hist_unregister_trigger() above the ->free() call for the same reason: hist_data does not outlive it once the trigger being removed is the one that owns it.
--- kernel/trace/trace_events_hist.c | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-)
kernel/trace/trace_events_hist.c +10 -7
@@ -6612,11 +6612,12 @@ static int hist_trigger_enable(struct event_trigger_data *data,update_cond_flag (file); -if (trace_event_trigger_enable_disable (file,1 ) <0 ) { -list_del_rcu (&data->list); -update_cond_flag (file); +/* +* On failure the caller undoes the registration, and +* hist_unregister_trigger() can only find the trigger here. +*/ +if (trace_event_trigger_enable_disable (file,1 ) <0 ) ret--; - }return ret; }
@@ -6694,13 +6695,13 @@ static void hist_unregister_trigger(char *glob, } } -if (test && test->cmd_ops->free) - test->cmd_ops->free (test); -if (hist_data->enable_timestamps) {if (!hist_data->remove || test)tracing_set_filter_buffering (file->tr, false); } + +if (test && test->cmd_ops->free) + test->cmd_ops->free (test); }static bool hist_file_check_refs (struct trace_event_file *file)
@@ -6905,6 +6906,8 @@ static int event_hist_trigger_parse(struct event_command *cmd_ops,return ret; out_unreg:event_trigger_unregister (cmd_ops, file, glob+1 , trigger_data); +/* The unregister frees trigger_data, skip out_free */ + goto out; out_free:remove_hist_vars (hist_data);
Signature · 1 line
2.53.0