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

David Carlier <[email protected]>
Newsgroups org.kernel.vger.linux-trace-kernel,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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 <[email protected]>
Fixes: 61d445af0a7c ("tracing: Add bulk garbage collection of freeing event_trigger_data")
Signed-off-by: David Carlier <[email protected]>
---
 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.