Linux Trace Kernel
 help / color / mirror / Atom feed
* [PATCH 0/2] tracing/user_events: fix use-after-free in user_event_mm_dup()
@ 2026-07-07 16:59 Michael Bommarito
  2026-07-07 16:59 ` [PATCH 1/2] " Michael Bommarito
  2026-07-07 16:59 ` [PATCH 2/2] selftests/user_events: wait for deferred event teardown after unregister Michael Bommarito
  0 siblings, 2 replies; 4+ messages in thread
From: Michael Bommarito @ 2026-07-07 16:59 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers
  Cc: Beau Belgrave, XIAO WU, linux-trace-kernel, linux-kernel, stable

This replaces the earlier single patch "tracing/user_events: fix
use-after-free of enabler in user_event_mm_dup()" that is in the tracing
for-linus branch; Steven agreed to drop that one and take this instead.

user_event_enabler_destroy() removes an enabler from the mm enabler list
that user_event_mm_dup() walks locklessly under rcu_read_lock() during
fork(), then drops the enabler's event reference and frees the enabler
without waiting for a grace period. A concurrent fork() walker can
therefore both dereference the freed enabler and take a reference on a
user_event that the put has already freed -- two use-after-frees, one on
the enabler and one on the user_event. The enabler use-after-free was
found first; XIAO WU then reported the user_event one, with a PoC and a
KASAN slab-use-after-free (write) in user_event_mm_dup(), and the
enabler-only fix did not address it.

Patch 1 holds both the enabler and its event reference until an RCU grace
period has elapsed, by deferring the put and the free to a work item
queued with queue_rcu_work(). The approach was suggested by Beau
Belgrave; it supersedes the enabler-only fix.

Patch 2 adjusts two user_events selftests that assumed the event is torn
down the instant an unregister returns; with the deferred put, DIAG_IOCSDEL
can briefly return -EBUSY, so they now wait for the delete to take effect.

Verified under KASAN on x86-64: the race faults on the unpatched kernel
(and panics with kasan.fault=panic), a benign serialized control is clean,
and the patched kernel is clean across repeated runs. The user_events
selftests pass on both kernels with patch 2 applied.

Michael Bommarito (2):
  tracing/user_events: fix use-after-free in user_event_mm_dup()
  selftests/user_events: wait for deferred event teardown after
    unregister

 kernel/trace/trace_events_user.c              | 39 +++++++++++++++----
 .../testing/selftests/user_events/abi_test.c  | 24 +++++++++++-
 .../testing/selftests/user_events/perf_test.c | 26 +++++++++++--
 3 files changed, 78 insertions(+), 11 deletions(-)


base-commit: f24ca6729076623c9a0547ecc71e4fc1c4b65c3c
-- 
2.53.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH 1/2] tracing/user_events: fix use-after-free in user_event_mm_dup()
  2026-07-07 16:59 [PATCH 0/2] tracing/user_events: fix use-after-free in user_event_mm_dup() Michael Bommarito
@ 2026-07-07 16:59 ` Michael Bommarito
  2026-07-07 16:59 ` [PATCH 2/2] selftests/user_events: wait for deferred event teardown after unregister Michael Bommarito
  1 sibling, 0 replies; 4+ messages in thread
From: Michael Bommarito @ 2026-07-07 16:59 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers
  Cc: Beau Belgrave, XIAO WU, linux-trace-kernel, linux-kernel, stable

user_event_mm_dup() walks the parent mm's enabler list locklessly under
rcu_read_lock() during fork() (from copy_process()); it does not take
event_mutex:

	rcu_read_lock();
	list_for_each_entry_rcu(enabler, &old_mm->enablers, mm_enablers_link)
		enabler->event = user_event_get(orig->event);

user_event_enabler_destroy() removes an enabler from that list with
list_del_rcu() and then, without waiting for a grace period, drops the
enabler's user_event reference with user_event_put() and frees the enabler
with kfree(). A reader that loaded the enabler before the list_del_rcu()
can still be walking it, which leads to two use-after-frees:

 - kfree(enabler) frees the enabler while that reader dereferences
   enabler->event.

 - user_event_put() may drop the last reference to the user_event, which
   is then freed (via delayed_destroy_user_event() on a work queue), while
   the same reader does user_event_get(orig->event) on it.

Both are reachable by an unprivileged task that can open user_events_data:
one multithreaded process that registers an enabler and then concurrently
unregisters it and calls fork() triggers the race. KASAN reports a
slab-use-after-free in user_event_mm_dup() during clone(), with a
"refcount_t: addition on 0" warning when the user_event is freed.

The enabler use-after-free was found first; the user_event one was reported
by XIAO WU, and the earlier enabler-only fix did not address it.

Defer both the user_event_put() and the kfree(enabler) to a work item
queued with queue_rcu_work(), so they run only after an RCU grace period,
once all readers walking the enabler list have finished. The put must run
in process context because user_event_put() takes event_mutex on the last
reference, so a work queue is used rather than call_rcu(). The now-unlocked
put lets the locked argument of user_event_enabler_destroy() be removed;
all callers are updated.

Fixes: 7235759084a4 ("tracing/user_events: Use remote writes for event enablement")
Cc: stable@vger.kernel.org
Reported-by: XIAO WU <xiaowu.417@qq.com>
Closes: https://lore.kernel.org/all/tencent_89647CE40DC452B891C65C94D1B271DE8E07@qq.com/
Suggested-by: Beau Belgrave <beaub@linux.microsoft.com>
Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
Since v1 (the enabler-only fix "tracing/user_events: fix use-after-free
of enabler in user_event_mm_dup()", being dropped from for-linus):
 - also fix the user_event-object UAF reported by XIAO WU; defer both the
   put and the free with queue_rcu_work() instead of kfree_rcu().
 - drop the locked argument now that the put runs from the work item.
 - add patch 2 for the selftest teardown timing.

 kernel/trace/trace_events_user.c | 39 ++++++++++++++++++++++++++------
 1 file changed, 32 insertions(+), 7 deletions(-)

diff --git a/kernel/trace/trace_events_user.c b/kernel/trace/trace_events_user.c
index c4ba484f7b38b..8c82ecb735f41 100644
--- a/kernel/trace/trace_events_user.c
+++ b/kernel/trace/trace_events_user.c
@@ -109,6 +109,9 @@ struct user_event_enabler {
 
 	/* Track enable bit, flags, etc. Aligned for bitops. */
 	unsigned long		values;
+
+	/* Defer the event put and enabler free past an RCU grace period. */
+	struct rcu_work		put_rwork;
 };
 
 /* Bits 0-5 are for the bit to update upon enable/disable (0-63 allowed) */
@@ -396,17 +399,39 @@ static struct user_event_group *user_event_group_create(void)
 	return NULL;
 };
 
-static void user_event_enabler_destroy(struct user_event_enabler *enabler,
-				       bool locked)
+static void delayed_user_event_enabler_put(struct work_struct *work)
 {
-	list_del_rcu(&enabler->mm_enablers_link);
+	struct user_event_enabler *enabler = container_of(to_rcu_work(work),
+			struct user_event_enabler, put_rwork);
 
 	/* No longer tracking the event via the enabler */
-	user_event_put(enabler->event, locked);
+	user_event_put(enabler->event, false);
 
+	/* Run from queue_rcu_work(), the RCU grace period has elapsed */
 	kfree(enabler);
 }
 
+static void user_event_enabler_destroy(struct user_event_enabler *enabler)
+{
+	list_del_rcu(&enabler->mm_enablers_link);
+
+	/*
+	 * The enabler is removed from an RCU-traversed list
+	 * (user_event_mm_dup() walks mm->enablers under rcu_read_lock() only),
+	 * and readers there dereference enabler->event and take a new ref on
+	 * it. Both the put of that event reference and the free of the enabler
+	 * therefore have to wait for a grace period so no reader can be looking
+	 * at the enabler or racing the last put of its event.
+	 *
+	 * The put itself must not run in RCU context: when it drops the last
+	 * reference user_event_put() takes event_mutex, which cannot be taken
+	 * from a softirq/RCU callback. Defer both to a work item scheduled
+	 * after a grace period via queue_rcu_work().
+	 */
+	INIT_RCU_WORK(&enabler->put_rwork, delayed_user_event_enabler_put);
+	queue_rcu_work(system_percpu_wq, &enabler->put_rwork);
+}
+
 static int user_event_mm_fault_in(struct user_event_mm *mm, unsigned long uaddr,
 				  int attempt)
 {
@@ -464,7 +489,7 @@ static void user_event_enabler_fault_fixup(struct work_struct *work)
 
 	/* User asked for enabler to be removed during fault */
 	if (test_bit(ENABLE_VAL_FREEING_BIT, ENABLE_BITOPS(enabler))) {
-		user_event_enabler_destroy(enabler, true);
+		user_event_enabler_destroy(enabler);
 		goto out;
 	}
 
@@ -764,7 +789,7 @@ static void user_event_mm_destroy(struct user_event_mm *mm)
 	struct user_event_enabler *enabler, *next;
 
 	list_for_each_entry_safe(enabler, next, &mm->enablers, mm_enablers_link)
-		user_event_enabler_destroy(enabler, false);
+		user_event_enabler_destroy(enabler);
 
 	mmdrop(mm->mm);
 	kfree(mm);
@@ -2645,7 +2670,7 @@ static long user_events_ioctl_unreg(unsigned long uarg)
 			flags |= enabler->values & ENABLE_VAL_COMPAT_MASK;
 
 			if (!test_bit(ENABLE_VAL_FAULTING_BIT, ENABLE_BITOPS(enabler)))
-				user_event_enabler_destroy(enabler, true);
+				user_event_enabler_destroy(enabler);
 
 			/* Removed at least one */
 			ret = 0;
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* [PATCH 2/2] selftests/user_events: wait for deferred event teardown after unregister
  2026-07-07 16:59 [PATCH 0/2] tracing/user_events: fix use-after-free in user_event_mm_dup() Michael Bommarito
  2026-07-07 16:59 ` [PATCH 1/2] " Michael Bommarito
@ 2026-07-07 16:59 ` Michael Bommarito
  2026-07-07 17:41   ` Steven Rostedt
  1 sibling, 1 reply; 4+ messages in thread
From: Michael Bommarito @ 2026-07-07 16:59 UTC (permalink / raw)
  To: Steven Rostedt, Masami Hiramatsu, Mathieu Desnoyers
  Cc: Beau Belgrave, XIAO WU, linux-trace-kernel, linux-kernel, stable

Unregistering a user event now defers the drop of the enabler's event
reference (and the freeing of the enabler) past an RCU grace period. As a
result DIAG_IOCSDEL can transiently fail with -EBUSY while that last
reference is still being dropped, where it previously succeeded
immediately.

Two tests assumed the delete takes effect the instant the unregister
returns:

  - abi_test "flags" deletes the event right after disabling it.
  - perf_test's fixture teardown clear() deletes __test_event before the
    next test registers the same name; a stale event makes the following
    registration fail with -EADDRINUSE.

Retry the delete until it succeeds (or the event is already gone) with a
bounded wait, matching the existing wait_for_delete() idiom in the same
suite, so the tests are robust to the deferred teardown.

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 .../testing/selftests/user_events/abi_test.c  | 24 ++++++++++++++++-
 .../testing/selftests/user_events/perf_test.c | 26 ++++++++++++++++---
 2 files changed, 46 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/user_events/abi_test.c b/tools/testing/selftests/user_events/abi_test.c
index 85892b3b719cc..9e2f84d281afc 100644
--- a/tools/testing/selftests/user_events/abi_test.c
+++ b/tools/testing/selftests/user_events/abi_test.c
@@ -132,6 +132,28 @@ static int event_delete(void)
 	return ret;
 }
 
+/*
+ * Deleting an event drops its last reference, but an unregister may defer
+ * that put (and the freeing of the associated enabler) past an RCU grace
+ * period. The delete can therefore transiently fail with -EBUSY while the
+ * previous reference is still being dropped. Retry for up to ~10 seconds.
+ */
+static int wait_for_event_delete(void)
+{
+	int i, ret;
+
+	for (i = 0; i < 10000; ++i) {
+		ret = event_delete();
+
+		if (ret == 0)
+			return 0;
+
+		usleep(1000);
+	}
+
+	return ret;
+}
+
 static int reg_enable_multi(void *enable, int size, int bit, int flags,
 			    char *args)
 {
@@ -262,7 +284,7 @@ TEST_F(user, flags) {
 	ASSERT_TRUE(event_exists());
 
 	/* Ensure we can delete it */
-	ASSERT_EQ(0, event_delete());
+	ASSERT_EQ(0, wait_for_event_delete());
 
 	/* USER_EVENT_REG_MAX or above is not allowed */
 	ASSERT_EQ(-1, reg_enable_flags(&self->check, sizeof(int), 0,
diff --git a/tools/testing/selftests/user_events/perf_test.c b/tools/testing/selftests/user_events/perf_test.c
index cafec0e52eb31..5727cb5b914cf 100644
--- a/tools/testing/selftests/user_events/perf_test.c
+++ b/tools/testing/selftests/user_events/perf_test.c
@@ -85,6 +85,7 @@ static int get_offset(void)
 static int clear(int *check)
 {
 	struct user_unreg unreg = {0};
+	int i, ret;
 
 	unreg.size = sizeof(unreg);
 	unreg.disable_bit = 31;
@@ -99,13 +100,32 @@ static int clear(int *check)
 		if (errno != ENOENT)
 			return -1;
 
-	if (ioctl(fd, DIAG_IOCSDEL, "__test_event") == -1)
-		if (errno != ENOENT)
+	/*
+	 * Deleting the event drops its last reference, but the unregister
+	 * above defers that put (and the freeing of the enabler) past an RCU
+	 * grace period. The delete can therefore transiently fail with -EBUSY
+	 * until that reference is dropped. Retry for up to ~10 seconds so the
+	 * event is actually gone before the next test registers the same name.
+	 */
+	for (i = 0; i < 10000; ++i) {
+		ret = ioctl(fd, DIAG_IOCSDEL, "__test_event");
+
+		if (ret == 0 || errno == ENOENT) {
+			ret = 0;
+			break;
+		}
+
+		if (errno != EBUSY) {
+			close(fd);
 			return -1;
+		}
+
+		usleep(1000);
+	}
 
 	close(fd);
 
-	return 0;
+	return ret;
 }
 
 FIXTURE(user) {
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH 2/2] selftests/user_events: wait for deferred event teardown after unregister
  2026-07-07 16:59 ` [PATCH 2/2] selftests/user_events: wait for deferred event teardown after unregister Michael Bommarito
@ 2026-07-07 17:41   ` Steven Rostedt
  0 siblings, 0 replies; 4+ messages in thread
From: Steven Rostedt @ 2026-07-07 17:41 UTC (permalink / raw)
  To: Michael Bommarito
  Cc: Masami Hiramatsu, Mathieu Desnoyers, Beau Belgrave, XIAO WU,
	linux-trace-kernel, linux-kernel, stable

On Tue,  7 Jul 2026 12:59:12 -0400
Michael Bommarito <michael.bommarito@gmail.com> wrote:

>  
> +/*
> + * Deleting an event drops its last reference, but an unregister may defer
> + * that put (and the freeing of the associated enabler) past an RCU grace
> + * period. The delete can therefore transiently fail with -EBUSY while the
> + * previous reference is still being dropped. Retry for up to ~10 seconds.
> + */
> +static int wait_for_event_delete(void)
> +{
> +	int i, ret;
> +
> +	for (i = 0; i < 10000; ++i) {
> +		ret = event_delete();
> +
> +		if (ret == 0)
> +			return 0;
> +
> +		usleep(1000);
> +	}
> +
> +	return ret;
> +}
> +

Care to address Sashiko's comment: https://sashiko.dev/#/patchset/20260707165912.2560537-2-michael.bommarito%40gmail.com

I'll pull in patch 1 and start testing it as this one is just the tools
change, it doesn't need my testing (my tests only tests kernel changes)

-- Steve

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-07-07 17:41 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-07 16:59 [PATCH 0/2] tracing/user_events: fix use-after-free in user_event_mm_dup() Michael Bommarito
2026-07-07 16:59 ` [PATCH 1/2] " Michael Bommarito
2026-07-07 16:59 ` [PATCH 2/2] selftests/user_events: wait for deferred event teardown after unregister Michael Bommarito
2026-07-07 17:41   ` Steven Rostedt

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox