All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v4] perf: Fix use-after-free when perf mmap() revival races with the last munmap()
@ 2026-08-31 16:21 Yilin Zhang
  2026-08-31 19:10 ` sashiko-bot
  2026-09-02  7:27 ` [tip: perf/urgent] " tip-bot2 for Yilin Zhang
  0 siblings, 2 replies; 3+ messages in thread
From: Yilin Zhang @ 2026-08-31 16:21 UTC (permalink / raw)
  To: peterz
  Cc: Yilin Zhang, mingo, acme, namhyung, kylebot, david.lee,
	linux-perf-users, stable, Kimi Security Team, Weiming Shi

perf_mmap_close() drops rb->mmap_count *without* holding
event->mmap_mutex (the refcount_dec_and_test() right before the
refcount_dec_and_mutex_lock() of event->mmap_count). A concurrent
perf_mmap_rb() can slot its entire "revival" path into that window
(perf_mmap holds event->mmap_mutex for its whole duration, including
rb_alloc):

  munmap side (perf_mmap_close)          mmap side (perf_mmap_rb)
  -----------------------------------    --------------------------------
  rb->mmap_count 1 -> 0   (no lock)      (holds event->mmap_mutex)
                                         inc_not_zero(rb->mmap_count) fails
                                         ring_buffer_attach(event, NULL)
                                         rb_alloc() + attach new rb
                                         refcount_set(&event->mmap_count, 1)
  lock; event->mmap_count 1 -> 0
  ring_buffer_attach(event, NULL)
  ring_buffer_put() -> frees the *new* rb

The revival's refcount_set(&event->mmap_count, 1) is an invisible
1 -> 1 write: the close frees the just-revived buffer although the
other process still has it mapped -- a page-level use-after-free
allowing local privilege escalation to root by any unprivileged user
(default kernel.perf_event_paranoid=2).

Swap the order of the two counter updates: event->mmap_count is
dropped first via refcount_dec_and_mutex_lock(), so its 1 -> 0
transition and the ring_buffer_attach() stay serialized with
perf_mmap(). rb->mmap_count == 0 then implies every event using the
buffer is detached already, so the result of the rb->mmap_count drop
can gate the remaining teardown directly and detach_rest is no longer
needed.

An earlier fix for this race from Kyle Zeng and David Lee takes
event->mmap_mutex around both counter updates [0]; here the not-last
close stays lockless.

Fixes: 59741451b49c ("perf: Identify the 0->1 transition for event::mmap_count")
Link: https://lore.kernel.org/linux-perf-users/20260804060931.711308-1-david.lee@trailofbits.com/ [0]
Cc: stable@vger.kernel.org # 6.18+
Reported-by: Kimi Security Team <bug-report@moonshot.ai>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Co-developed-by: Weiming Shi <shiweiming@moonshot.ai>
Signed-off-by: Weiming Shi <shiweiming@moonshot.ai>
Signed-off-by: Yilin Zhang <yilinzhang@moonshot.ai>
---
v3: https://lore.kernel.org/linux-perf-users/20260831133152.1231045-1-yilinzhang@moonshot.ai/
v4: restore the ordering comment; no functional change

 kernel/events/core.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index a6c8e38a3110..f02780529b43 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7029,7 +7029,6 @@ static void perf_mmap_close(struct vm_area_struct *vma)
 	mapped_f unmapped = get_mapped(event, event_unmapped);
 	struct perf_buffer *rb = ring_buffer_get(event);
 	struct user_struct *mmap_user = rb->mmap_user;
-	bool detach_rest = false;
 
 	/* FIXIES vs perf_pmu_unregister() */
 	if (unmapped)
@@ -7060,17 +7059,18 @@ static void perf_mmap_close(struct vm_area_struct *vma)
 		mutex_unlock(&rb->aux_mutex);
 	}
 
-	if (refcount_dec_and_test(&rb->mmap_count))
-		detach_rest = true;
-
-	if (!refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex))
-		goto out_put;
-
-	ring_buffer_attach(event, NULL);
-	mutex_unlock(&event->mmap_mutex);
+	/*
+	 * Drop references in reverse order of perf_mmap() to prevent
+	 * rb revival after rb->mmap_count reaches zero.
+	 */
+	if (refcount_dec_and_mutex_lock(&event->mmap_count,
+					&event->mmap_mutex)) {
+		ring_buffer_attach(event, NULL);
+		mutex_unlock(&event->mmap_mutex);
+	}
 
 	/* If there's still other mmap()s of this buffer, we're done. */
-	if (!detach_rest)
+	if (!refcount_dec_and_test(&rb->mmap_count))
 		goto out_put;
 
 	/*
-- 
2.43.0

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

* Re: [PATCH v4] perf: Fix use-after-free when perf mmap() revival races with the last munmap()
  2026-08-31 16:21 [PATCH v4] perf: Fix use-after-free when perf mmap() revival races with the last munmap() Yilin Zhang
@ 2026-08-31 19:10 ` sashiko-bot
  2026-09-02  7:27 ` [tip: perf/urgent] " tip-bot2 for Yilin Zhang
  1 sibling, 0 replies; 3+ messages in thread
From: sashiko-bot @ 2026-08-31 19:10 UTC (permalink / raw)
  To: Yilin Zhang; +Cc: linux-perf-users

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

Pre-existing issues:
- [High] Preexisting memory accounting leak for locked_vm when mmap() is called multiple times on the same perf event (alias mmaps).
--

commit a96fa5a03867a00fbbf2c3b6dd3bf82931c0fdde
Author: Yilin Zhang <yilinzhang@moonshot.ai>

perf: Fix use-after-free when perf mmap() revival races with the last munmap()

This commit swaps the order of counter updates in perf_mmap_close() to fix
a page-level use-after-free. It drops event->mmap_count first, serializing
its 1 -> 0 transition and ring_buffer_attach() with perf_mmap(), preventing
buffer revival after rb->mmap_count reaches zero.

Link: https://lore.kernel.org/linux-perf-users/20260804060931.711308-1-david.lee@trailofbits.com/

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index a6c8e38a31104..f02780529b439 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
[ ... ]
> @@ -7060,17 +7059,18 @@ static void perf_mmap_close(struct vm_area_struct *vma)
>  		mutex_unlock(&rb->aux_mutex);
>  	}
>  
> -	if (refcount_dec_and_test(&rb->mmap_count))
> -		detach_rest = true;
> -
> -	if (!refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex))
> -		goto out_put;
> -
> -	ring_buffer_attach(event, NULL);
> -	mutex_unlock(&event->mmap_mutex);
> +	/*
> +	 * Drop references in reverse order of perf_mmap() to prevent
> +	 * rb revival after rb->mmap_count reaches zero.
> +	 */
> +	if (refcount_dec_and_mutex_lock(&event->mmap_count,
> +					&event->mmap_mutex)) {
> +		ring_buffer_attach(event, NULL);
> +		mutex_unlock(&event->mmap_mutex);
> +	}
>  
>  	/* If there's still other mmap()s of this buffer, we're done. */
> -	if (!detach_rest)
> +	if (!refcount_dec_and_test(&rb->mmap_count))
>  		goto out_put;

[Severity: High]
This is a pre-existing issue, but does this goto bypass the unaccounting for
all but the final close?

If an application calls mmap() on the same perf event file descriptor multiple
times, perf_mmap_rb() accounts user_extra to current_user()->locked_vm on
every alias mmap:

kernel/events/core.c:perf_mmap_rb() {
    ...
			perf_mmap_account(vma, user_extra, extra);
			refcount_inc(&event->mmap_count);
			return 0;
    ...
}

When munmap() is called for all of them, perf_mmap_close() only calls
perf_mmap_unaccount() when rb->mmap_count drops to zero, which happens only
once.

Could this permanently leak locked_vm charges on every alias munmap,
potentially exhausting the user's RLIMIT_MEMLOCK quota and causing a Denial
of Service for other subsystems (like BPF or io_uring) relying on it?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260831162155.1437652-1-yilinzhang@moonshot.ai?part=1

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

* [tip: perf/urgent] perf: Fix use-after-free when perf mmap() revival races with the last munmap()
  2026-08-31 16:21 [PATCH v4] perf: Fix use-after-free when perf mmap() revival races with the last munmap() Yilin Zhang
  2026-08-31 19:10 ` sashiko-bot
@ 2026-09-02  7:27 ` tip-bot2 for Yilin Zhang
  1 sibling, 0 replies; 3+ messages in thread
From: tip-bot2 for Yilin Zhang @ 2026-09-02  7:27 UTC (permalink / raw)
  To: linux-tip-commits
  Cc: Kimi Security Team, Peter Zijlstra, Weiming Shi, Yilin Zhang,
	stable, stable, #, 6.18+, x86, linux-kernel

The following commit has been merged into the perf/urgent branch of tip:

Commit-ID:     58a8108bc73de0740d5b88150465d6690ea5f85f
Gitweb:        https://git.kernel.org/tip/58a8108bc73de0740d5b88150465d6690ea5f85f
Author:        Yilin Zhang <yilinzhang@moonshot.ai>
AuthorDate:    Tue, 01 Sep 2026 00:21:55 +08:00
Committer:     Peter Zijlstra <peterz@infradead.org>
CommitterDate: Wed, 02 Sep 2026 09:18:00 +02:00

perf: Fix use-after-free when perf mmap() revival races with the last munmap()

perf_mmap_close() drops rb->mmap_count *without* holding
event->mmap_mutex (the refcount_dec_and_test() right before the
refcount_dec_and_mutex_lock() of event->mmap_count). A concurrent
perf_mmap_rb() can slot its entire "revival" path into that window
(perf_mmap holds event->mmap_mutex for its whole duration, including
rb_alloc):

  munmap side (perf_mmap_close)          mmap side (perf_mmap_rb)
  -----------------------------------    --------------------------------
  rb->mmap_count 1 -> 0   (no lock)      (holds event->mmap_mutex)
                                         inc_not_zero(rb->mmap_count) fails
                                         ring_buffer_attach(event, NULL)
                                         rb_alloc() + attach new rb
                                         refcount_set(&event->mmap_count, 1)
  lock; event->mmap_count 1 -> 0
  ring_buffer_attach(event, NULL)
  ring_buffer_put() -> frees the *new* rb

The revival's refcount_set(&event->mmap_count, 1) is an invisible
1 -> 1 write: the close frees the just-revived buffer although the
other process still has it mapped -- a page-level use-after-free
allowing local privilege escalation to root by any unprivileged user
(default kernel.perf_event_paranoid=2).

Swap the order of the two counter updates: event->mmap_count is
dropped first via refcount_dec_and_mutex_lock(), so its 1 -> 0
transition and the ring_buffer_attach() stay serialized with
perf_mmap(). rb->mmap_count == 0 then implies every event using the
buffer is detached already, so the result of the rb->mmap_count drop
can gate the remaining teardown directly and detach_rest is no longer
needed.

An earlier fix for this race from Kyle Zeng and David Lee takes
event->mmap_mutex around both counter updates [0]; here the not-last
close stays lockless.

Fixes: 59741451b49c ("perf: Identify the 0->1 transition for event::mmap_count")
Reported-by: Kimi Security Team <bug-report@moonshot.ai>
Suggested-by: Peter Zijlstra <peterz@infradead.org>
Co-developed-by: Weiming Shi <shiweiming@moonshot.ai>
Signed-off-by: Weiming Shi <shiweiming@moonshot.ai>
Signed-off-by: Yilin Zhang <yilinzhang@moonshot.ai>
Signed-off-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Link: https://lore.kernel.org/linux-perf-users/20260804060931.711308-1-david.lee@trailofbits.com/ [0]
Cc: <stable@vger.kernel.org>
Cc: stable@vger.kernel.org # 6.18+
Link: https://patch.msgid.link/20260831162155.1437652-1-yilinzhang@moonshot.ai
---
 kernel/events/core.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index a6c8e38..f027805 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7029,7 +7029,6 @@ static void perf_mmap_close(struct vm_area_struct *vma)
 	mapped_f unmapped = get_mapped(event, event_unmapped);
 	struct perf_buffer *rb = ring_buffer_get(event);
 	struct user_struct *mmap_user = rb->mmap_user;
-	bool detach_rest = false;
 
 	/* FIXIES vs perf_pmu_unregister() */
 	if (unmapped)
@@ -7060,17 +7059,18 @@ static void perf_mmap_close(struct vm_area_struct *vma)
 		mutex_unlock(&rb->aux_mutex);
 	}
 
-	if (refcount_dec_and_test(&rb->mmap_count))
-		detach_rest = true;
-
-	if (!refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex))
-		goto out_put;
-
-	ring_buffer_attach(event, NULL);
-	mutex_unlock(&event->mmap_mutex);
+	/*
+	 * Drop references in reverse order of perf_mmap() to prevent
+	 * rb revival after rb->mmap_count reaches zero.
+	 */
+	if (refcount_dec_and_mutex_lock(&event->mmap_count,
+					&event->mmap_mutex)) {
+		ring_buffer_attach(event, NULL);
+		mutex_unlock(&event->mmap_mutex);
+	}
 
 	/* If there's still other mmap()s of this buffer, we're done. */
-	if (!detach_rest)
+	if (!refcount_dec_and_test(&rb->mmap_count))
 		goto out_put;
 
 	/*

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

end of thread, other threads:[~2026-09-02  7:27 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-31 16:21 [PATCH v4] perf: Fix use-after-free when perf mmap() revival races with the last munmap() Yilin Zhang
2026-08-31 19:10 ` sashiko-bot
2026-09-02  7:27 ` [tip: perf/urgent] " tip-bot2 for Yilin Zhang

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.