On Sun, 12 Jul 2026 17:10:06 +0100
David Carlier <[email protected]> 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 <[email protected]>
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
