* [PATCH] tracing/user_events: Fail fork when event state duplication fails
@ 2026-10-02 22:26 Jeff Barnes
2026-10-04 8:00 ` Steven Rostedt
0 siblings, 1 reply; 7+ messages in thread
From: Jeff Barnes @ 2026-10-02 22:26 UTC (permalink / raw)
To: linux-trace-kernel
Cc: mhiramat, rostedt, mathieu.desnoyers, linux-kernel, akpm, kees
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: stable@vger.kernel.org
Signed-off-by: Jeff Barnes <jeffbarnes@linux.microsoft.com>
---
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,
--
2.43.0
^ permalink raw reply [flat|nested] 7+ messages in thread* Re: [PATCH] tracing/user_events: Fail fork when event state duplication fails 2026-10-02 22:26 [PATCH] tracing/user_events: Fail fork when event state duplication fails Jeff Barnes @ 2026-10-04 8:00 ` Steven Rostedt 2026-10-05 21:56 ` Beau Belgrave 0 siblings, 1 reply; 7+ messages in thread From: Steven Rostedt @ 2026-10-04 8:00 UTC (permalink / raw) To: Beau Belgrave Cc: Jeff Barnes, linux-trace-kernel, mhiramat, mathieu.desnoyers, linux-kernel, akpm, kees Beau, Can you review this? Thanks, -- Steve On Fri, 2 Oct 2026 18:26:39 -0400 Jeff Barnes <jeffbarnes@linux.microsoft.com> 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: stable@vger.kernel.org > Signed-off-by: Jeff Barnes <jeffbarnes@linux.microsoft.com> > --- > 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, ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tracing/user_events: Fail fork when event state duplication fails 2026-10-04 8:00 ` Steven Rostedt @ 2026-10-05 21:56 ` Beau Belgrave 2026-10-06 12:01 ` Jeff Barnes 0 siblings, 1 reply; 7+ messages in thread From: Beau Belgrave @ 2026-10-05 21:56 UTC (permalink / raw) To: Steven Rostedt Cc: Jeff Barnes, linux-trace-kernel, mhiramat, mathieu.desnoyers, linux-kernel, akpm, kees On Sun, Oct 04, 2026 at 04:00:16AM -0400, Steven Rostedt wrote: > Beau, > > Can you review this? > Sure thing. > Thanks, > > -- Steve > > > On Fri, 2 Oct 2026 18:26:39 -0400 > Jeff Barnes <jeffbarnes@linux.microsoft.com> 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. > > This code makes fork fail if we cannot duplicate the user_event enablements, uprobes just warns in these cases. The changes introduced look correct if that's what we want (to fail fork() when enablements would have been left behind). If we do not want the fork to actually fail, a simple call to user_event_mm_remove() within user_event_mm_dup() would address the inconsistency. However, it would be silent. Some questions follow. Steve, is it fine to have fork() fail when we cannot copy state? Uprobe seems to just warn here. Jeff, I assume we have cases in mind where we really prefer fork() to fail when user_events cannot be propogated? (For sure we want the inconsistency fixed, just not sure about if silent is OK or not). Thanks, -Beau > > 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: stable@vger.kernel.org > > Signed-off-by: Jeff Barnes <jeffbarnes@linux.microsoft.com> > > --- > > 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, ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tracing/user_events: Fail fork when event state duplication fails 2026-10-05 21:56 ` Beau Belgrave @ 2026-10-06 12:01 ` Jeff Barnes 2026-10-06 12:38 ` Steven Rostedt 0 siblings, 1 reply; 7+ messages in thread From: Jeff Barnes @ 2026-10-06 12:01 UTC (permalink / raw) To: Beau Belgrave Cc: Steven Rostedt, linux-trace-kernel, mhiramat, mathieu.desnoyers, linux-kernel, akpm, kees On Oct 5 2026, at 5:56 pm, Beau Belgrave <beaub@linux.microsoft.com> wrote: > On Sun, Oct 04, 2026 at 04:00:16AM -0400, Steven Rostedt wrote: >> Beau, >> >> Can you review this? >> > > Sure thing. > >> Thanks, >> >> -- Steve >> >> >> On Fri, 2 Oct 2026 18:26:39 -0400 >> Jeff Barnes <jeffbarnes@linux.microsoft.com> 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. >> > > > This code makes fork fail if we cannot duplicate the user_event > enablements, uprobes just warns in these cases. The changes introduced > look correct if that's what we want (to fail fork() when enablements > would have been left behind). > > If we do not want the fork to actually fail, a simple call to > user_event_mm_remove() within user_event_mm_dup() would address the > inconsistency. However, it would be silent. Some questions follow. > > Steve, is it fine to have fork() fail when we cannot copy state? Uprobe > seems to just warn here. > > Jeff, I assume we have cases in mind where we really prefer fork() to > fail when user_events cannot be propogated? (For sure we want the > inconsistency fixed, just not sure about if silent is OK or not). > > Thanks, > -Beau Yes, that was intentional. My concern with allowing fork() to succeed after removing the child's user_events state is that the allocation failure then becomes a silent loss of inherited tracing state. The enable word is the userspace-visible indication that an event is enabled. If the child loses its inherited enablers, later enable and disable changes will no longer be reflected in that child. Removing the state fixes the stale-value inconsistency, but userspace has no indication from fork() that the child is no longer following the inherited tracing state. I also think there is a potential security implication here. If user_events are being used for tracing or auditing, an allocation failure could result in a successfully created child silently no longer following subsequent enablement changes. I don't want to characterize that as a security vulnerability without a demonstrated security boundary, but silently losing that state seems undesirable for auditing in particular. That is why I favored returning -ENOMEM: either the child is created with the inherited user_events state intact, or the failure is visible to userspace and the fork is unwound. I agree that uprobes provides a useful comparison. If you think user_events should likewise be best-effort across fork, then removing the state on duplication failure would address the inconsistency without introducing the new fork() failure path. Thanks, Jeff > >> > 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: stable@vger.kernel.org >> > Signed-off-by: Jeff Barnes <jeffbarnes@linux.microsoft.com> >> > --- >> > 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, > ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tracing/user_events: Fail fork when event state duplication fails 2026-10-06 12:01 ` Jeff Barnes @ 2026-10-06 12:38 ` Steven Rostedt 2026-10-06 13:42 ` Jeff Barnes 0 siblings, 1 reply; 7+ messages in thread From: Steven Rostedt @ 2026-10-06 12:38 UTC (permalink / raw) To: Jeff Barnes Cc: Beau Belgrave, linux-trace-kernel, mhiramat, mathieu.desnoyers, linux-kernel, akpm, kees On Tue, 6 Oct 2026 08:01:14 -0400 Jeff Barnes <jeffbarnes@linux.microsoft.com> wrote: > Yes, that was intentional. My concern with allowing fork() to succeed > after removing the child's user_events state is that the allocation > failure then becomes a silent loss of inherited tracing state. > > The enable word is the userspace-visible indication that an event is > enabled. If the child loses its inherited enablers, later enable and > disable changes will no longer be reflected in that child. Removing the > state fixes the stale-value inconsistency, but userspace has no > indication from fork() that the child is no longer following the > inherited tracing state. > > I also think there is a potential security implication here. If > user_events are being used for tracing or auditing, an allocation > failure could result in a successfully created child silently no longer > following subsequent enablement changes. I don't want to characterize > that as a security vulnerability without a demonstrated security > boundary, but silently losing that state seems undesirable for auditing > in particular. > > That is why I favored returning -ENOMEM: either the child is created > with the inherited user_events state intact, or the failure is visible > to userspace and the fork is unwound. > > I agree that uprobes provides a useful comparison. If you think > user_events should likewise be best-effort across fork, then removing > the state on duplication failure would address the inconsistency without > introducing the new fork() failure path. Question, to use user events the application needs to be involved, correct? That is, there's code in the application specific for user_events, as supposed to uprobes that can attach to any application. Thus, it makes sense for uprobes to only warn on failure. Why should a task fail to fork if something attaches a uprobe on it and it causes issues. Now if user_events is driven by the application that has them, then yes, it makes sense for fork() to fail if the user_event it created fails processing inside the fork(). If the user application is expecting something, then if it fails it should know about it. But this is only if user_events is driven by the application doing the fork(). -- Steve ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tracing/user_events: Fail fork when event state duplication fails 2026-10-06 12:38 ` Steven Rostedt @ 2026-10-06 13:42 ` Jeff Barnes 2026-10-06 14:14 ` Steven Rostedt 0 siblings, 1 reply; 7+ messages in thread From: Jeff Barnes @ 2026-10-06 13:42 UTC (permalink / raw) To: Steven Rostedt Cc: Beau Belgrave, linux-trace-kernel, mhiramat, mathieu.desnoyers, linux-kernel, akpm, kees On Oct 6 2026, at 8:38 am, Steven Rostedt <rostedt@goodmis.org> wrote: > On Tue, 6 Oct 2026 08:01:14 -0400 > Jeff Barnes <jeffbarnes@linux.microsoft.com> wrote: > >> Yes, that was intentional. My concern with allowing fork() to succeed >> after removing the child's user_events state is that the allocation >> failure then becomes a silent loss of inherited tracing state. >> >> The enable word is the userspace-visible indication that an event is >> enabled. If the child loses its inherited enablers, later enable and >> disable changes will no longer be reflected in that child. Removing the >> state fixes the stale-value inconsistency, but userspace has no >> indication from fork() that the child is no longer following the >> inherited tracing state. >> >> I also think there is a potential security implication here. If >> user_events are being used for tracing or auditing, an allocation >> failure could result in a successfully created child silently no longer >> following subsequent enablement changes. I don't want to characterize >> that as a security vulnerability without a demonstrated security >> boundary, but silently losing that state seems undesirable for auditing >> in particular. >> >> That is why I favored returning -ENOMEM: either the child is created >> with the inherited user_events state intact, or the failure is visible >> to userspace and the fork is unwound. >> >> I agree that uprobes provides a useful comparison. If you think >> user_events should likewise be best-effort across fork, then removing >> the state on duplication failure would address the inconsistency without >> introducing the new fork() failure path. > > Question, to use user events the application needs to be involved, > correct? That is, there's code in the application specific for > user_events, as supposed to uprobes that can attach to any application. > > Thus, it makes sense for uprobes to only warn on failure. Why should a > task fail to fork if something attaches a uprobe on it and it causes > issues. > > Now if user_events is driven by the application that has them, then > yes, it makes sense for fork() to fail if the user_event it created > fails processing inside the fork(). If the user application is > expecting something, then if it fails it should know about it. > > But this is only if user_events is driven by the application doing the > fork(). > > -- Steve > Yes, that's correct. user_events registration is initiated by the application through the user_events interface, rather than being attached externally to an arbitrary application like an uprobe. So in this case the state being duplicated during fork() is state that the application itself established. That's why I think propagating the allocation failure back through fork() is preferable to silently allowing the child to lose that state. Thanks, Jeff ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] tracing/user_events: Fail fork when event state duplication fails 2026-10-06 13:42 ` Jeff Barnes @ 2026-10-06 14:14 ` Steven Rostedt 0 siblings, 0 replies; 7+ messages in thread From: Steven Rostedt @ 2026-10-06 14:14 UTC (permalink / raw) To: Jeff Barnes Cc: Beau Belgrave, linux-trace-kernel, mhiramat, mathieu.desnoyers, linux-kernel, akpm, kees On Tue, 6 Oct 2026 09:42:39 -0400 Jeff Barnes <jeffbarnes@linux.microsoft.com> wrote: > So in this case the state being duplicated during fork() is state that > the application itself established. That's why I think propagating the > allocation failure back through fork() is preferable to silently > allowing the child to lose that state. I agree. -- Steve ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2026-10-06 14:14 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2026-10-02 22:26 [PATCH] tracing/user_events: Fail fork when event state duplication fails Jeff Barnes 2026-10-04 8:00 ` Steven Rostedt 2026-10-05 21:56 ` Beau Belgrave 2026-10-06 12:01 ` Jeff Barnes 2026-10-06 12:38 ` Steven Rostedt 2026-10-06 13:42 ` Jeff Barnes 2026-10-06 14:14 ` Steven Rostedt
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®