[PATCH] perf: Fix mmap_count accounting on the perf_mmap_close() race path

[PATCH] perf: Fix mmap_count accounting on the perf_mmap_close() race path

[Aohan Mei]

From: Aohan Mei <henrymei@tencent.com>

perf_mmap_rb()'s raced path (a concurrent perf_mmap_close() has dropped
rb->mmap_count to 0 but is still blocked on event->mmap_mutex inside
refcount_dec_and_mutex_lock()) installs a fresh ring buffer and then
does refcount_set(&event->mmap_count, 1).

That is wrong: the blocked closer still holds a pending decrement and
the count is 1 at this point. The refcount_set() leaves the count at 1,
so once perf_mmap() drops the mutex the closer observes a 1->0
transition, detaches the *new* buffer via ring_buffer_attach(event,
NULL) and drops its last reference, freeing pages that the racing
mmap() is about to map. A later munmap() of that mapping then finds
event->rb == NULL and crashes the kernel in perf_mmap_close().

Restore the pre-59741451b49c accounting on the raced path: the count is
guaranteed non-zero there, because the closer's decrement of
event->mmap_count only happens while holding mmap_mutex, which the
mapper holds. Use refcount_inc(). Keep refcount_set(..., 1) for the
genuine first mmap, where the 0->1 transition is required and
refcount_inc() would WARN.

Fixes: 59741451b49c ("perf: Identify the 0->1 transition for event::mmap_count")
Reported-by: TencentOS Corvus AI <corvus@tencent.com>
Cc: stable@vger.kernel.org
Assisted-by: CodeBuddy:Kimi-K3
Signed-off-by: Aohan Mei <henrymei@tencent.com>
---
 kernel/events/core.c | 18 +++++++++++++++++-
 1 file changed, 17 insertions(+), 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 4638544205f2..adcc04ca05f8 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7273,6 +7273,7 @@ static int perf_mmap_rb(struct vm_area_struct *vma, struct perf_event *event,
 	long extra = 0, user_extra = nr_pages;
 	struct perf_buffer *rb;
 	int rb_flags = 0;
+	bool raced = false;
 
 	nr_pages -= 1;
 
@@ -7314,6 +7315,7 @@ static int perf_mmap_rb(struct vm_area_struct *vma, struct perf_event *event,
 		 * event and continue as if !event->rb
 		 */
 		ring_buffer_attach(event, NULL);
+		raced = true;
 	}
 
 	if (!perf_mmap_calc_limits(vma, &user_extra, &extra))
@@ -7338,7 +7340,21 @@ static int perf_mmap_rb(struct vm_area_struct *vma, struct perf_event *event,
 	perf_event_update_userpage(event);
 
 	perf_mmap_account(vma, user_extra, extra);
-	refcount_set(&event->mmap_count, 1);
+
+	/*
+	 * On the raced path above, a concurrent perf_mmap_close() can
+	 * still have a pending decrement of event->mmap_count: it sits
+	 * blocked inside refcount_dec_and_mutex_lock() on event->mmap_mutex
+	 * (which we hold) with the count still at 1. Using
+	 * refcount_set(..., 1) here would make that closer observe a 1->0
+	 * transition once we drop the mutex, causing it to detach and free
+	 * the buffer we just installed, while this mmap() still maps it.
+	 * The count is guaranteed non-zero on the raced path, so increment.
+	 */
+	if (raced)
+		refcount_inc(&event->mmap_count);
+	else
+		refcount_set(&event->mmap_count, 1);
 
 	return 0;
 }
-- 
2.43.7

Re: [PATCH] perf: Fix mmap_count accounting on the perf_mmap_close() race path

[sashiko-bot]

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

New issues:
- [Critical] Incrementing event->mmap_count on the raced path causes the concurrent perf_mmap_close() to skip cleanup and unaccounting of the old ring buffer, permanently leaking memory charges and the old ring buffer itself, leading to a Use-After-Free.

Pre-existing issues:
- [High] perf_mmap_aux() lacks protection against mmaping redirected events, causing a refcount saturation WARN on event->mmap_count.
- [High] Mmapping the same perf event multiple times permanently leaks locked_vm and pinned_vm accounting charges.
--

commit 74767b5c0f42ab023d9f220cb4e3a861a837fda1
Author: Aohan Mei <henrymei@tencent.com>

perf: Fix mmap_count accounting on the perf_mmap_close() race path

This patch aims to fix an issue on the perf_mmap_rb() raced path where
a concurrent perf_mmap_close() has dropped the ring buffer's mmap_count to
zero. It restores the previous behavior of incrementing event->mmap_count
instead of setting it to 1, to prevent the concurrent closer from observing
a 1->0 transition and prematurely freeing the new buffer.

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index a6c8e38a31104..7b55156bbc023 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7281,6 +7281,7 @@ static int perf_mmap_rb(struct vm_area_struct *vma, struct perf_event *event,
>  	long extra = 0, user_extra = nr_pages;
>  	struct perf_buffer *rb;
>  	int rb_flags = 0;
> +	bool raced = false;
>  
>  	nr_pages -= 1;

[Severity: High]
This is a pre-existing issue, but could mmapping the same perf event multiple
times permanently leak locked_vm and pinned_vm accounting charges?

When perf_mmap_rb() handles overlapping alias mappings (just before the raced
path check), it calls perf_mmap_account() for each mapping.

However, during teardown in perf_mmap_close(), perf_mmap_unaccount() is only
called once when the final mapping is unmapped and detach_rest is true. For
all prior munmap() calls, the unaccounting seems to be completely skipped
because detach_rest is false.

Could a user exhaust their RLIMIT_MEMLOCK quota by repeatedly mmapping and
munmapping the same perf event?

[ ... ]

> @@ -7346,7 +7348,21 @@ static int perf_mmap_rb(struct vm_area_struct *vma, struct perf_event *event,
>  	perf_event_update_userpage(event);
>  
>  	perf_mmap_account(vma, user_extra, extra);
> -	refcount_set(&event->mmap_count, 1);
> +
> +	/*
> +	 * On the raced path above, a concurrent perf_mmap_close() can
> +	 * still have a pending decrement of event->mmap_count: it sits
> +	 * blocked inside refcount_dec_and_mutex_lock() on event->mmap_mutex
> +	 * (which we hold) with the count still at 1. Using
> +	 * refcount_set(..., 1) here would make that closer observe a 1->0
> +	 * transition once we drop the mutex, causing it to detach and free
> +	 * the buffer we just installed, while this mmap() still maps it.
> +	 * The count is guaranteed non-zero on the raced path, so increment.
> +	 */
> +	if (raced)
> +		refcount_inc(&event->mmap_count);

[Severity: Critical]
Might incrementing event->mmap_count on the raced path inadvertently break the
teardown logic in the racing closer?

By inflating the count from 1 to 2, the concurrent perf_mmap_close() for the
old ring buffer might see refcount_dec_and_mutex_lock() return false. This
would cause it to jump directly to the out_put label, completely skipping the
detachment of other events from old_rb->event_list and skipping
perf_mmap_unaccount().

Would this cause the old_rb to be freed while other events are still attached,
leading to a use-after-free if those events attempt to write to the freed
buffer? Does this also permanently leak memory charges since the unaccounting
is skipped?

[Severity: High]
This isn't a bug introduced by this patch, but it appears perf_mmap_aux()
lacks protection against mmapping redirected events. Could this lead to a
refcount saturation WARN?

In perf_mmap_aux(), the code validates that an rb exists but seems to fail to
verify if the event actually owns the mmap count via a check like
!refcount_read(&event->mmap_count). It unconditionally calls
refcount_inc(&event->mmap_count).

If a user redirects event B's output to event A via
ioctl(PERF_EVENT_IOC_SET_OUTPUT), setting B->mmap_count to 0, and then calls
mmap() on event B with a non-zero offset, will this cause a refcount
saturation WARN?

> +	else
> +		refcount_set(&event->mmap_count, 1);
>  
>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260830145512.2583689-1-ljp1205831794@gmail.com?part=1