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

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

[Henry Martin]

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 */
-	user_event_destroy_fields(user);
+	list_splice_init(&user->fields, &fields);
 
 	ret = user_event_set_call_visible(user, false);
 
-	if (ret)
+	if (ret) {
+		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);
 
-- 
2.43.0

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

[Steven Rostedt]

Beau,

Can you review this patch?

Thanks,

-- Steve


On Mon, 24 Aug 2026 18:29:07 +0800
Henry Martin <bsdhenrymartin@gmail.com> 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 <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 */
> -	user_event_destroy_fields(user);
> +	list_splice_init(&user->fields, &fields);
>  
>  	ret = user_event_set_call_visible(user, false);
>  
> -	if (ret)
> +	if (ret) {
> +		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);
>  

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

[Beau Belgrave]

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

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

[Henry Martin]

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

 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

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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Data race leading to out-of-bounds read and BPF context size check bypass when event removal races with BPF attach.
--

commit 67af0e963857d01d3ef2cc1bd224b3d373fe0483
Author: Henry Martin <bsdhenrymartin@gmail.com>

tracing/user_events: Don't destroy fields when event removal fails

destroy_user_event() previously destroyed fields before attempting to
remove the trace event call. This patch moves the field destruction after 
the call removal and temporarily moves the fields to a local list to prevent
them from being irreversibly destroyed if the removal fails.

> diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
> index 2bbc89d4a2666..dbd8e9f95a4a7 100644
> --- a/kernel/trace/trace_events_user.c
> +++ b/kernel/trace/trace_events_user.c
[ ... ]
> @@ -1499,17 +1498,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);

[Severity: High]
This is a pre-existing issue, but both the old behavior and this new 
list_splice_init() leave the published field list temporarily empty while 
the event is still active. Does this create a data race with concurrent 
BPF attaches?

A concurrent PERF_EVENT_IOC_SET_BPF can call trace_event_get_offsets(), 
which locklessly accesses this list without holding event_mutex or 
trace_event_sem:

    tail = list_first_entry(head, struct ftrace_event_field, link);
    return tail->offset + tail->size;

Because the list is empty at this point, list_first_entry() resolves to the
list head itself (&user->fields). This causes tail->offset and tail->size
to read garbage values from adjacent struct members (such as
user->put_work).

Could this out-of-bounds read result in the BPF verifier bypassing context
size checks, potentially allowing a kernel memory information leak?

>  
>  	ret = user_event_set_call_visible(user, false);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260904115223.2976446-1-bsdhenrymartin@gmail.com?part=1

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

[Beau Belgrave]

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 <bsdhenrymartin@gmail.com>
> ---
> 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 <beaub@linux.microsoft.com>

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