Re: [PATCH] tracing: Defer trigger private data frees past the grace period

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

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