perf: Fix deadlock in perf_mmap()

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 [email protected]

Closes https://syzkaller.appspot.com/bug?extid=196a82fd904572696b3c

Signed-off-by Qing Wang <[email protected]>

---
 kernel/events/core.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

kernel/events/core.c +4 -2

index 1f5699b339ec..e5ce03ce926d 100644
@@ -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

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/[email protected]/
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

Quoted 6 lines · Ian Rogers

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/[email protected]/
that should handle both the original race condition and the deadlock.

Oh, this fix is too large.

Quoted 26 lines

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);
}
```

Quoted 2 lines

Thanks,
Ian

LGTM, your fix is better.

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

Thanks,
Qing

Quoted 37 lines · Qing Wang

Quoted 6 lines · Ian Rogers

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/[email protected]/
that should handle both the original race condition and the deadlock.

Oh, this fix is too large.

Quoted 26 lines

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);
}
```

Quoted 6 lines

Quoted 2 lines

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

Quoted 2 lines

Thanks,
Qing

Quoted 48 lines · Ian Rogers

Quoted 37 lines · Qing Wang

Quoted 6 lines · Ian Rogers

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/[email protected]/
that should handle both the original race condition and the deadlock.

Oh, this fix is too large.

Quoted 26 lines

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);
}
```

Quoted 6 lines

Quoted 2 lines

Thanks,
Ian

LGTM, your fix is better.

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

Hi Qing,

Quoted 7 lines

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

Quoted 5 lines

Thanks,
Ian

Quoted 2 lines

Thanks,
Qing

From: Haocheng Yu <[email protected]>

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 <[email protected]>

Closes https://lore.kernel.org/oe-kbuild-all/[email protected]/ open here

Suggested-by Ian Rogers <[email protected]>

Signed-off-by Haocheng Yu <[email protected]>

---
 kernel/events/core.c | 78 +++++++++++++++++++++++++++-----------------
 1 file changed, 48 insertions(+), 30 deletions(-)

kernel/events/core.c +48 -30

index 2c35acc2722b..a3228c587de1 100644
@@ -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

kernel/events/core.c +3 -2

index 2c35acc2722b..a3228c587de1 100644
@@ -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;

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

Quoted 18 lines · On Wed, Mar 25, 2026 at 06:20:53PM +0800, [email protected] wrote:

kernel/events/core.c +3 -2

index 2c35acc2722b..a3228c587de1 100644
@@ -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;

kernel/events/core.c static void perf_mmap_close(struct vm_area_struct *vma) · line 6779

Quoted 8 lines

kernel/events/core.c +3 -1

@@ -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.

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

Quoted 18 lines · On Wed, Mar 25, 2026 at 06:20:53PM +0800, [email protected] wrote:

kernel/events/core.c +3 -2

index 2c35acc2722b..a3228c587de1 100644
@@ -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;

kernel/events/core.c static void perf_mmap_close(struct vm_area_struct *vma) · line 6779

Quoted 8 lines

kernel/events/core.c +3 -1

@@ -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.

Quoted 33 lines · Peter Zijlstra

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

Quoted 18 lines · On Wed, Mar 25, 2026 at 06:20:53PM +0800, [email protected] wrote:

kernel/events/core.c +3 -2

index 2c35acc2722b..a3228c587de1 100644
@@ -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;

kernel/events/core.c static void perf_mmap_close(struct vm_area_struct *vma) · line 6779

Quoted 8 lines

kernel/events/core.c +3 -1

@@ -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.

Quoted 1 line

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/[email protected]/T/#m0f82e8ecdfdfce4acd5121bcb799e864cf05ebf9

kernel/events/core.c +4 -1

index 1f5699b339ec..e5ce03ce926d 100644
@@ -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?

From: Haocheng Yu <[email protected]>

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 <[email protected]>

Closes https://lore.kernel.org/oe-kbuild-all/[email protected]/ open here

Reviewed-by Ian Rogers <[email protected]>

Reviewed-by Peter Zijlstra <[email protected]>

Signed-off-by Haocheng Yu <[email protected]>

---
 kernel/events/core.c        | 59 +++++++++++++++++++++++++++++--------
 kernel/events/internal.h    |  1 +
 kernel/events/ring_buffer.c |  2 ++
 3 files changed, 49 insertions(+), 13 deletions(-)

kernel/events/core.c +46 -13

index 22a0f405585b..d3f978402b1e 100644
@@ -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;
 }
 

kernel/events/internal.h +1 -0

index d9cc57083091..c03c4f2eea57 100644
@@ -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);
 }
 

kernel/events/ring_buffer.c +2 -0

index 3e7de2661417..9fe92161715e 100644
@@ -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

From: Haocheng Yu <[email protected]>

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 <[email protected]>

Closes https://lore.kernel.org/oe-kbuild-all/[email protected]/ open here

Reviewed-by Ian Rogers <[email protected]>

Reviewed-by Peter Zijlstra <[email protected]>

Signed-off-by Haocheng Yu <[email protected]>