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]>
---
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
