kernel/trace/trace.h | 1 + kernel/trace/trace_events_hist.c | 19 +++++++++++++------ kernel/trace/trace_events_trigger.c | 18 +++++++++++++++--- 3 files changed, 29 insertions(+), 9 deletions(-)
Commit 61d445af0a7c ("tracing: Add bulk garbage collection of freeing
event_trigger_data") made trigger_data_free() defer the kfree() of the
event_trigger_data to a kthread that runs tracepoint_synchronize_unregister()
before freeing. The .free callbacks that own satellite data kept freeing it
synchronously right after calling trigger_data_free(), relying on the
synchronize that used to run inline.
With that synchronize now deferred, event_hist_trigger_free(),
event_enable_trigger_free() and event_hist_trigger_named_free() free
hist_data, enable_data and cmd_ops while a concurrent tracepoint handler can
still dereference them through the list_del_rcu()'d trigger, causing a
use-after-free.
Add an optional free_private() callback to event_trigger_data, invoked by
the free kthread after the grace period, and move the satellite frees into
it. The event_mutex-requiring bookkeeping (remove_hist_vars(),
unregister_field_var_hists()) stays synchronous; only the handler-visible
memory free is deferred.
Fixes: 61d445af0a7c ("tracing: Add bulk garbage collection of freeing event_trigger_data")
Signed-off-by: David Carlier <devnexen@gmail.com>
---
kernel/trace/trace.h | 1 +
kernel/trace/trace_events_hist.c | 19 +++++++++++++------
kernel/trace/trace_events_trigger.c | 18 +++++++++++++++---
3 files changed, 29 insertions(+), 9 deletions(-)
diff --git a/kernel/trace/trace.h b/kernel/trace/trace.h
index 80fe152af1dd..f04598337060 100644
--- a/kernel/trace/trace.h
+++ b/kernel/trace/trace.h
@@ -1941,6 +1941,7 @@ struct event_trigger_data {
struct list_head named_list;
struct event_trigger_data *named_data;
struct llist_node llist;
+ void (*free_private)(struct event_trigger_data *data);
};
/* Avoid typos */
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 82ce492ab268..bc696e4bd695 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -6335,6 +6335,16 @@ static void unregister_field_var_hists(struct hist_trigger_data *hist_data)
}
}
+static void hist_trigger_free_private(struct event_trigger_data *data)
+{
+ destroy_hist_data(data->private_data);
+}
+
+static void hist_trigger_named_free_private(struct event_trigger_data *data)
+{
+ kfree(data->cmd_ops);
+}
+
static void event_hist_trigger_free(struct event_trigger_data *data)
{
struct hist_trigger_data *hist_data = data->private_data;
@@ -6347,13 +6357,12 @@ static void event_hist_trigger_free(struct event_trigger_data *data)
if (data->name)
del_named_trigger(data);
- trigger_data_free(data);
-
remove_hist_vars(hist_data);
unregister_field_var_hists(hist_data);
- destroy_hist_data(hist_data);
+ data->free_private = hist_trigger_free_private;
+ trigger_data_free(data);
}
free_hist_pad();
}
@@ -6384,11 +6393,9 @@ static void event_hist_trigger_named_free(struct event_trigger_data *data)
data->ref--;
if (!data->ref) {
- struct event_command *cmd_ops = data->cmd_ops;
-
del_named_trigger(data);
+ data->free_private = hist_trigger_named_free_private;
trigger_data_free(data);
- kfree(cmd_ops);
}
}
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 655db2e82513..27c54da041b7 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -38,6 +38,13 @@ static void trigger_create_kthread_locked(void)
}
}
+static void trigger_data_free_one(struct event_trigger_data * data)
+{
+ if (data->free_private)
+ data->free_private(data);
+ kfree(data);
+}
+
static void trigger_data_free_queued_locked(void)
{
struct event_trigger_data *data, *tmp;
@@ -52,7 +59,7 @@ static void trigger_data_free_queued_locked(void)
tracepoint_synchronize_unregister();
llist_for_each_entry_safe(data, tmp, llnodes, llist)
- kfree(data);
+ trigger_data_free_one(data);
}
/* Bulk garbage collection of event_trigger_data elements */
@@ -75,7 +82,7 @@ static int trigger_kthread_fn(void *ignore)
tracepoint_synchronize_unregister();
llist_for_each_entry_safe(data, tmp, llnodes, llist)
- kfree(data);
+ trigger_data_free_one(data);
}
return 0;
@@ -1717,6 +1724,11 @@ int event_enable_trigger_print(struct seq_file *m,
return 0;
}
+static void enable_trigger_free_private(struct event_trigger_data *data)
+{
+ kfree(data->private_data);
+}
+
void event_enable_trigger_free(struct event_trigger_data *data)
{
struct enable_trigger_data *enable_data = data->private_data;
@@ -1729,8 +1741,8 @@ void event_enable_trigger_free(struct event_trigger_data *data)
/* Remove the SOFT_MODE flag */
trace_event_enable_disable(enable_data->file, 0, 1);
trace_event_put_ref(enable_data->file->event_call);
+ data->free_private = enable_trigger_free_private;
trigger_data_free(data);
- kfree(enable_data);
}
}
--
2.53.0On Sun, 12 Jul 2026 17:10:06 +0100
David Carlier <devnexen@gmail.com> wrote:
> Commit 61d445af0a7c ("tracing: Add bulk garbage collection of freeing
> event_trigger_data") made trigger_data_free() defer the kfree() of the
> event_trigger_data to a kthread that runs tracepoint_synchronize_unregister()
> before freeing. The .free callbacks that own satellite data kept freeing it
> synchronously right after calling trigger_data_free(), relying on the
> synchronize that used to run inline.
>
> With that synchronize now deferred, event_hist_trigger_free(),
> event_enable_trigger_free() and event_hist_trigger_named_free() free
> hist_data, enable_data and cmd_ops while a concurrent tracepoint handler can
> still dereference them through the list_del_rcu()'d trigger, causing a
> use-after-free.
>
> Add an optional free_private() callback to event_trigger_data, invoked by
> the free kthread after the grace period, and move the satellite frees into
> it. The event_mutex-requiring bookkeeping (remove_hist_vars(),
> unregister_field_var_hists()) stays synchronous; only the handler-visible
> memory free is deferred.
>
> Fixes: 61d445af0a7c ("tracing: Add bulk garbage collection of freeing event_trigger_data")
> Signed-off-by: David Carlier <devnexen@gmail.com>
OK, so this makes one of the self tests fail:
tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic-eprobe.tc
Which has at the end:
echo "-:$EPROBE" >> dynamic_events
echo '!'"hist:keys=common_pid:filename=\$__arg__1,ret=ret:onmatch($SYSTEM.$START).trace($SYNTH,\$filename,\$ret)" > events/$SYSTEM/$END/trigger
echo '!'"hist:keys=common_pid:__arg__1=$FIELD" > events/$SYSTEM/$START/trigger
echo '!'"$SYNTH u64 filename; s64 ret;" >> synthetic_events
The issue is that now the command that removes the onmatch() trigger has
its cleanup delayed, it returns before the synthetic event is actually
removed from the histogram. This allows the last command to execute before
it is removed and the removal of the synthetic event fails with -EBUSY
because the synthetic event is still attached to the histogram when it is
executed. The synthetic event *must* be removed from the histogram before
that operation returns.
Thus, I don't think adding a free_private() is appropriate. As you state in
the change log, the code that freed it was relying on the implicit
synchronize_rcu() from the trigger code. Now I think it just needs to call
it directly.
Hence, something like this:
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index 82ce492ab268..ddd2f70dac4f 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -6349,6 +6349,8 @@ static void event_hist_trigger_free(struct event_trigger_data *data)
trigger_data_free(data);
+ synchronize_rcu();
+
remove_hist_vars(hist_data);
unregister_field_var_hists(hist_data);
@@ -6388,6 +6390,7 @@ static void event_hist_trigger_named_free(struct event_trigger_data *data)
del_named_trigger(data);
trigger_data_free(data);
+ synchronize_rcu();
kfree(cmd_ops);
}
}
diff --git a/kernel/trace/trace_events_trigger.c b/kernel/trace/trace_events_trigger.c
index 655db2e82513..c3f54f2540b6 100644
--- a/kernel/trace/trace_events_trigger.c
+++ b/kernel/trace/trace_events_trigger.c
@@ -1730,6 +1730,7 @@ void event_enable_trigger_free(struct event_trigger_data *data)
trace_event_enable_disable(enable_data->file, 0, 1);
trace_event_put_ref(enable_data->file->event_call);
trigger_data_free(data);
+ synchronize_rcu();
kfree(enable_data);
}
}
The above makes the test pass again and I believe it fixes the problem you
found. Feel free to resend this change as v2.
-- Steve
On Sun, 12 Jul 2026 17:10:06 +0100
David Carlier <devnexen@gmail.com> wrote:
> diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
> index 82ce492ab268..bc696e4bd695 100644
> --- a/kernel/trace/trace_events_hist.c
> +++ b/kernel/trace/trace_events_hist.c
> @@ -6335,6 +6335,16 @@ static void unregister_field_var_hists(struct hist_trigger_data *hist_data)
> }
> }
>
> +static void hist_trigger_free_private(struct event_trigger_data *data)
> +{
> + destroy_hist_data(data->private_data);
> +}
> +
> +static void hist_trigger_named_free_private(struct event_trigger_data *data)
> +{
> + kfree(data->cmd_ops);
> +}
> +
This triggered lockdep:
[ 785.093618] ------------[ cut here ]------------
[ 785.097043] WARNING: kernel/trace/trace_events_hist.c:3597 at action_data_des
troy+0x74/0x80, CPU#3: trigger_data_fr/10557
[ 785.104157] Modules linked in: [last unloaded: trace_printk]
[ 785.108151] CPU: 3 UID: 0 PID: 10557 Comm: trigger_data_fr Tainted: G
W 7.2.0-rc4-ftest-00009-g22f7a9d07cb0 #174 PREEMPT(lazy)
[ 785.116393] Tainted: [W]=WARN
[ 785.118863] Hardware name: QEMU Standard PC (Q35 + ICH9, 2009), BIOS 1.17.0-debian-1.17.0-1 04/01/2014
[ 785.127417] RIP: 0010:action_data_destroy+0x74/0x80
[ 785.130947] Code: 8b bd 30 03 00 00 e8 bb f2 1d 00 48 89 ef 5b 5d e9 b1 f2 1d 00 be ff ff ff ff 48 c7 c7 60 75 7f 83 e8 d0 4d e9 00 85 c0 75 a0 <0f> 0b eb 9c 0f 1f 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90 90
[ 785.142648] RSP: 0018:ffffc90002803e78 EFLAGS: 00010246
[ 785.146271] RAX: 0000000000000000 RBX: ffff88811837e800 RCX: 0000000000000000
[ 785.150595] RDX: 0000000000000000 RSI: ffffffff82d24bb2 RDI: ffffffff82d5aacb
[ 785.156356] RBP: ffff88812bc8d400 R08: 0000000000000001 R09: 0000000000000000
[ 785.160229] R10: 0000000000000003 R11: ffff88811e290f60 R12: ffff88812bc8d400
[ 785.164090] R13: ffff88811e290000 R14: ffffffff815f1630 R15: 0000000000000000
[ 785.167949] FS: 0000000000000000(0000) GS:ffff8882f9727000(0000) knlGS:0000000000000000
[ 785.172381] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[ 785.175483] CR2: 00007f02ef65341c CR3: 000000000366a001 CR4: 0000000000172ef0
[ 785.179019] Call Trace:
[ 785.180523] <TASK>
[ 785.181881] destroy_hist_data+0x24f/0x260
[ 785.185686] trigger_kthread_fn+0x87/0xc0
[ 785.187762] ? __pfx_trigger_kthread_fn+0x10/0x10
[ 785.190099] kthread+0xf5/0x130
[ 785.191807] ? __pfx_kthread+0x10/0x10
[ 785.193752] ret_from_fork+0x336/0x470
[ 785.195716] ? __pfx_kthread+0x10/0x10
[ 785.197654] ret_from_fork_asm+0x1a/0x30
[ 785.199694] </TASK>
[ 785.201037] irq event stamp: 3815
[ 785.202762] hardirqs last enabled at (3827): [<ffffffff814be4ee>] __up_console_sem+0x5e/0x70
[ 785.206442] hardirqs last disabled at (3838): [<ffffffff814be4d3>] __up_console_sem+0x43/0x70
[ 785.209954] softirqs last enabled at (3476): [<ffffffff8141115d>] handle_softirqs+0x35d/0x430
[ 785.213524] softirqs last disabled at (3471): [<ffffffff81411346>] __irq_exit_rcu+0x106/0x1a0
[ 785.218790] ---[ end trace 0000000000000000 ]---
Can you fold this into your patch:
diff --git a/kernel/trace/trace_events_hist.c b/kernel/trace/trace_events_hist.c
index bc696e4bd695..1f438de90d09 100644
--- a/kernel/trace/trace_events_hist.c
+++ b/kernel/trace/trace_events_hist.c
@@ -6337,6 +6337,7 @@ static void unregister_field_var_hists(struct hist_trigger_data *hist_data)
static void hist_trigger_free_private(struct event_trigger_data *data)
{
+ guard(mutex)(&event_mutex);
destroy_hist_data(data->private_data);
}
-- Steve
© 2016 - 2026 Red Hat, Inc.