Re: [PATCH v2] tracing/user_events: Clear copied tracing state before fork duplication

[Bradley Morgan <brads@mainlining.org>]

On 26 August 2026 22:44:15 BST, "Jérémy Jean"
<Jeremy.Jean@oss.cyber.gouv.fr> wrote:
>
>dup_task_struct() copies user_event_mm from the parent into the child,
>without grabbing a reference to it. user_event_mm_dup() should
>replace it, but it leaves that copied pointer unmodified if
>user_event_mm_alloc() fails.
>
>When the child exits, user_event_mm_remove() decrements a reference
>the child never owned, which ultimately frees user_event_mm, while
>the parent still as a stale pointer to it. This creates a UAF, which
>KASAN reports as:
>
>    BUG: KASAN: slab-use-after-free in
>    current_user_event_mm+0x51/0x1d0 Write of size 4 at addr
>    ffff888005010d30 by task init/44
>
>    Call Trace:
>     <TASK>
>     kasan_report+0xce/0x100
>     kasan_check_range+0x10f/0x1e0
>     current_user_event_mm+0x51/0x1d0
>     user_events_ioctl+0x82e/0x15c0
>     __x64_sys_ioctl+0x139/0x1c0
>     do_syscall_64+0xce/0x450
>     entry_SYSCALL_64_after_hwframe+0x77/0x7f
>
>    Allocated by task 44:
>     __kasan_kmalloc+0x8f/0xa0
>     __kmalloc_cache_noprof+0x180/0x3a0
>     user_event_mm_alloc+0x3c/0x1f0
>     current_user_event_mm+0x88/0x1d0
>
>    Freed by task 42:
>     __kasan_slab_free+0x43/0x70
>     kfree+0x13a/0x390
>     process_one_work+0x696/0xf90
>     worker_thread+0x420/0xba0
>
>The fix simply clears the copied pointer before starting the
>duplication, before any possible failure. In case of failure,
>the child then has nothing to free.
>
>Fixes: 7235759084a4 ("tracing/user_events: Use remote writes for event enablement")
>Cc: stable@vger.kernel.org
>Assisted-by: Codex:gpt-5
>Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
>---
>
>Change in v2:
> Move the pointer reset into user_event_mm_dup(), before the first
> allocation (suggestion by Steven Rostedt).
>
> kernel/trace/trace_events_user.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
>diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
>index 2bbc89d4a266..339e18085af3 100644
>--- a/kernel/trace/trace_events_user.c
>+++ b/kernel/trace/trace_events_user.c
>@@ -865,9 +865,12 @@ void user_event_mm_remove(struct task_struct *t)
> 
> void user_event_mm_dup(struct task_struct *t, struct user_event_mm
> *old_mm)
> {
>-	struct user_event_mm *mm = user_event_mm_alloc(t);
>+	struct user_event_mm *mm;
> 	struct user_event_enabler *enabler;
> 

Comment?

/* Failure must leave the child with no copied state to free. */

I mean, okay, you don't got to, but itd be nice, if your happy with it, add

Reviewed-by: Bradley Morgan <brads@mainlining.org>


Also, let me talk about one of Steve's nits a bit, yk, the one where he
says the description is too long

Maybe you could add this to your memories


"The description length should be about the same as the change being
added,unless there is a splat, or something else like a table which needs
to be added to the description, keep the description length the same as
thepatch size, e.g:

Instead of doing 3 paragraths about a one liner, we could do a small two
or more line description describing:

What causes the issue?
Why is it bad?
How did you fix it?"

That should work for you. Just to prevent annoying other maintainers 


>+	t->user_event_mm = NULL;
>+	mm = user_event_mm_alloc(t);
>+
> 	if (!mm)
> 		return;
> 
>


--- Thanks!
https://lore.kernel.org/all/EE579805-42F2-4C58-B752-F28779EEB717@grrlz.net/