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

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

[David Carlier]

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.0