mirror of
https://github.com/torvalds/linux.git
synced 2026-07-27 17:47:41 +02:00
tracing: Fix use-after-free freeing trigger private data
Commit61d445af0a("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 synchronization 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 synchronization 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 histogram teardown must stay synchronous: remove_hist_vars() and unregister_field_var_hists() have to detach a synthetic event from the histogram before the trigger-removal write returns, otherwise a following command races in and the synthetic-event removal fails with -EBUSY, as the trigger-synthetic-eprobe.tc selftest catches. Make those callbacks wait with the correct barrier - tracepoint_synchronize_unregister(), matching the free kthread - before freeing. The enable trigger has no such synchronous requirement, and a blocking synchronize there would re-serialize the path that commit deliberately deferred. Give it an optional private_data_free() callback that the free kthread runs after its grace period, and free enable_data from there. Link: https://patch.msgid.link/20260724030523.19081-1-devnexen@gmail.com Suggested-by: Masami Hiramatsu (Google) <mhiramat@kernel.org> Suggested-by: Steven Rostedt <rostedt@goodmis.org> Fixes:61d445af0a("tracing: Add bulk garbage collection of freeing event_trigger_data") Signed-off-by: David Carlier <devnexen@gmail.com> Signed-off-by: Steven Rostedt <rostedt@goodmis.org>
This commit is contained in:
parent
8f76afb9b1
commit
7909781215
|
|
@ -1941,6 +1941,7 @@ struct event_trigger_data {
|
|||
struct list_head named_list;
|
||||
struct event_trigger_data *named_data;
|
||||
struct llist_node llist;
|
||||
void (*private_data_free)(struct event_trigger_data *data);
|
||||
};
|
||||
|
||||
/* Avoid typos */
|
||||
|
|
|
|||
|
|
@ -6349,6 +6349,7 @@ static void event_hist_trigger_free(struct event_trigger_data *data)
|
|||
|
||||
trigger_data_free(data);
|
||||
|
||||
tracepoint_synchronize_unregister();
|
||||
remove_hist_vars(hist_data);
|
||||
|
||||
unregister_field_var_hists(hist_data);
|
||||
|
|
@ -6388,6 +6389,7 @@ static void event_hist_trigger_named_free(struct event_trigger_data *data)
|
|||
|
||||
del_named_trigger(data);
|
||||
trigger_data_free(data);
|
||||
tracepoint_synchronize_unregister();
|
||||
kfree(cmd_ops);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -38,6 +38,13 @@ static void trigger_create_kthread_locked(void)
|
|||
}
|
||||
}
|
||||
|
||||
static void trigger_data_free_one(struct event_trigger_data *data)
|
||||
{
|
||||
if (data->private_data_free)
|
||||
data->private_data_free(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_private_data_free(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;
|
||||
|
|
@ -1728,9 +1740,9 @@ void event_enable_trigger_free(struct event_trigger_data *data)
|
|||
if (!data->ref) {
|
||||
/* Remove the SOFT_MODE flag */
|
||||
trace_event_enable_disable(enable_data->file, 0, 1);
|
||||
data->private_data_free = enable_trigger_private_data_free;
|
||||
trace_event_put_ref(enable_data->file->event_call);
|
||||
trigger_data_free(data);
|
||||
kfree(enable_data);
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user