[PATCH v2] tracing: Restore synchronize_rcu() before freeing trigger private data

[PATCH v2] tracing: Restore synchronize_rcu() before freeing trigger private data

[David Carlier]

Commit 61d445af0a7c ("tracing: Add bulk garbage collection of freeing
event_trigger_data") moved the kfree() of event_trigger_data to a kthread
that runs tracepoint_synchronize_unregister() before freeing. That removed
the synchronize_rcu() the trigger .free callbacks used to get implicitly and
inline from trigger_data_free().

event_hist_trigger_free(), event_hist_trigger_named_free() and
event_enable_trigger_free() free their satellite data (hist_data, cmd_ops,
enable_data) right after trigger_data_free() returns. With the synchronize
now deferred to the kthread, a concurrent tracepoint handler can still reach
that data through the list_del_rcu()'d trigger, causing a use-after-free.

The teardown must also stay synchronous for correctness: the histogram
cleanup (remove_hist_vars(), unregister_field_var_hists()) has to detach a
synthetic event from the histogram before the trigger-removal write returns.
Deferring it lets a following command race in and fail the synthetic-event
removal with -EBUSY, as caught by
tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic-eprobe.tc.

Call synchronize_rcu() directly before freeing the satellite data instead.

Suggested-by: Steven Rostedt <rostedt@goodmis.org>
Fixes: 61d445af0a7c ("tracing: Add bulk garbage collection of freeing event_trigger_data")
Signed-off-by: David Carlier <devnexen@gmail.com>
---
 kernel/trace/trace_events_hist.c    | 3 +++
 kernel/trace/trace_events_trigger.c | 1 +
 2 files changed, 4 insertions(+)

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);
 	}
 }
-- 
2.53.0

Re: [PATCH v2] tracing: Restore synchronize_rcu() before freeing trigger private data

[Steven Rostedt]

On Thu, 23 Jul 2026 18:02:12 +0100
David Carlier <devnexen@gmail.com> wrote:

> ---
>  kernel/trace/trace_events_hist.c    | 3 +++
>  kernel/trace/trace_events_trigger.c | 1 +
>  2 files changed, 4 insertions(+)
> 
> 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();

According to Sashiko[1], it appears we actually need tracepoint_synchronize_unregister().
And I believe Sashiko is actually correct.

> +
>  		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();

This needs to be changed too.

>  		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();

According to Sashiko, this one basically causes a revert of the original
delay. 

OK, let's bring back a partial part of your original patch. Have the
"free_private()" again, but only apply it to this trigger, and switch the
above other to synchronize_rcu() into the tracepoint_synchronize_unregister().

Thanks,

-- Steve

>  		kfree(enable_data);
>  	}

[1] https://sashiko.dev/#/patchset/20260723170212.55325-1-devnexen%40gmail.com

Re: [PATCH v2] tracing: Restore synchronize_rcu() before freeing trigger private data

[Masami Hiramatsu]

On Thu, 23 Jul 2026 18:02:12 +0100
David Carlier <devnexen@gmail.com> wrote:

> Commit 61d445af0a7c ("tracing: Add bulk garbage collection of freeing
> event_trigger_data") moved the kfree() of event_trigger_data to a kthread
> that runs tracepoint_synchronize_unregister() before freeing. That removed
> the synchronize_rcu() the trigger .free callbacks used to get implicitly and
> inline from trigger_data_free().
> 
> event_hist_trigger_free(), event_hist_trigger_named_free() and
> event_enable_trigger_free() free their satellite data (hist_data, cmd_ops,
> enable_data) right after trigger_data_free() returns. With the synchronize
> now deferred to the kthread, a concurrent tracepoint handler can still reach
> that data through the list_del_rcu()'d trigger, causing a use-after-free.
> 
> The teardown must also stay synchronous for correctness: the histogram
> cleanup (remove_hist_vars(), unregister_field_var_hists()) has to detach a
> synthetic event from the histogram before the trigger-removal write returns.
> Deferring it lets a following command race in and fail the synthetic-event
> removal with -EBUSY, as caught by
> tools/testing/selftests/ftrace/test.d/trigger/inter-event/trigger-synthetic-eprobe.tc.
> 
> Call synchronize_rcu() directly before freeing the satellite data instead.
> 
> Suggested-by: Steven Rostedt <rostedt@goodmis.org>
> Fixes: 61d445af0a7c ("tracing: Add bulk garbage collection of freeing event_trigger_data")
> Signed-off-by: David Carlier <devnexen@gmail.com>

Hi David,

Instead of adding just a synchronize_rcu(), can you reconsider to introduce
.private_data_free() to event_trigger_data? I hit the same issue on wprobe
and fixed it with ".private_free()" (I think private_data_free() is better
naming)

https://patchwork.kernel.org/project/linux-trace-kernel/patch/178476144141.26117.8004746214529353062.stgit@devnote2/

Thank you,

> ---
>  kernel/trace/trace_events_hist.c    | 3 +++
>  kernel/trace/trace_events_trigger.c | 1 +
>  2 files changed, 4 insertions(+)
> 
> 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);
>  	}
>  }
> -- 
> 2.53.0
> 


-- 
Masami Hiramatsu (Google) <mhiramat@kernel.org>