FAILED: patch "[PATCH] tracing: Undo the registration when enabling the histogram" failed to apply to 6.1-stable tree
Subsystems (2)
[email protected]
maintainerThe patch below does not apply to the 6.1-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.1.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 '2026091604-putdown-resort-2cec@gregkh' --subject-prefix 'PATCH 6.1.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
@@ -1557,7 +1557,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;
@@ -1800,7 +1800,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
@@ -574,7 +574,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,
@@ -603,8 +603,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
@@ -6020,7 +6020,7 @@ static void event_hist_trigger_free(struct event_trigger_data *data) } } -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,
@@ -6052,15 +6052,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; }
@@ -6657,38 +6657,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
@@ -831,7 +831,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);
@@ -1360,38 +1360,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 )
@@ -1473,21 +1473,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;
@@ -1568,21 +1568,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;
@@ -1693,28 +1693,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,
@@ -1898,10 +1898,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: Remove get_trigger_ops() and add count_func() from trigger ops
From: Steven Rostedt <[email protected]>
[ Upstream commit bdafb4d4cb3bb18b29517eaae09fb49d25f854f0 ]
The struct event_command has a callback function called get_trigger_ops(). This callback returns the "trigger_ops" to use for the trigger. These ops define the trigger function, how to init the trigger, how to print the trigger and how to free it.
The only reason there's a callback function to get these ops is because some triggers have two types of operations. One is an "always on" operation, and the other is a "count down" operation. If a user passes in a parameter to say how many times the trigger should execute. For example:
echo stacktrace:5 > events/kmem/kmem_cache_alloc/trigger
It will trigger the stacktrace for the first 5 times the kmem_cache_alloc event is hit.
Instead of having two different trigger_ops since the only difference between them is the tigger itself (the print, init and free functions are all the same), just use a single ops that the event_command points to and add a function field to the trigger_ops to have a count_func.
When a trigger is added to an event, if there's a count attached to it and the trigger ops has the count_func field, the data allocated to represent this trigger will have a new flag set called COUNT.
Then when the trigger executes, it will check if the COUNT data flag is set, and if so, it will call the ops count_func(). If that returns false, it returns without executing the trigger.
This removes the need for duplicate event_trigger_ops structures.
Stable adaptation for 92383cef66791a0c63a2f27755cadbdb2fbf270b:
Keep the stable snapshot trigger's unregister_trigger callback; this tree has no snapshot reference counting or unregister_snapshot_trigger helper. Retain the stable explicit locking and existing trigger init semantics.
Fold in the callback migration from b052d70f7c9c so the target can use cmd_ops->free directly, including the distinct lifetime of copied command ops for shared named histograms. The stable histogram initializer cannot fail, so its upstream hist_pad allocation error handling is unnecessary. Select the histogram clock before allocating copied command ops or publishing a named trigger, and enable filter buffering after init, so a clock selection failure cannot strand those resources.
Also drop the redundant event_trigger_reset_filter call as ina4e0ea0e10a26: trigger_data_free already performs that cleanup. This provides the target's out_free context without changing its patch.
No functions beyond those already introduced by bdafb4d4cb3b are added. The target fix itself is left unapplied.
Stable-dep-of: 92383cef6679 ("tracing: Undo the registration when enabling the histogram trigger fails")
--- kernel/trace/trace.h | 124 +++++----- kernel/trace/trace_eprobe.c | 19 +- kernel/trace/trace_events_hist.c | 156 ++++--------- kernel/trace/trace_events_trigger.c | 344 ++++++++++------------------ 4 files changed, 232 insertions(+), 411 deletions(-)
kernel/trace/trace.h +55 -69
@@ -1551,13 +1551,13 @@ extern void clear_event_triggers(struct trace_array *tr);enum {EVENT_TRIGGER_FL_PROBE =BIT (0 ), +EVENT_TRIGGER_FL_COUNT =BIT (1 ), };struct event_trigger_data {unsigned long count;int ref;int flags; -const struct event_trigger_ops *ops;struct event_command *cmd_ops;struct event_filter __rcu *filter;char *filter_str;
@@ -1582,6 +1582,10 @@ struct enable_trigger_data {bool hist; }; +bool event_trigger_count (struct event_trigger_data *data, +struct trace_buffer *buffer,void *rec, +struct ring_buffer_event *event); +extern int event_enable_trigger_print (struct seq_file *m,struct event_trigger_data *data);extern void event_enable_trigger_free (struct event_trigger_data *data);
@@ -1644,64 +1648,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 *
@@ -1751,7 +1697,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
@@ -1760,7 +1706,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). *
@@ -1774,12 +1720,41 @@ 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. +* 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. +* +* @count_func: If defined and a numeric parameter is passed to the +* trigger, then this function will be called before @trigger +* is called. If this function returns false, then @trigger is not +* executed. +* +* @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;
@@ -1800,7 +1775,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); +bool (*count_func)(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); };/**
@@ -1821,7 +1807,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 +4 -15
@@ -574,13 +574,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,
@@ -603,12 +596,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 ,
@@ -617,8 +604,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 *
@@ -638,7 +628,6 @@ 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;/* * EVENT PROBE triggers are not registered as commands with
kernel/trace/trace_events_hist.c +50 -106
251 lines not on this page · raw message
kernel/trace/trace_events_trigger.c +123 -221
577 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
@@ -6293,11 +6293,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; }
@@ -6375,13 +6376,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)
@@ -6586,6 +6587,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