FAILED: patch "[PATCH] tracing: Set the trace clock before registering the histogram" failed to apply to 6.12-stable tree
Subsystems (2)
[email protected]
maintainerThe patch below does not apply to the 6.12-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.12.y
git checkout FETCH_HEAD
git cherry-pick -x 6ede78d0563a2a3ae3e46f9c07cedb5d79645429
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2026091625-wrongful-sensuous-63b5@gregkh' --subject-prefix 'PATCH 6.12.y' 'HEAD^..'
Possible dependencies:
thanks,
greg k-h
------------------ original commit in Linus's tree ------------------
From 6ede78d0563a2a3ae3e46f9c07cedb5d79645429 Mon Sep 17 00:00:00 2001
From: Donggeun Yoo <[email protected]>
Date: Mon, 7 Sep 2026 18:14:15 +0900
Subject: [PATCH] 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.
kernel/trace/trace_events_hist.c +9 -6
@@ -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);
Sasha Levin
maintainerPATCH 1/4 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: 6ede78d0563a ("tracing: Set the trace clock before registering the histogram trigger")
--- 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
@@ -1698,7 +1698,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;
@@ -1941,7 +1941,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
@@ -6326,7 +6326,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,
@@ -6356,15 +6356,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; }
@@ -6961,38 +6961,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
@@ -849,7 +849,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);
@@ -1391,38 +1391,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 )
@@ -1515,21 +1515,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;
@@ -1610,21 +1610,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;
@@ -1735,28 +1735,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,
@@ -1940,10 +1940,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/4 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: 6ede78d0563a ("tracing: Set the trace clock before registering the histogram trigger")
--- 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
@@ -1692,6 +1692,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 {
@@ -1723,6 +1724,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);
@@ -1810,6 +1815,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
@@ -1837,6 +1847,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,
@@ -1863,6 +1877,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
@@ -1914,17 +1931,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,
@@ -1941,7 +1952,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
@@ -508,21 +508,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 3/4 tracing: Merge struct event_trigger_ops into struct event_command
In reply to Sasha LevinFrom: 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: 6ede78d0563a ("tracing: Set the trace clock before registering the histogram trigger")
--- 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
@@ -1699,7 +1699,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;
@@ -1790,73 +1789,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 *
@@ -1877,9 +1809,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
@@ -1909,7 +1838,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
@@ -1918,7 +1847,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). *
@@ -1931,11 +1860,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,
@@ -1952,6 +1916,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); };/**
@@ -1972,7 +1948,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
@@ -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,
@@ -512,12 +505,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 *
@@ -537,7 +533,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 4/4 tracing: Set the trace clock before registering the histogram trigger
In reply to Sasha LevinFrom: Donggeun Yoo <[email protected]>
[ Upstream commit 6ede78d0563a2a3ae3e46f9c07cedb5d79645429 ]
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.
--- kernel/trace/trace_events_hist.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-)
kernel/trace/trace_events_hist.c +9 -6
@@ -6569,12 +6569,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;
@@ -6587,6 +6581,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);
Signature · 1 line
2.53.0