Re: [PATCH] tracing/user_events: Don't destroy fields when event removal fails

[Beau Belgrave <beaub@linux.microsoft.com>]

On Mon, Aug 24, 2026 at 04:39:55PM -0400, Steven Rostedt wrote:
> 
> Beau,
> 
> Can you review this patch?
> 

Sure thing.

> Thanks,
> 
> -- Steve
> 
> 
> On Mon, 24 Aug 2026 18:29:07 +0800
> Henry Martin <bsdhenrymartin@gmail.com> wrote:
> 

Hey Henry, thanks for this!

> > 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 <bsdhenrymartin@gmail.com>
> > ---
> >  kernel/trace/trace_events_user.c | 16 ++++++++++------
> >  1 file changed, 10 insertions(+), 6 deletions(-)
> > 
> > diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
> > index 8c82ecb735f41..0b021c212995a 100644
> > --- a/kernel/trace/trace_events_user.c
> > +++ b/kernel/trace/trace_events_user.c
> > @@ -1119,10 +1119,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);
> > @@ -1499,17 +1498,21 @@ 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 */

We really need a comment to stick around, it's not going to be obvious
why we have to remove the fields before destroy (It's because removing
the event frees the list memory, which are owned by user_events).

> > -	user_event_destroy_fields(user);
> > +	list_splice_init(&user->fields, &fields);
> >  
> >  	ret = user_event_set_call_visible(user, false);
> >  
> > -	if (ret)
> > +	if (ret) {

Would be good to get a comment on here stating why we are putting the
fields back. We have to recover them.

> > +		list_splice(&fields, &user->fields);
> >  		return ret;
> > +	}
> > +
> > +	user_event_destroy_fields(&fields);
> >  
> >  	dyn_event_remove(&user->devent);
> >  	hash_del(&user->node);
> > @@ -2209,7 +2212,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);
> >  

Thanks,
-Beau