[PATCH] perf: Fix deadlock in perf_mmap()

[PATCH] perf: Fix deadlock in perf_mmap()

[Qing Wang]

There is a possible deadlock in perf_mmap() if mmap_range() fails then
perf_mmap_close() is called while holding event->mmap_mutex. Since
perf_mmap_close() also acquire the same mutex, this result in self-deadlock.

Fix this by moving the cleanup(perf_mmap_close()) outside the scope of
scoped_guard(mutex, &event->mmap_mutex).

Fixes: 77de62ad3de3 ("perf/core: Fix refcount bug and potential UAF in perf_mmap")
Reported-by: syzbot+196a82fd904572696b3c@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=196a82fd904572696b3c
Signed-off-by: Qing Wang <wangqing7171@gmail.com>
---
 kernel/events/core.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 1f5699b339ec..e5ce03ce926d 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7485,9 +7485,12 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 		 */
 		ret = map_range(event->rb, vma);
 		if (ret)
-			perf_mmap_close(vma);
+			goto out_close;
 	}
+	return 0;
 
+out_close:
+	perf_mmap_close(vma);
 	return ret;
 }
 
-- 
2.34.1

Re: [PATCH] perf: Fix deadlock in perf_mmap()

[Ian Rogers]

On Mon, Mar 9, 2026 at 1:26 AM Qing Wang <wangqing7171@gmail.com> wrote:
>
> There is a possible deadlock in perf_mmap() if mmap_range() fails then
> perf_mmap_close() is called while holding event->mmap_mutex. Since
> perf_mmap_close() also acquire the same mutex, this result in self-deadlock.
>
> Fix this by moving the cleanup(perf_mmap_close()) outside the scope of
> scoped_guard(mutex, &event->mmap_mutex).
>
> Fixes: 77de62ad3de3 ("perf/core: Fix refcount bug and potential UAF in perf_mmap")
> Reported-by: syzbot+196a82fd904572696b3c@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=196a82fd904572696b3c
> Signed-off-by: Qing Wang <wangqing7171@gmail.com>

Hi Qing, thank you for looking into this. I proposed a similar fix:
https://lore.kernel.org/lkml/CAP-5=fW-wHEv=TCULwk_HVOhWHdqRd8AZoESZsU_vnhLjghUBQ@mail.gmail.com/
but Haocheng noted it reintroduced the race condition the original fix
was targeting. Haocheng has a larger fix in:
https://lore.kernel.org/lkml/20260306093616.84299-1-yuhaocheng035@gmail.com/
that should handle both the original race condition and the deadlock.
I wonder that fix may be made smaller by passing a parameter like
"holds_event_mmap_mutex" to perf_mmap_close, something like:
```
static void __perf_mmap_close(struct vm_area_struct *vma, struct
perf_event *event, bool holds_event_mmap_lock)
{
   ... // code from original perf_mmap_close
   if ((!holds_event_mmap_lock &&
!refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex))
||
       (holds_event_mmap_lock && !refcount_dec_and_test(&event->mmap_count)))
     ...

static void perf_mmap_close(struct vm_area_struct *vma)
{
       struct perf_event *event = vma->vm_file->private_data;
       __perf_mmap_close(vma, event, /*holds_event_mmap_lock=*/false);
}

static void perf_mmap_close_locked(struct vm_area_struct *vma, struct
perf_event *event)
{
       __perf_mmap_close(vma, event, /*holds_event_mmap_lock=*/true);
}
```

Thanks,
Ian

> ---
>  kernel/events/core.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 1f5699b339ec..e5ce03ce926d 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7485,9 +7485,12 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
>                  */
>                 ret = map_range(event->rb, vma);
>                 if (ret)
> -                       perf_mmap_close(vma);
> +                       goto out_close;
>         }
> +       return 0;
>
> +out_close:
> +       perf_mmap_close(vma);
>         return ret;
>  }
>
> --
> 2.34.1
>

Re: [PATCH] perf: Fix deadlock in perf_mmap()

[Qing Wang]

On Tue, 10 Mar 2026 at 02:59, Ian Rogers <irogers@google.com> wrote:
> Hi Qing, thank you for looking into this. I proposed a similar fix:
> https://lore.kernel.org/lkml/CAP-5=fW-wHEv=TCULwk_HVOhWHdqRd8AZoESZsU_vnhLjghUBQ@mail.gmail.com/
> but Haocheng noted it reintroduced the race condition the original fix
> was targeting. Haocheng has a larger fix in:
> https://lore.kernel.org/lkml/20260306093616.84299-1-yuhaocheng035@gmail.com/
> that should handle both the original race condition and the deadlock.

Oh, this fix is too large.

> I wonder that fix may be made smaller by passing a parameter like
> "holds_event_mmap_mutex" to perf_mmap_close, something like:
> ```
> static void __perf_mmap_close(struct vm_area_struct *vma, struct
> perf_event *event, bool holds_event_mmap_lock)
> {
>    ... // code from original perf_mmap_close
>    if ((!holds_event_mmap_lock &&
> !refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex))
> ||
>        (holds_event_mmap_lock && !refcount_dec_and_test(&event->mmap_count)))
>      ...
> 
> static void perf_mmap_close(struct vm_area_struct *vma)
> {
>        struct perf_event *event = vma->vm_file->private_data;
>        __perf_mmap_close(vma, event, /*holds_event_mmap_lock=*/false);
> }
> 
> static void perf_mmap_close_locked(struct vm_area_struct *vma, struct
> perf_event *event)
> {
>        __perf_mmap_close(vma, event, /*holds_event_mmap_lock=*/true);
> }
> ```
> 
> Thanks,
> Ian

LGTM, your fix is better.

But, can you explain why there is still race issue after applying my patch?

Thanks,
Qing

Re: [PATCH] perf: Fix deadlock in perf_mmap()

[Ian Rogers]

On Mon, Mar 9, 2026 at 8:37 PM Qing Wang <wangqing7171@gmail.com> wrote:
>
> On Tue, 10 Mar 2026 at 02:59, Ian Rogers <irogers@google.com> wrote:
> > Hi Qing, thank you for looking into this. I proposed a similar fix:
> > https://lore.kernel.org/lkml/CAP-5=fW-wHEv=TCULwk_HVOhWHdqRd8AZoESZsU_vnhLjghUBQ@mail.gmail.com/
> > but Haocheng noted it reintroduced the race condition the original fix
> > was targeting. Haocheng has a larger fix in:
> > https://lore.kernel.org/lkml/20260306093616.84299-1-yuhaocheng035@gmail.com/
> > that should handle both the original race condition and the deadlock.
>
> Oh, this fix is too large.
>
> > I wonder that fix may be made smaller by passing a parameter like
> > "holds_event_mmap_mutex" to perf_mmap_close, something like:
> > ```
> > static void __perf_mmap_close(struct vm_area_struct *vma, struct
> > perf_event *event, bool holds_event_mmap_lock)
> > {
> >    ... // code from original perf_mmap_close
> >    if ((!holds_event_mmap_lock &&
> > !refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex))
> > ||
> >        (holds_event_mmap_lock && !refcount_dec_and_test(&event->mmap_count)))
> >      ...
> >
> > static void perf_mmap_close(struct vm_area_struct *vma)
> > {
> >        struct perf_event *event = vma->vm_file->private_data;
> >        __perf_mmap_close(vma, event, /*holds_event_mmap_lock=*/false);
> > }
> >
> > static void perf_mmap_close_locked(struct vm_area_struct *vma, struct
> > perf_event *event)
> > {
> >        __perf_mmap_close(vma, event, /*holds_event_mmap_lock=*/true);
> > }
> > ```
> >
> > Thanks,
> > Ian
>
> LGTM, your fix is better.
>
> But, can you explain why there is still race issue after applying my patch?

Hi Qing,

Haocheng has a complex explanation here:
https://lore.kernel.org/lkml/CAAoXzSoYPvE-AdhT28Rhik2BtFhFuntaM6uhrMqKLUaDgkyiNg@mail.gmail.com/
with the original cause, which I think stemmed from syzkaller fuzzing.
I can follow the explanation there but I'm not familiar enough with
the ring buffer code to know whether the race with open is benign or
not. I'm guessing Peter Z thought not, hence the original patch
landed.

Thanks,
Ian

> Thanks,
> Qing

Re: [PATCH] perf: Fix deadlock in perf_mmap()

[Ian Rogers]

On Mon, Mar 9, 2026 at 9:45 PM Ian Rogers <irogers@google.com> wrote:
>
> On Mon, Mar 9, 2026 at 8:37 PM Qing Wang <wangqing7171@gmail.com> wrote:
> >
> > On Tue, 10 Mar 2026 at 02:59, Ian Rogers <irogers@google.com> wrote:
> > > Hi Qing, thank you for looking into this. I proposed a similar fix:
> > > https://lore.kernel.org/lkml/CAP-5=fW-wHEv=TCULwk_HVOhWHdqRd8AZoESZsU_vnhLjghUBQ@mail.gmail.com/
> > > but Haocheng noted it reintroduced the race condition the original fix
> > > was targeting. Haocheng has a larger fix in:
> > > https://lore.kernel.org/lkml/20260306093616.84299-1-yuhaocheng035@gmail.com/
> > > that should handle both the original race condition and the deadlock.
> >
> > Oh, this fix is too large.
> >
> > > I wonder that fix may be made smaller by passing a parameter like
> > > "holds_event_mmap_mutex" to perf_mmap_close, something like:
> > > ```
> > > static void __perf_mmap_close(struct vm_area_struct *vma, struct
> > > perf_event *event, bool holds_event_mmap_lock)
> > > {
> > >    ... // code from original perf_mmap_close
> > >    if ((!holds_event_mmap_lock &&
> > > !refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex))
> > > ||
> > >        (holds_event_mmap_lock && !refcount_dec_and_test(&event->mmap_count)))
> > >      ...
> > >
> > > static void perf_mmap_close(struct vm_area_struct *vma)
> > > {
> > >        struct perf_event *event = vma->vm_file->private_data;
> > >        __perf_mmap_close(vma, event, /*holds_event_mmap_lock=*/false);
> > > }
> > >
> > > static void perf_mmap_close_locked(struct vm_area_struct *vma, struct
> > > perf_event *event)
> > > {
> > >        __perf_mmap_close(vma, event, /*holds_event_mmap_lock=*/true);
> > > }
> > > ```
> > >
> > > Thanks,
> > > Ian
> >
> > LGTM, your fix is better.
> >
> > But, can you explain why there is still race issue after applying my patch?
>
> Hi Qing,
>
> Haocheng has a complex explanation here:
> https://lore.kernel.org/lkml/CAAoXzSoYPvE-AdhT28Rhik2BtFhFuntaM6uhrMqKLUaDgkyiNg@mail.gmail.com/
> with the original cause, which I think stemmed from syzkaller fuzzing.
> I can follow the explanation there but I'm not familiar enough with
> the ring buffer code to know whether the race with open is benign or
> not. I'm guessing Peter Z thought not, hence the original patch
> landed.

I think no fix has landed yet and the original patch with the issue
has appeared in backports. Haocheng, did you want to see if you could
incorporate my feedback into your fix?

Thanks,
Ian

> Thanks,
> Ian
>
> > Thanks,
> > Qing

Re: [PATCH] perf: Fix deadlock in perf_mmap()

[Haocheng Yu]

I apologize for the previous misunderstanding—I assumed you
would submit a separate patch, since you came up with the whole
idea. Thus I didn't update my patch. Your solution is indeed better.
I'd be happy to incorporate your feedback into my fix if you wish.

Thanks,
Haocheng

Ian Rogers <irogers@google.com> 于2026年3月25日周三 02:38写道：
>
> On Mon, Mar 9, 2026 at 9:45 PM Ian Rogers <irogers@google.com> wrote:
> >
> > On Mon, Mar 9, 2026 at 8:37 PM Qing Wang <wangqing7171@gmail.com> wrote:
> > >
> > > On Tue, 10 Mar 2026 at 02:59, Ian Rogers <irogers@google.com> wrote:
> > > > Hi Qing, thank you for looking into this. I proposed a similar fix:
> > > > https://lore.kernel.org/lkml/CAP-5=fW-wHEv=TCULwk_HVOhWHdqRd8AZoESZsU_vnhLjghUBQ@mail.gmail.com/
> > > > but Haocheng noted it reintroduced the race condition the original fix
> > > > was targeting. Haocheng has a larger fix in:
> > > > https://lore.kernel.org/lkml/20260306093616.84299-1-yuhaocheng035@gmail.com/
> > > > that should handle both the original race condition and the deadlock.
> > >
> > > Oh, this fix is too large.
> > >
> > > > I wonder that fix may be made smaller by passing a parameter like
> > > > "holds_event_mmap_mutex" to perf_mmap_close, something like:
> > > > ```
> > > > static void __perf_mmap_close(struct vm_area_struct *vma, struct
> > > > perf_event *event, bool holds_event_mmap_lock)
> > > > {
> > > >    ... // code from original perf_mmap_close
> > > >    if ((!holds_event_mmap_lock &&
> > > > !refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex))
> > > > ||
> > > >        (holds_event_mmap_lock && !refcount_dec_and_test(&event->mmap_count)))
> > > >      ...
> > > >
> > > > static void perf_mmap_close(struct vm_area_struct *vma)
> > > > {
> > > >        struct perf_event *event = vma->vm_file->private_data;
> > > >        __perf_mmap_close(vma, event, /*holds_event_mmap_lock=*/false);
> > > > }
> > > >
> > > > static void perf_mmap_close_locked(struct vm_area_struct *vma, struct
> > > > perf_event *event)
> > > > {
> > > >        __perf_mmap_close(vma, event, /*holds_event_mmap_lock=*/true);
> > > > }
> > > > ```
> > > >
> > > > Thanks,
> > > > Ian
> > >
> > > LGTM, your fix is better.
> > >
> > > But, can you explain why there is still race issue after applying my patch?
> >
> > Hi Qing,
> >
> > Haocheng has a complex explanation here:
> > https://lore.kernel.org/lkml/CAAoXzSoYPvE-AdhT28Rhik2BtFhFuntaM6uhrMqKLUaDgkyiNg@mail.gmail.com/
> > with the original cause, which I think stemmed from syzkaller fuzzing.
> > I can follow the explanation there but I'm not familiar enough with
> > the ring buffer code to know whether the race with open is benign or
> > not. I'm guessing Peter Z thought not, hence the original patch
> > landed.
>
> I think no fix has landed yet and the original patch with the issue
> has appeared in backports. Haocheng, did you want to see if you could
> incorporate my feedback into your fix?
>
> Thanks,
> Ian
>
> > Thanks,
> > Ian
> >
> > > Thanks,
> > > Qing

[PATCH v3] perf/core: Fix refcount bug and potential UAF in perf_mmap

[yuhaocheng035]

From: Haocheng Yu <yuhaocheng035@gmail.com>

Syzkaller reported a refcount_t: addition on 0; use-after-free warning
in perf_mmap.

The issue is caused by a race condition between a failing mmap() setup
and a concurrent mmap() on a dependent event (e.g., using output
redirection).

In perf_mmap(), the ring_buffer (rb) is allocated and assigned to
event->rb with the mmap_mutex held. The mutex is then released to
perform map_range().

If map_range() fails, perf_mmap_close() is called to clean up.
However, since the mutex was dropped, another thread attaching to
this event (via inherited events or output redirection) can acquire
the mutex, observe the valid event->rb pointer, and attempt to
increment its reference count. If the cleanup path has already
dropped the reference count to zero, this results in a
use-after-free or refcount saturation warning.

Fix this by extending the scope of mmap_mutex to cover the
map_range() call. This ensures that the ring buffer initialization
and mapping (or cleanup on failure) happens atomically effectively,
preventing other threads from accessing a half-initialized or
dying ring buffer.

v2:
Because expanding the guarded region would cause the event->mmap_mutex
to be acquired repeatedly in the perf_mmap_close function, potentially
leading to a self deadlock, the original logic of perf_mmap_close was
retained, and the mutex-holding logic was modified to obtain the
perf_mmap_close_locked function.

v3:
The fix is made smaller by passing the parameter "holds_event_mmap_mutex"
to perf_mmap_close.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202602020208.m7KIjdzW-lkp@intel.com/
Suggested-by: Ian Rogers <irogers@google.com>
Signed-off-by: Haocheng Yu <yuhaocheng035@gmail.com>
---
 kernel/events/core.c | 78 +++++++++++++++++++++++++++-----------------
 1 file changed, 48 insertions(+), 30 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 2c35acc2722b..a3228c587de1 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -6730,9 +6730,10 @@ static void perf_pmu_output_stop(struct perf_event *event);
  * the buffer here, where we still have a VM context. This means we need
  * to detach all events redirecting to us.
  */
-static void perf_mmap_close(struct vm_area_struct *vma)
+static void __perf_mmap_close(struct vm_area_struct *vma, struct perf_event *event,
+			      bool holds_event_mmap_lock)
 {
-	struct perf_event *event = vma->vm_file->private_data;
+	struct perf_event *iter_event;
 	mapped_f unmapped = get_mapped(event, event_unmapped);
 	struct perf_buffer *rb = ring_buffer_get(event);
 	struct user_struct *mmap_user = rb->mmap_user;
@@ -6772,11 +6773,14 @@ static void perf_mmap_close(struct vm_area_struct *vma)
 	if (refcount_dec_and_test(&rb->mmap_count))
 		detach_rest = true;
 
-	if (!refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex))
+	if ((!holds_event_mmap_lock &&
+	     !refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex)) ||
+	    (holds_event_mmap_lock && !refcount_dec_and_test(&event->mmap_count)))
 		goto out_put;
 
 	ring_buffer_attach(event, NULL);
-	mutex_unlock(&event->mmap_mutex);
+	if (!holds_event_mmap_lock)
+		mutex_unlock(&event->mmap_mutex);
 
 	/* If there's still other mmap()s of this buffer, we're done. */
 	if (!detach_rest)
@@ -6789,8 +6793,8 @@ static void perf_mmap_close(struct vm_area_struct *vma)
 	 */
 again:
 	rcu_read_lock();
-	list_for_each_entry_rcu(event, &rb->event_list, rb_entry) {
-		if (!atomic_long_inc_not_zero(&event->refcount)) {
+	list_for_each_entry_rcu(iter_event, &rb->event_list, rb_entry) {
+		if (!atomic_long_inc_not_zero(&iter_event->refcount)) {
 			/*
 			 * This event is en-route to free_event() which will
 			 * detach it and remove it from the list.
@@ -6799,7 +6803,8 @@ static void perf_mmap_close(struct vm_area_struct *vma)
 		}
 		rcu_read_unlock();
 
-		mutex_lock(&event->mmap_mutex);
+		if (!holds_event_mmap_lock)
+			mutex_lock(&iter_event->mmap_mutex);
 		/*
 		 * Check we didn't race with perf_event_set_output() which can
 		 * swizzle the rb from under us while we were waiting to
@@ -6810,11 +6815,12 @@ static void perf_mmap_close(struct vm_area_struct *vma)
 		 * still restart the iteration to make sure we're not now
 		 * iterating the wrong list.
 		 */
-		if (event->rb == rb)
-			ring_buffer_attach(event, NULL);
+		if (iter_event->rb == rb)
+			ring_buffer_attach(iter_event, NULL);
 
-		mutex_unlock(&event->mmap_mutex);
-		put_event(event);
+		if (!holds_event_mmap_lock)
+			mutex_unlock(&iter_event->mmap_mutex);
+		put_event(iter_event);
 
 		/*
 		 * Restart the iteration; either we're on the wrong list or
@@ -6842,6 +6848,18 @@ static void perf_mmap_close(struct vm_area_struct *vma)
 	ring_buffer_put(rb); /* could be last */
 }
 
+static void perf_mmap_close(struct vm_area_struct *vma)
+{
+	struct perf_event *event = vma->vm_file->private_data;
+
+	__perf_mmap_close(vma, event, false);
+}
+
+static void perf_mmap_close_locked(struct vm_area_struct *vma, struct perf_event *event)
+{
+	__perf_mmap_close(vma, event, true);
+}
+
 static vm_fault_t perf_mmap_pfn_mkwrite(struct vm_fault *vmf)
 {
 	/* The first page is the user control page, others are read-only. */
@@ -7167,28 +7185,28 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 			ret = perf_mmap_aux(vma, event, nr_pages);
 		if (ret)
 			return ret;
-	}
 
-	/*
-	 * Since pinned accounting is per vm we cannot allow fork() to copy our
-	 * vma.
-	 */
-	vm_flags_set(vma, VM_DONTCOPY | VM_DONTEXPAND | VM_DONTDUMP);
-	vma->vm_ops = &perf_mmap_vmops;
+		/*
+		 * Since pinned accounting is per vm we cannot allow fork() to copy our
+		 * vma.
+		 */
+		vm_flags_set(vma, VM_DONTCOPY | VM_DONTEXPAND | VM_DONTDUMP);
+		vma->vm_ops = &perf_mmap_vmops;
 
-	mapped = get_mapped(event, event_mapped);
-	if (mapped)
-		mapped(event, vma->vm_mm);
+		mapped = get_mapped(event, event_mapped);
+		if (mapped)
+			mapped(event, vma->vm_mm);
 
-	/*
-	 * Try to map it into the page table. On fail, invoke
-	 * perf_mmap_close() to undo the above, as the callsite expects
-	 * full cleanup in this case and therefore does not invoke
-	 * vmops::close().
-	 */
-	ret = map_range(event->rb, vma);
-	if (ret)
-		perf_mmap_close(vma);
+		/*
+		 * Try to map it into the page table. On fail, invoke
+		 * perf_mmap_close() to undo the above, as the callsite expects
+		 * full cleanup in this case and therefore does not invoke
+		 * vmops::close().
+		 */
+		ret = map_range(event->rb, vma);
+		if (ret)
+			perf_mmap_close_locked(vma, event);
+	}
 
 	return ret;
 }

base-commit: 7d0a66e4bb9081d75c82ec4957c50034cb0ea449
-- 
2.51.0

Re: [PATCH v3] perf/core: Fix refcount bug and potential UAF in perf_mmap

[Ian Rogers]

On Wed, Mar 25, 2026 at 3:21 AM <yuhaocheng035@gmail.com> wrote:
>
> From: Haocheng Yu <yuhaocheng035@gmail.com>
>
> Syzkaller reported a refcount_t: addition on 0; use-after-free warning
> in perf_mmap.
>
> The issue is caused by a race condition between a failing mmap() setup
> and a concurrent mmap() on a dependent event (e.g., using output
> redirection).
>
> In perf_mmap(), the ring_buffer (rb) is allocated and assigned to
> event->rb with the mmap_mutex held. The mutex is then released to
> perform map_range().
>
> If map_range() fails, perf_mmap_close() is called to clean up.
> However, since the mutex was dropped, another thread attaching to
> this event (via inherited events or output redirection) can acquire
> the mutex, observe the valid event->rb pointer, and attempt to
> increment its reference count. If the cleanup path has already
> dropped the reference count to zero, this results in a
> use-after-free or refcount saturation warning.
>
> Fix this by extending the scope of mmap_mutex to cover the
> map_range() call. This ensures that the ring buffer initialization
> and mapping (or cleanup on failure) happens atomically effectively,
> preventing other threads from accessing a half-initialized or
> dying ring buffer.
>
> v2:
> Because expanding the guarded region would cause the event->mmap_mutex
> to be acquired repeatedly in the perf_mmap_close function, potentially
> leading to a self deadlock, the original logic of perf_mmap_close was
> retained, and the mutex-holding logic was modified to obtain the
> perf_mmap_close_locked function.
>
> v3:
> The fix is made smaller by passing the parameter "holds_event_mmap_mutex"
> to perf_mmap_close.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202602020208.m7KIjdzW-lkp@intel.com/
> Suggested-by: Ian Rogers <irogers@google.com>
> Signed-off-by: Haocheng Yu <yuhaocheng035@gmail.com>

Reviewed-by: Ian Rogers <irogers@google.com>

Thanks,
Ian

> ---
>  kernel/events/core.c | 78 +++++++++++++++++++++++++++-----------------
>  1 file changed, 48 insertions(+), 30 deletions(-)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 2c35acc2722b..a3228c587de1 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -6730,9 +6730,10 @@ static void perf_pmu_output_stop(struct perf_event *event);
>   * the buffer here, where we still have a VM context. This means we need
>   * to detach all events redirecting to us.
>   */
> -static void perf_mmap_close(struct vm_area_struct *vma)
> +static void __perf_mmap_close(struct vm_area_struct *vma, struct perf_event *event,
> +                             bool holds_event_mmap_lock)
>  {
> -       struct perf_event *event = vma->vm_file->private_data;
> +       struct perf_event *iter_event;
>         mapped_f unmapped = get_mapped(event, event_unmapped);
>         struct perf_buffer *rb = ring_buffer_get(event);
>         struct user_struct *mmap_user = rb->mmap_user;
> @@ -6772,11 +6773,14 @@ static void perf_mmap_close(struct vm_area_struct *vma)
>         if (refcount_dec_and_test(&rb->mmap_count))
>                 detach_rest = true;
>
> -       if (!refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex))
> +       if ((!holds_event_mmap_lock &&
> +            !refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex)) ||
> +           (holds_event_mmap_lock && !refcount_dec_and_test(&event->mmap_count)))
>                 goto out_put;
>
>         ring_buffer_attach(event, NULL);
> -       mutex_unlock(&event->mmap_mutex);
> +       if (!holds_event_mmap_lock)
> +               mutex_unlock(&event->mmap_mutex);
>
>         /* If there's still other mmap()s of this buffer, we're done. */
>         if (!detach_rest)
> @@ -6789,8 +6793,8 @@ static void perf_mmap_close(struct vm_area_struct *vma)
>          */
>  again:
>         rcu_read_lock();
> -       list_for_each_entry_rcu(event, &rb->event_list, rb_entry) {
> -               if (!atomic_long_inc_not_zero(&event->refcount)) {
> +       list_for_each_entry_rcu(iter_event, &rb->event_list, rb_entry) {
> +               if (!atomic_long_inc_not_zero(&iter_event->refcount)) {
>                         /*
>                          * This event is en-route to free_event() which will
>                          * detach it and remove it from the list.
> @@ -6799,7 +6803,8 @@ static void perf_mmap_close(struct vm_area_struct *vma)
>                 }
>                 rcu_read_unlock();
>
> -               mutex_lock(&event->mmap_mutex);
> +               if (!holds_event_mmap_lock)
> +                       mutex_lock(&iter_event->mmap_mutex);
>                 /*
>                  * Check we didn't race with perf_event_set_output() which can
>                  * swizzle the rb from under us while we were waiting to
> @@ -6810,11 +6815,12 @@ static void perf_mmap_close(struct vm_area_struct *vma)
>                  * still restart the iteration to make sure we're not now
>                  * iterating the wrong list.
>                  */
> -               if (event->rb == rb)
> -                       ring_buffer_attach(event, NULL);
> +               if (iter_event->rb == rb)
> +                       ring_buffer_attach(iter_event, NULL);
>
> -               mutex_unlock(&event->mmap_mutex);
> -               put_event(event);
> +               if (!holds_event_mmap_lock)
> +                       mutex_unlock(&iter_event->mmap_mutex);
> +               put_event(iter_event);
>
>                 /*
>                  * Restart the iteration; either we're on the wrong list or
> @@ -6842,6 +6848,18 @@ static void perf_mmap_close(struct vm_area_struct *vma)
>         ring_buffer_put(rb); /* could be last */
>  }
>
> +static void perf_mmap_close(struct vm_area_struct *vma)
> +{
> +       struct perf_event *event = vma->vm_file->private_data;
> +
> +       __perf_mmap_close(vma, event, false);
> +}
> +
> +static void perf_mmap_close_locked(struct vm_area_struct *vma, struct perf_event *event)
> +{
> +       __perf_mmap_close(vma, event, true);
> +}
> +
>  static vm_fault_t perf_mmap_pfn_mkwrite(struct vm_fault *vmf)
>  {
>         /* The first page is the user control page, others are read-only. */
> @@ -7167,28 +7185,28 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
>                         ret = perf_mmap_aux(vma, event, nr_pages);
>                 if (ret)
>                         return ret;
> -       }
>
> -       /*
> -        * Since pinned accounting is per vm we cannot allow fork() to copy our
> -        * vma.
> -        */
> -       vm_flags_set(vma, VM_DONTCOPY | VM_DONTEXPAND | VM_DONTDUMP);
> -       vma->vm_ops = &perf_mmap_vmops;
> +               /*
> +                * Since pinned accounting is per vm we cannot allow fork() to copy our
> +                * vma.
> +                */
> +               vm_flags_set(vma, VM_DONTCOPY | VM_DONTEXPAND | VM_DONTDUMP);
> +               vma->vm_ops = &perf_mmap_vmops;
>
> -       mapped = get_mapped(event, event_mapped);
> -       if (mapped)
> -               mapped(event, vma->vm_mm);
> +               mapped = get_mapped(event, event_mapped);
> +               if (mapped)
> +                       mapped(event, vma->vm_mm);
>
> -       /*
> -        * Try to map it into the page table. On fail, invoke
> -        * perf_mmap_close() to undo the above, as the callsite expects
> -        * full cleanup in this case and therefore does not invoke
> -        * vmops::close().
> -        */
> -       ret = map_range(event->rb, vma);
> -       if (ret)
> -               perf_mmap_close(vma);
> +               /*
> +                * Try to map it into the page table. On fail, invoke
> +                * perf_mmap_close() to undo the above, as the callsite expects
> +                * full cleanup in this case and therefore does not invoke
> +                * vmops::close().
> +                */
> +               ret = map_range(event->rb, vma);
> +               if (ret)
> +                       perf_mmap_close_locked(vma, event);
> +       }
>
>         return ret;
>  }
>
> base-commit: 7d0a66e4bb9081d75c82ec4957c50034cb0ea449
> --
> 2.51.0
>

Re: [PATCH v3] perf/core: Fix refcount bug and potential UAF in perf_mmap

[Peter Zijlstra]

Argh,. why is this hidden in this old thread :/

On Wed, Mar 25, 2026 at 06:20:53PM +0800, yuhaocheng035@gmail.com wrote:

> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 2c35acc2722b..a3228c587de1 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -6730,9 +6730,10 @@ static void perf_pmu_output_stop(struct perf_event *event);
>   * the buffer here, where we still have a VM context. This means we need
>   * to detach all events redirecting to us.
>   */
> -static void perf_mmap_close(struct vm_area_struct *vma)
> +static void __perf_mmap_close(struct vm_area_struct *vma, struct perf_event *event,
> +			      bool holds_event_mmap_lock)
>  {
> -	struct perf_event *event = vma->vm_file->private_data;
> +	struct perf_event *iter_event;
>  	mapped_f unmapped = get_mapped(event, event_unmapped);
>  	struct perf_buffer *rb = ring_buffer_get(event);
>  	struct user_struct *mmap_user = rb->mmap_user;
> @@ -6772,11 +6773,14 @@ static void perf_mmap_close(struct vm_area_struct *vma)
>  	if (refcount_dec_and_test(&rb->mmap_count))
>  		detach_rest = true;
>  
> -	if (!refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex))
> +	if ((!holds_event_mmap_lock &&
> +	     !refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex)) ||
> +	    (holds_event_mmap_lock && !refcount_dec_and_test(&event->mmap_count)))
>  		goto out_put;

*groan*, this is horrible.

Let me have a poke to see if there isn't a saner variant around.

Re: [PATCH v3] perf/core: Fix refcount bug and potential UAF in perf_mmap

[Peter Zijlstra]

On Wed, Mar 25, 2026 at 04:17:35PM +0100, Peter Zijlstra wrote:
> 
> Argh,. why is this hidden in this old thread :/
> 
> On Wed, Mar 25, 2026 at 06:20:53PM +0800, yuhaocheng035@gmail.com wrote:
> 
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > index 2c35acc2722b..a3228c587de1 100644
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -6730,9 +6730,10 @@ static void perf_pmu_output_stop(struct perf_event *event);
> >   * the buffer here, where we still have a VM context. This means we need
> >   * to detach all events redirecting to us.
> >   */
> > -static void perf_mmap_close(struct vm_area_struct *vma)
> > +static void __perf_mmap_close(struct vm_area_struct *vma, struct perf_event *event,
> > +			      bool holds_event_mmap_lock)
> >  {
> > -	struct perf_event *event = vma->vm_file->private_data;
> > +	struct perf_event *iter_event;
> >  	mapped_f unmapped = get_mapped(event, event_unmapped);
> >  	struct perf_buffer *rb = ring_buffer_get(event);
> >  	struct user_struct *mmap_user = rb->mmap_user;
> > @@ -6772,11 +6773,14 @@ static void perf_mmap_close(struct vm_area_struct *vma)
> >  	if (refcount_dec_and_test(&rb->mmap_count))
> >  		detach_rest = true;
> >  
> > -	if (!refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex))
> > +	if ((!holds_event_mmap_lock &&
> > +	     !refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex)) ||
> > +	    (holds_event_mmap_lock && !refcount_dec_and_test(&event->mmap_count)))
> >  		goto out_put;
> 
> *groan*, this is horrible.
> 
> Let me have a poke to see if there isn't a saner variant around.

Also, I just realized this patch doesn't even apply, it is against a
tree without 77de62ad3de3 ("perf/core: Fix refcount bug and potential
UAF in perf_mmap").

Re: [PATCH v3] perf/core: Fix refcount bug and potential UAF in perf_mmap

[Qing Wang]

On Wed, 25 Mar 2026 at 23:17, Peter Zijlstra <peterz@infradead.org> wrote:
> Argh,. why is this hidden in this old thread :/
> 
> On Wed, Mar 25, 2026 at 06:20:53PM +0800, yuhaocheng035@gmail.com wrote:
> 
> > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > index 2c35acc2722b..a3228c587de1 100644
> > --- a/kernel/events/core.c
> > +++ b/kernel/events/core.c
> > @@ -6730,9 +6730,10 @@ static void perf_pmu_output_stop(struct perf_event *event);
> >   * the buffer here, where we still have a VM context. This means we need
> >   * to detach all events redirecting to us.
> >   */
> > -static void perf_mmap_close(struct vm_area_struct *vma)
> > +static void __perf_mmap_close(struct vm_area_struct *vma, struct perf_event *event,
> > +			      bool holds_event_mmap_lock)
> >  {
> > -	struct perf_event *event = vma->vm_file->private_data;
> > +	struct perf_event *iter_event;
> >  	mapped_f unmapped = get_mapped(event, event_unmapped);
> >  	struct perf_buffer *rb = ring_buffer_get(event);
> >  	struct user_struct *mmap_user = rb->mmap_user;
> > @@ -6772,11 +6773,14 @@ static void perf_mmap_close(struct vm_area_struct *vma)
> >  	if (refcount_dec_and_test(&rb->mmap_count))
> >  		detach_rest = true;
> >  
> > -	if (!refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex))
> > +	if ((!holds_event_mmap_lock &&
> > +	     !refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex)) ||
> > +	    (holds_event_mmap_lock && !refcount_dec_and_test(&event->mmap_count)))
> >  		goto out_put;
> 
> *groan*, this is horrible.
> 
> Let me have a poke to see if there isn't a saner variant around.

I think it's ok to move perf_mmap_close() outside the mutex lock, like this:

https://lore.kernel.org/all/20260325153240.GK3739106@noisy.programming.kicks-ass.net/T/#m0f82e8ecdfdfce4acd5121bcb799e864cf05ebf9

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 1f5699b339ec..e5ce03ce926d 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7485,9 +7485,12 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 		 */
 		ret = map_range(event->rb, vma);
 		if (ret)
-			perf_mmap_close(vma);
+			goto out_close;
 	}
+	return 0;
 
+out_close:
+	perf_mmap_close(vma);
 	return ret;
 }

How do you think?

--
Qing

Re: [PATCH v3] perf/core: Fix refcount bug and potential UAF in perf_mmap

[Peter Zijlstra]

On Thu, Mar 26, 2026 at 11:18:06AM +0800, Qing Wang wrote:
> On Wed, 25 Mar 2026 at 23:17, Peter Zijlstra <peterz@infradead.org> wrote:
> > Argh,. why is this hidden in this old thread :/
> > 
> > On Wed, Mar 25, 2026 at 06:20:53PM +0800, yuhaocheng035@gmail.com wrote:
> > 
> > > diff --git a/kernel/events/core.c b/kernel/events/core.c
> > > index 2c35acc2722b..a3228c587de1 100644
> > > --- a/kernel/events/core.c
> > > +++ b/kernel/events/core.c
> > > @@ -6730,9 +6730,10 @@ static void perf_pmu_output_stop(struct perf_event *event);
> > >   * the buffer here, where we still have a VM context. This means we need
> > >   * to detach all events redirecting to us.
> > >   */
> > > -static void perf_mmap_close(struct vm_area_struct *vma)
> > > +static void __perf_mmap_close(struct vm_area_struct *vma, struct perf_event *event,
> > > +			      bool holds_event_mmap_lock)
> > >  {
> > > -	struct perf_event *event = vma->vm_file->private_data;
> > > +	struct perf_event *iter_event;
> > >  	mapped_f unmapped = get_mapped(event, event_unmapped);
> > >  	struct perf_buffer *rb = ring_buffer_get(event);
> > >  	struct user_struct *mmap_user = rb->mmap_user;
> > > @@ -6772,11 +6773,14 @@ static void perf_mmap_close(struct vm_area_struct *vma)
> > >  	if (refcount_dec_and_test(&rb->mmap_count))
> > >  		detach_rest = true;
> > >  
> > > -	if (!refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex))
> > > +	if ((!holds_event_mmap_lock &&
> > > +	     !refcount_dec_and_mutex_lock(&event->mmap_count, &event->mmap_mutex)) ||
> > > +	    (holds_event_mmap_lock && !refcount_dec_and_test(&event->mmap_count)))
> > >  		goto out_put;
> > 
> > *groan*, this is horrible.
> > 
> > Let me have a poke to see if there isn't a saner variant around.
> 
> I think it's ok to move perf_mmap_close() outside the mutex lock, like this:
> 
> https://lore.kernel.org/all/20260325153240.GK3739106@noisy.programming.kicks-ass.net/T/#m0f82e8ecdfdfce4acd5121bcb799e864cf05ebf9
> 
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 1f5699b339ec..e5ce03ce926d 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7485,9 +7485,12 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
>  		 */
>  		ret = map_range(event->rb, vma);
>  		if (ret)
> -			perf_mmap_close(vma);
> +			goto out_close;
>  	}
> +	return 0;
>  
> +out_close:
> +	perf_mmap_close(vma);
>  	return ret;
>  }
> 
> How do you think?

Well, that will just re-introduce the original problem. As you were told
there.

What about something like this?

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 1f5699b339ec..0bb1d8b83bc9 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7010,6 +7010,7 @@ static void perf_mmap_open(struct vm_area_struct *vma)
 }
 
 static void perf_pmu_output_stop(struct perf_event *event);
+static void perf_mmap_unaccount(struct vm_area_struct *vma, struct perf_buffer *rb);
 
 /*
  * A buffer can be mmap()ed multiple times; either directly through the same
@@ -7025,8 +7026,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;
-	int mmap_locked = rb->mmap_locked;
-	unsigned long size = perf_data_size(rb);
 	bool detach_rest = false;
 
 	/* FIXIES vs perf_pmu_unregister() */
@@ -7121,11 +7120,7 @@ static void perf_mmap_close(struct vm_area_struct *vma)
 	 * Aside from that, this buffer is 'fully' detached and unmapped,
 	 * undo the VM accounting.
 	 */
-
-	atomic_long_sub((size >> PAGE_SHIFT) + 1 - mmap_locked,
-			&mmap_user->locked_vm);
-	atomic64_sub(mmap_locked, &vma->vm_mm->pinned_vm);
-	free_uid(mmap_user);
+	perf_mmap_unaccount(vma, rb);
 
 out_put:
 	ring_buffer_put(rb); /* could be last */
@@ -7265,6 +7260,15 @@ static void perf_mmap_account(struct vm_area_struct *vma, long user_extra, long
 	atomic64_add(extra, &vma->vm_mm->pinned_vm);
 }
 
+static void perf_mmap_unaccount(struct vm_area_struct *vma, struct perf_buffer *rb)
+{
+	struct user_struct *user = rb->mmap_user;
+
+	atomic_long_sub((perf_data_size(rb) >> PAGE_SHIFT) + 1 - rb->mmap_locked,
+			&user->locked_vm);
+	atomic64_sub(rb->mmap_locked, &vma->vm_mm->pinned_vm);
+}
+
 static int perf_mmap_rb(struct vm_area_struct *vma, struct perf_event *event,
 			unsigned long nr_pages)
 {
@@ -7327,8 +7331,6 @@ static int perf_mmap_rb(struct vm_area_struct *vma, struct perf_event *event,
 	if (!rb)
 		return -ENOMEM;
 
-	refcount_set(&rb->mmap_count, 1);
-	rb->mmap_user = get_current_user();
 	rb->mmap_locked = extra;
 
 	ring_buffer_attach(event, rb);
@@ -7484,10 +7486,43 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 		 * vmops::close().
 		 */
 		ret = map_range(event->rb, vma);
-		if (ret)
-			perf_mmap_close(vma);
+		if (likely(!ret))
+			return 0;
+
+		/* Error path */
+
+		/*
+		 * If this is the first mmap(), then event->mmap_count should
+		 * be stable at 1. It is only modified by:
+		 * perf_mmap_{open,close}() and perf_mmap().
+		 *
+		 * The former are not possible because this mmap() hasn't been
+		 * successful yet, and the latter is serialized by
+		 * event->mmap_mutex which we still hold (note that mmap_lock
+		 * is not strictly sufficient here, because the event fd can
+		 * be passed to another process through trivial means like
+		 * fork(), leading to concurrent mmap() from different mm).
+		 *
+		 * Make sure to remove event->rb before releasing
+		 * event->mmap_mutex, such that any concurrent mmap() will not
+		 * attempt use this failed buffer.
+		 */
+		if (refcount_read(&event->mmap_count) == 1) {
+			/*
+			 * Minimal perf_mmap_close(); there can't be AUX or
+			 * other events on account of this being the first.
+			 */
+			mapped = get_mapped(event, event_unmapped);
+			if (mapped)
+				mapped(event, vma->vm_mm);
+			perf_mmap_unaccount(vma, event->rb);
+			ring_buffer_attach(event, NULL);	/* drops last rb->refcount */
+			refcount_set(&event->mmap_count, 0);
+			return ret;
+		}
 	}
 
+	perf_mmap_close(vma);
 	return ret;
 }
 
diff --git a/kernel/events/internal.h b/kernel/events/internal.h
index d9cc57083091..c03c4f2eea57 100644
--- a/kernel/events/internal.h
+++ b/kernel/events/internal.h
@@ -67,6 +67,7 @@ static inline void rb_free_rcu(struct rcu_head *rcu_head)
 	struct perf_buffer *rb;
 
 	rb = container_of(rcu_head, struct perf_buffer, rcu_head);
+	free_uid(rb->mmap_user);
 	rb_free(rb);
 }
 
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index 3e7de2661417..9fe92161715e 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -340,6 +340,8 @@ ring_buffer_init(struct perf_buffer *rb, long watermark, int flags)
 		rb->paused = 1;
 
 	mutex_init(&rb->aux_mutex);
+	rb->mmap_user = get_current_user();
+	refcount_set(&rb->mmap_count, 1);
 }
 
 void perf_aux_output_flag(struct perf_output_handle *handle, u64 flags)

[PATCH v4] perf/core: Fix refcount bug and potential UAF in perf_mmap

[yuhaocheng035]

From: Haocheng Yu <yuhaocheng035@gmail.com>

Syzkaller reported a refcount_t: addition on 0; use-after-free warning
in perf_mmap.

The issue is caused by a race condition between a failing mmap() setup
and a concurrent mmap() on a dependent event (e.g., using output
redirection).

In perf_mmap(), the ring_buffer (rb) is allocated and assigned to
event->rb with the mmap_mutex held. The mutex is then released to
perform map_range().

If map_range() fails, perf_mmap_close() is called to clean up.
However, since the mutex was dropped, another thread attaching to
this event (via inherited events or output redirection) can acquire
the mutex, observe the valid event->rb pointer, and attempt to
increment its reference count. If the cleanup path has already
dropped the reference count to zero, this results in a
use-after-free or refcount saturation warning.

Fix this by extending the scope of mmap_mutex to cover the
map_range() call. This ensures that the ring buffer initialization
and mapping (or cleanup on failure) happens atomically effectively,
preventing other threads from accessing a half-initialized or
dying ring buffer.

v2:
Because expanding the guarded region would cause the event->mmap_mutex
to be acquired repeatedly in the perf_mmap_close function, potentially
leading to a self deadlock, the original logic of perf_mmap_close was
retained, and the mutex-holding logic was modified to obtain the
perf_mmap_close_locked function.

v3:
The fix is made smaller by passing the parameter "holds_event_mmap_mutex"
to perf_mmap_close.

v4:
This problem is solved in a smarter way.

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/oe-kbuild-all/202602020208.m7KIjdzW-lkp@intel.com/
Reviewed-by: Ian Rogers <irogers@google.com>
Reviewed-by: Peter Zijlstra <peterz@infradead.org>
Signed-off-by: Haocheng Yu <yuhaocheng035@gmail.com>
---
 kernel/events/core.c        | 59 +++++++++++++++++++++++++++++--------
 kernel/events/internal.h    |  1 +
 kernel/events/ring_buffer.c |  2 ++
 3 files changed, 49 insertions(+), 13 deletions(-)

diff --git a/kernel/events/core.c b/kernel/events/core.c
index 22a0f405585b..d3f978402b1e 100644
--- a/kernel/events/core.c
+++ b/kernel/events/core.c
@@ -7010,7 +7010,7 @@ static void perf_mmap_open(struct vm_area_struct *vma)
 }
 
 static void perf_pmu_output_stop(struct perf_event *event);
-
+static void perf_mmap_unaccount(struct vm_area_struct *vma, struct perf_buffer *rb);
 /*
  * A buffer can be mmap()ed multiple times; either directly through the same
  * event, or through other events by use of perf_event_set_output().
@@ -7025,8 +7025,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;
-	int mmap_locked = rb->mmap_locked;
-	unsigned long size = perf_data_size(rb);
 	bool detach_rest = false;
 
 	/* FIXIES vs perf_pmu_unregister() */
@@ -7121,11 +7119,7 @@ static void perf_mmap_close(struct vm_area_struct *vma)
 	 * Aside from that, this buffer is 'fully' detached and unmapped,
 	 * undo the VM accounting.
 	 */
-
-	atomic_long_sub((size >> PAGE_SHIFT) + 1 - mmap_locked,
-			&mmap_user->locked_vm);
-	atomic64_sub(mmap_locked, &vma->vm_mm->pinned_vm);
-	free_uid(mmap_user);
+	perf_mmap_unaccount(vma, rb);
 
 out_put:
 	ring_buffer_put(rb); /* could be last */
@@ -7265,6 +7259,15 @@ static void perf_mmap_account(struct vm_area_struct *vma, long user_extra, long
 	atomic64_add(extra, &vma->vm_mm->pinned_vm);
 }
 
+static void perf_mmap_unaccount(struct vm_area_struct *vma, struct perf_buffer *rb)
+{
+	struct user_struct *user = rb->mmap_user;
+
+	atomic_long_sub((perf_data_size(rb) >> PAGE_SHIFT) + 1 - rb->mmap_locked,
+			&user->locked_vm);
+	atomic64_sub(rb->mmap_locked, &vma->vm_mm->pinned_vm);
+}
+
 static int perf_mmap_rb(struct vm_area_struct *vma, struct perf_event *event,
 			unsigned long nr_pages)
 {
@@ -7327,8 +7330,6 @@ static int perf_mmap_rb(struct vm_area_struct *vma, struct perf_event *event,
 	if (!rb)
 		return -ENOMEM;
 
-	refcount_set(&rb->mmap_count, 1);
-	rb->mmap_user = get_current_user();
 	rb->mmap_locked = extra;
 
 	ring_buffer_attach(event, rb);
@@ -7484,10 +7485,42 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
 		 * vmops::close().
 		 */
 		ret = map_range(event->rb, vma);
-		if (ret)
-			perf_mmap_close(vma);
-	}
+		if (likely(!ret))
+			return 0;
+
+		/* Error path */
 
+		/*
+		 * If this is the first mmap(), then event->mmap_count should
+		 * be stable at 1. It is only modified by:
+		 * perf_mmap_{open,close}() and perf_mmap().
+		 *
+		 * The former are not possible because this mmap() hasn't been
+		 * successful yet, and the latter is serialized by
+		 * event->mmap_mutex which we still hold (note that mmap_lock
+		 * is not strictly sufficient here, because the event fd can
+		 * be passed to another process through trivial means like
+		 * fork(), leading to concurrent mmap() from different mm).
+		 *
+		 * Make sure to remove event->rb before releasing
+		 * event->mmap_mutex, such that any concurrent mmap() will not
+		 * attempt use this failed buffer.
+		 */
+		if (refcount_read(&event->mmap_count) == 1) {
+			/*
+			 * Minimal perf_mmap_close(); there can't be AUX or
+			 * other events on account of this being the first.
+			 */
+			mapped = get_mapped(event, event_unmapped);
+			if (mapped)
+				mapped(event, vma->vm_mm);
+			perf_mmap_unaccount(vma, event->rb);
+			ring_buffer_attach(event, NULL);	/* drops last rb->refcount */
+			refcount_set(&event->mmap_count, 0);
+			return ret;
+		}
+	}
+	perf_mmap_close(vma);
 	return ret;
 }
 
diff --git a/kernel/events/internal.h b/kernel/events/internal.h
index d9cc57083091..c03c4f2eea57 100644
--- a/kernel/events/internal.h
+++ b/kernel/events/internal.h
@@ -67,6 +67,7 @@ static inline void rb_free_rcu(struct rcu_head *rcu_head)
 	struct perf_buffer *rb;
 
 	rb = container_of(rcu_head, struct perf_buffer, rcu_head);
+	free_uid(rb->mmap_user);
 	rb_free(rb);
 }
 
diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
index 3e7de2661417..9fe92161715e 100644
--- a/kernel/events/ring_buffer.c
+++ b/kernel/events/ring_buffer.c
@@ -340,6 +340,8 @@ ring_buffer_init(struct perf_buffer *rb, long watermark, int flags)
 		rb->paused = 1;
 
 	mutex_init(&rb->aux_mutex);
+	rb->mmap_user = get_current_user();
+	refcount_set(&rb->mmap_count, 1);
 }
 
 void perf_aux_output_flag(struct perf_output_handle *handle, u64 flags)

base-commit: 77de62ad3de3967818c3dbe656b7336ebee461d2
-- 
2.51.0

Re: [PATCH v4] perf/core: Fix refcount bug and potential UAF in perf_mmap

[Haocheng Yu]

Your solution looks much better.

I tried to incorporate it and submit a patch v4. If you are already
handling it, please ignore my patch.

Thanks,
Haocheng

> From: Haocheng Yu <yuhaocheng035@gmail.com>
>
> Syzkaller reported a refcount_t: addition on 0; use-after-free warning
> in perf_mmap.
>
> The issue is caused by a race condition between a failing mmap() setup
> and a concurrent mmap() on a dependent event (e.g., using output
> redirection).
>
> In perf_mmap(), the ring_buffer (rb) is allocated and assigned to
> event->rb with the mmap_mutex held. The mutex is then released to
> perform map_range().
>
> If map_range() fails, perf_mmap_close() is called to clean up.
> However, since the mutex was dropped, another thread attaching to
> this event (via inherited events or output redirection) can acquire
> the mutex, observe the valid event->rb pointer, and attempt to
> increment its reference count. If the cleanup path has already
> dropped the reference count to zero, this results in a
> use-after-free or refcount saturation warning.
>
> Fix this by extending the scope of mmap_mutex to cover the
> map_range() call. This ensures that the ring buffer initialization
> and mapping (or cleanup on failure) happens atomically effectively,
> preventing other threads from accessing a half-initialized or
> dying ring buffer.
>
> v2:
> Because expanding the guarded region would cause the event->mmap_mutex
> to be acquired repeatedly in the perf_mmap_close function, potentially
> leading to a self deadlock, the original logic of perf_mmap_close was
> retained, and the mutex-holding logic was modified to obtain the
> perf_mmap_close_locked function.
>
> v3:
> The fix is made smaller by passing the parameter "holds_event_mmap_mutex"
> to perf_mmap_close.
>
> v4:
> This problem is solved in a smarter way.
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202602020208.m7KIjdzW-lkp@intel.com/
> Reviewed-by: Ian Rogers <irogers@google.com>
> Reviewed-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Haocheng Yu <yuhaocheng035@gmail.com>
> ---
>  kernel/events/core.c        | 59 +++++++++++++++++++++++++++++--------
>  kernel/events/internal.h    |  1 +
>  kernel/events/ring_buffer.c |  2 ++
>  3 files changed, 49 insertions(+), 13 deletions(-)
>
> diff --git a/kernel/events/core.c b/kernel/events/core.c
> index 22a0f405585b..d3f978402b1e 100644
> --- a/kernel/events/core.c
> +++ b/kernel/events/core.c
> @@ -7010,7 +7010,7 @@ static void perf_mmap_open(struct vm_area_struct *vma)
>  }
>
>  static void perf_pmu_output_stop(struct perf_event *event);
> -
> +static void perf_mmap_unaccount(struct vm_area_struct *vma, struct perf_buffer *rb);
>  /*
>   * A buffer can be mmap()ed multiple times; either directly through the same
>   * event, or through other events by use of perf_event_set_output().
> @@ -7025,8 +7025,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;
> -       int mmap_locked = rb->mmap_locked;
> -       unsigned long size = perf_data_size(rb);
>         bool detach_rest = false;
>
>         /* FIXIES vs perf_pmu_unregister() */
> @@ -7121,11 +7119,7 @@ static void perf_mmap_close(struct vm_area_struct *vma)
>          * Aside from that, this buffer is 'fully' detached and unmapped,
>          * undo the VM accounting.
>          */
> -
> -       atomic_long_sub((size >> PAGE_SHIFT) + 1 - mmap_locked,
> -                       &mmap_user->locked_vm);
> -       atomic64_sub(mmap_locked, &vma->vm_mm->pinned_vm);
> -       free_uid(mmap_user);
> +       perf_mmap_unaccount(vma, rb);
>
>  out_put:
>         ring_buffer_put(rb); /* could be last */
> @@ -7265,6 +7259,15 @@ static void perf_mmap_account(struct vm_area_struct *vma, long user_extra, long
>         atomic64_add(extra, &vma->vm_mm->pinned_vm);
>  }
>
> +static void perf_mmap_unaccount(struct vm_area_struct *vma, struct perf_buffer *rb)
> +{
> +       struct user_struct *user = rb->mmap_user;
> +
> +       atomic_long_sub((perf_data_size(rb) >> PAGE_SHIFT) + 1 - rb->mmap_locked,
> +                       &user->locked_vm);
> +       atomic64_sub(rb->mmap_locked, &vma->vm_mm->pinned_vm);
> +}
> +
>  static int perf_mmap_rb(struct vm_area_struct *vma, struct perf_event *event,
>                         unsigned long nr_pages)
>  {
> @@ -7327,8 +7330,6 @@ static int perf_mmap_rb(struct vm_area_struct *vma, struct perf_event *event,
>         if (!rb)
>                 return -ENOMEM;
>
> -       refcount_set(&rb->mmap_count, 1);
> -       rb->mmap_user = get_current_user();
>         rb->mmap_locked = extra;
>
>         ring_buffer_attach(event, rb);
> @@ -7484,10 +7485,42 @@ static int perf_mmap(struct file *file, struct vm_area_struct *vma)
>                  * vmops::close().
>                  */
>                 ret = map_range(event->rb, vma);
> -               if (ret)
> -                       perf_mmap_close(vma);
> -       }
> +               if (likely(!ret))
> +                       return 0;
> +
> +               /* Error path */
>
> +               /*
> +                * If this is the first mmap(), then event->mmap_count should
> +                * be stable at 1. It is only modified by:
> +                * perf_mmap_{open,close}() and perf_mmap().
> +                *
> +                * The former are not possible because this mmap() hasn't been
> +                * successful yet, and the latter is serialized by
> +                * event->mmap_mutex which we still hold (note that mmap_lock
> +                * is not strictly sufficient here, because the event fd can
> +                * be passed to another process through trivial means like
> +                * fork(), leading to concurrent mmap() from different mm).
> +                *
> +                * Make sure to remove event->rb before releasing
> +                * event->mmap_mutex, such that any concurrent mmap() will not
> +                * attempt use this failed buffer.
> +                */
> +               if (refcount_read(&event->mmap_count) == 1) {
> +                       /*
> +                        * Minimal perf_mmap_close(); there can't be AUX or
> +                        * other events on account of this being the first.
> +                        */
> +                       mapped = get_mapped(event, event_unmapped);
> +                       if (mapped)
> +                               mapped(event, vma->vm_mm);
> +                       perf_mmap_unaccount(vma, event->rb);
> +                       ring_buffer_attach(event, NULL);        /* drops last rb->refcount */
> +                       refcount_set(&event->mmap_count, 0);
> +                       return ret;
> +               }
> +       }
> +       perf_mmap_close(vma);
>         return ret;
>  }
>
> diff --git a/kernel/events/internal.h b/kernel/events/internal.h
> index d9cc57083091..c03c4f2eea57 100644
> --- a/kernel/events/internal.h
> +++ b/kernel/events/internal.h
> @@ -67,6 +67,7 @@ static inline void rb_free_rcu(struct rcu_head *rcu_head)
>         struct perf_buffer *rb;
>
>         rb = container_of(rcu_head, struct perf_buffer, rcu_head);
> +       free_uid(rb->mmap_user);
>         rb_free(rb);
>  }
>
> diff --git a/kernel/events/ring_buffer.c b/kernel/events/ring_buffer.c
> index 3e7de2661417..9fe92161715e 100644
> --- a/kernel/events/ring_buffer.c
> +++ b/kernel/events/ring_buffer.c
> @@ -340,6 +340,8 @@ ring_buffer_init(struct perf_buffer *rb, long watermark, int flags)
>                 rb->paused = 1;
>
>         mutex_init(&rb->aux_mutex);
> +       rb->mmap_user = get_current_user();
> +       refcount_set(&rb->mmap_count, 1);
>  }
>
>  void perf_aux_output_flag(struct perf_output_handle *handle, u64 flags)
>
> base-commit: 77de62ad3de3967818c3dbe656b7336ebee461d2
> --
> 2.51.0
>

Re: [PATCH v4] perf/core: Fix refcount bug and potential UAF in perf_mmap

[Peter Zijlstra]

On Fri, Mar 27, 2026 at 08:29:52PM +0800, yuhaocheng035@gmail.com wrote:
> From: Haocheng Yu <yuhaocheng035@gmail.com>
> 
> Syzkaller reported a refcount_t: addition on 0; use-after-free warning
> in perf_mmap.
> 
> The issue is caused by a race condition between a failing mmap() setup
> and a concurrent mmap() on a dependent event (e.g., using output
> redirection).
> 
> In perf_mmap(), the ring_buffer (rb) is allocated and assigned to
> event->rb with the mmap_mutex held. The mutex is then released to
> perform map_range().
> 
> If map_range() fails, perf_mmap_close() is called to clean up.
> However, since the mutex was dropped, another thread attaching to
> this event (via inherited events or output redirection) can acquire
> the mutex, observe the valid event->rb pointer, and attempt to
> increment its reference count. If the cleanup path has already
> dropped the reference count to zero, this results in a
> use-after-free or refcount saturation warning.
> 
> Fix this by extending the scope of mmap_mutex to cover the
> map_range() call. This ensures that the ring buffer initialization
> and mapping (or cleanup on failure) happens atomically effectively,
> preventing other threads from accessing a half-initialized or
> dying ring buffer.
> 
> v2:
> Because expanding the guarded region would cause the event->mmap_mutex
> to be acquired repeatedly in the perf_mmap_close function, potentially
> leading to a self deadlock, the original logic of perf_mmap_close was
> retained, and the mutex-holding logic was modified to obtain the
> perf_mmap_close_locked function.
> 
> v3:
> The fix is made smaller by passing the parameter "holds_event_mmap_mutex"
> to perf_mmap_close.
> 
> v4:
> This problem is solved in a smarter way.
> 
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/oe-kbuild-all/202602020208.m7KIjdzW-lkp@intel.com/
> Reviewed-by: Ian Rogers <irogers@google.com>
> Reviewed-by: Peter Zijlstra <peterz@infradead.org>
> Signed-off-by: Haocheng Yu <yuhaocheng035@gmail.com>

You can't claim this as your patch. I was the one who wrote it --
yesterday.