Beau, Can you review this?
Thanks, -- Steve On Fri, 2 Oct 2026 18:26:39 -0400 Jeff Barnes <[email protected]> wrote: > Registered user events are retained across fork, but duplicating their > state for a child with a separate mm can fail. Both user_event_mm_dup() and > user_events_fork() currently return void, so allocation failure silently > creates a child without the inherited registration state. > > The child can consequently retain a stale copy-on-write enable word and > miss later event enable and disable updates. > > Return an error from user_event_mm_dup() and user_events_fork(), and > perform the duplication in copy_process() while failure can still be > unwound. Return -ENOMEM when the child user_event_mm or any of its enablers > cannot be duplicated. > > Add a cleanup path so successfully acquired user-events state is removed if > a later fork operation fails. Preserve the existing CLONE_VM behavior and > its task reference accounting. > > A deterministic allocation-failure test on upstream master previously > allowed fork() to succeed while the child missed an enablement update. With > this change, the same fork fails with ENOMEM. The complete user_events ABI > suite passes. > > Fixes: 7235759084a4 ("tracing/user_events: Use remote writes for event > enablement") > Cc: [email protected] > Signed-off-by: Jeff Barnes <[email protected]> > --- > include/linux/user_events.h | 16 +++++++--------- > kernel/fork.c | 8 ++++++-- > kernel/trace/trace_events_user.c | 8 +++++--- > 3 files changed, 18 insertions(+), 14 deletions(-) > > diff --git a/include/linux/user_events.h b/include/linux/user_events.h > index 57d1ff006090..75f184126727 100644 > --- a/include/linux/user_events.h > +++ b/include/linux/user_events.h > @@ -27,28 +27,26 @@ struct user_event_mm { > struct rcu_work put_rwork; > }; > > -extern void user_event_mm_dup(struct task_struct *t, > - struct user_event_mm *old_mm); > +int user_event_mm_dup(struct task_struct *t, struct user_event_mm *old_mm); > > extern void user_event_mm_remove(struct task_struct *t); > > -static inline void user_events_fork(struct task_struct *t, > - u64 clone_flags) > +static inline int user_events_fork(struct task_struct *t, u64 clone_flags) > { > struct user_event_mm *old_mm; > > if (!t || !current->user_event_mm) > - return; > + return 0; > > old_mm = current->user_event_mm; > > if (clone_flags & CLONE_VM) { > t->user_event_mm = old_mm; > refcount_inc(&old_mm->tasks); > - return; > + return 0; > } > > - user_event_mm_dup(t, old_mm); > + return user_event_mm_dup(t, old_mm); > } > > static inline void user_events_execve(struct task_struct *t) > @@ -67,9 +65,9 @@ static inline void user_events_exit(struct task_struct *t) > user_event_mm_remove(t); > } > #else > -static inline void user_events_fork(struct task_struct *t, > - u64 clone_flags) > +static inline int user_events_fork(struct task_struct *t, u64 clone_flags) > { > + return 0; > } > > static inline void user_events_execve(struct task_struct *t) > diff --git a/kernel/fork.c b/kernel/fork.c > index 10f2d05d816a..9e3da2e6059f 100644 > --- a/kernel/fork.c > +++ b/kernel/fork.c > @@ -2311,9 +2311,12 @@ __latent_entropy struct task_struct *copy_process( > retval = copy_mm(clone_flags, p); > if (retval) > goto bad_fork_cleanup_signal; > - retval = copy_namespaces(clone_flags, p); > + retval = user_events_fork(p, clone_flags); > if (retval) > goto bad_fork_cleanup_mm; > + retval = copy_namespaces(clone_flags, p); > + if (retval) > + goto bad_fork_cleanup_user_events; > retval = copy_io(clone_flags, p); > if (retval) > goto bad_fork_cleanup_namespaces; > @@ -2575,7 +2578,6 @@ __latent_entropy struct task_struct *copy_process( > > trace_task_newtask(p, clone_flags); > uprobe_copy_process(p, clone_flags); > - user_events_fork(p, clone_flags); > > copy_oom_score_adj(clone_flags, p); > > @@ -2602,6 +2604,8 @@ __latent_entropy struct task_struct *copy_process( > exit_io_context(p); > bad_fork_cleanup_namespaces: > exit_nsproxy_namespaces(p); > +bad_fork_cleanup_user_events: > + user_events_exit(p); > bad_fork_cleanup_mm: > sched_cache_fork_cleanup(p); > if (p->mm) { > diff --git a/kernel/trace/trace_events_user.c > b/kernel/trace/trace_events_user.c > index f658c3a77aa7..8941c8d7c193 100644 > --- a/kernel/trace/trace_events_user.c > +++ b/kernel/trace/trace_events_user.c > @@ -863,7 +863,7 @@ void user_event_mm_remove(struct task_struct *t) > queue_rcu_work(system_percpu_wq, &mm->put_rwork); > } > > -void user_event_mm_dup(struct task_struct *t, struct user_event_mm *old_mm) > +int 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_enabler *enabler; > @@ -872,7 +872,7 @@ void user_event_mm_dup(struct task_struct *t, struct > user_event_mm *old_mm) > t->user_event_mm = NULL; > > if (!mm) > - return; > + return -ENOMEM; > > rcu_read_lock(); > > @@ -884,10 +884,12 @@ void user_event_mm_dup(struct task_struct *t, struct > user_event_mm *old_mm) > rcu_read_unlock(); > > user_event_mm_attach(mm, t); > - return; > + return 0; > error: > rcu_read_unlock(); > user_event_mm_destroy(mm); > + > + return -ENOMEM; > } > > static bool current_user_event_enabler_exists(unsigned long uaddr,
