FAILED: patch "[PATCH] tracing: Undo the registration when enabling the histogram" failed to apply to 6.18-stable tree
Subsystems (2)
[email protected]
maintainerThe patch below does not apply to the 6.18-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.18.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 '2026091657-justly-crank-ffed@gregkh' --subject-prefix 'PATCH 6.18.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: 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-dep-of: 92383cef6679 ("tracing: Undo the registration when enabling the histogram trigger fails")
--- kernel/trace/trace.h | 26 ++- kernel/trace/trace_eprobe.c | 8 +- kernel/trace/trace_events_hist.c | 60 +------ kernel/trace/trace_events_trigger.c | 257 ++++++++++------------------ 4 files changed, 116 insertions(+), 235 deletions(-)
kernel/trace/trace.h +18 -8
@@ -1757,6 +1757,7 @@ 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 {
@@ -1788,6 +1789,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);
@@ -1875,6 +1880,11 @@ extern void event_file_put(struct trace_event_file *file);* 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
@@ -1902,6 +1912,10 @@ struct event_trigger_ops {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,
@@ -1928,6 +1942,9 @@ struct event_trigger_ops {* @name: The unique name that identifies the event command. This is * the name used when setting triggers via trigger files. * +* @trigger_ops: The event_trigger_ops implementation associated with +* the command. +* * @trigger_type: A unique id that identifies the event command * 'type'. This value has two purposes, the first to ensure that * only one trigger of the same type can be set at a given time
@@ -1979,17 +1996,11 @@ struct event_trigger_ops {* event command, filters set by the user for the command will be * 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. */ struct event_command {struct list_head list;char *name; +const struct event_trigger_ops *trigger_ops;enum event_trigger_type trigger_type;int flags; int (*parse)(struct event_command *cmd_ops,
@@ -2006,7 +2017,6 @@ 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); };/**
kernel/trace/trace_eprobe.c +1 -7
@@ -514,21 +514,15 @@ 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 , .flags =EVENT_CMD_FL_NEEDS_REC , + .trigger_ops = &eprobe_trigger_ops, .parse = eprobe_trigger_cmd_parse, .reg = eprobe_trigger_reg_func, .unreg = eprobe_trigger_unreg_func, .unreg_all =NULL , - .get_trigger_ops = eprobe_trigger_get_ops, .set_filter =NULL , };
kernel/trace/trace_events_hist.c +5 -55
115 lines not on this page · raw message
kernel/trace/trace_events_trigger.c +92 -165
476 lines not on this page · raw message
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.
Stable-dep-of: 92383cef6679 ("tracing: Undo the registration when enabling the histogram trigger fails")
--- kernel/trace/trace.h | 126 +++++++++++----------------- kernel/trace/trace_eprobe.c | 13 +-- kernel/trace/trace_events_hist.c | 93 ++++++++++---------- kernel/trace/trace_events_trigger.c | 121 +++++++++++--------------- 4 files changed, 151 insertions(+), 202 deletions(-)
kernel/trace/trace.h +51 -75
@@ -1764,7 +1764,6 @@ 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;
@@ -1855,73 +1854,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. -* -* @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_trigger_ops { -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); -}; -/** * struct event_command - callbacks and data members for event commands *
@@ -1942,9 +1874,6 @@ struct event_trigger_ops {* @name: The unique name that identifies the event command. This is * the name used when setting triggers via trigger files. * -* @trigger_ops: The event_trigger_ops implementation associated with -* the command. -* * @trigger_type: A unique id that identifies the event command * 'type'. This value has two purposes, the first to ensure that * only one trigger of the same type can be set at a given time
@@ -1974,7 +1903,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
@@ -1983,7 +1912,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). *
@@ -1996,11 +1925,46 @@ struct event_trigger_ops {* event command, filters set by the user for the command will be * ignored. This is usually implemented by the generic utility * function @set_trigger_filter() (see trace_event_triggers.c). +* +* 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;char *name; -const struct event_trigger_ops *trigger_ops;enum event_trigger_type trigger_type;int flags; int (*parse)(struct event_command *cmd_ops,
@@ -2017,6 +1981,18 @@ struct event_command {int (*set_filter)(char *filter_str,struct event_trigger_data *data,struct trace_event_file *file); +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); };/**
@@ -2037,7 +2013,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 -9
@@ -485,13 +485,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,
@@ -518,12 +511,15 @@ static struct event_command event_trigger_cmd = { .name = "eprobe", .trigger_type = ETT_EVENT_EPROBE, .flags = EVENT_CMD_FL_NEEDS_REC, - .trigger_ops = &eprobe_trigger_ops, .parse = eprobe_trigger_cmd_parse, .reg = eprobe_trigger_reg_func, .unreg = eprobe_trigger_unreg_func, .unreg_all = NULL, .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 +539,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 +48 -45
194 lines not on this page · raw message
kernel/trace/trace_events_trigger.c +48 -73
268 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
@@ -6632,11 +6632,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; }
@@ -6714,13 +6715,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)
@@ -6925,6 +6926,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:event_trigger_reset_filter (cmd_ops, trigger_data);
Signature · 1 line
2.53.0