On Fri, Sep 04, 2026 at 07:52:23PM +0800, Henry Martin wrote:
> destroy_user_event() destroys the event's fields before attempting to
> remove the trace event call. If user_event_set_call_visible() fails,
> e.g. because the event is still enabled and trace_remove_event_call()
> returns -EBUSY, the event is left registered with an irreversibly
> destroyed field list. Any subsequent interaction with the event then
> operates on an empty field list while it is still fully visible in
> tracefs.
>
> Move the field destruction after the call removal, and splice the
> field list back onto the event when the removal fails so the event
> remains in a consistent state.
>
> Fixes: 7f5a08c79df35 ("user_events: Add minimal support for trace_event into
> ftrace")
> Signed-off-by: Henry Martin <[email protected]>
> ---
> v2:
> - Restore the comment on detaching the fields before removal: removing
> the event frees the field list memory, which is allocated and owned
> by user_events (Beau).
> - Comment why the fields are spliced back onto the event when removal
> fails: the event stays registered and the fields must be recovered
> (Beau).
>
This looks good to me.
Reviewed-by: Beau Belgrave <[email protected]>
Thanks,
-Beau
> kernel/trace/trace_events_user.c | 26 ++++++++++++++++++++-------
> 1 file changed, 20 insertions(+), 6 deletions(-)
>
> diff --git a/kernel/trace/trace_events_user.c
> b/kernel/trace/trace_events_user.c
> index 93cda2f6f2692..f658c3a77aa7a 100644
> --- a/kernel/trace/trace_events_user.c
> +++ b/kernel/trace/trace_events_user.c
> @@ -1122,10 +1122,9 @@ static void user_event_destroy_validators(struct
> user_event *user)
> }
> }
>
> -static void user_event_destroy_fields(struct user_event *user)
> +static void user_event_destroy_fields(struct list_head *head)
> {
> struct ftrace_event_field *field, *next;
> - struct list_head *head = &user->fields;
>
> list_for_each_entry_safe(field, next, head, link) {
> list_del(&field->link);
> @@ -1502,17 +1501,32 @@ static int user_event_set_call_visible(struct
> user_event *user, bool visible)
>
> static int destroy_user_event(struct user_event *user)
> {
> + LIST_HEAD(fields);
> int ret = 0;
>
> lockdep_assert_held(&event_mutex);
>
> - /* Must destroy fields before call removal */
> - user_event_destroy_fields(user);
> + /*
> + * Detach the fields before removing the call. Removing the event
> + * frees the field list memory (trace_destroy_fields() is run on
> + * successful removal and kmem_cache_free()s the fields), but the
> + * fields here are allocated and owned by user_events. Destroy
> + * them separately once removal has succeeded.
> + */
> + list_splice_init(&user->fields, &fields);
>
> ret = user_event_set_call_visible(user, false);
>
> - if (ret)
> + if (ret) {
> + /*
> + * Removal failed and the event stays registered, recover
> + * the fields so it is left in a consistent state.
> + */
> + list_splice(&fields, &user->fields);
> return ret;
> + }
> +
> + user_event_destroy_fields(&fields);
>
> dyn_event_remove(&user->devent);
> hash_del(&user->node);
> @@ -2212,7 +2226,7 @@ static int user_event_parse(struct user_event_group
> *group, char *name,
> put_user_lock:
> mutex_unlock(&event_mutex);
> put_user:
> - user_event_destroy_fields(user);
> + user_event_destroy_fields(&user->fields);
> user_event_destroy_validators(user);
> kfree(user->call.print_fmt);
>
> --
> 2.43.0
