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@xxxxxxxxxxxxx> wrote:
> Argh,. why is this hidden in this old thread :/
> 
> On Wed, Mar 25, 2026 at 06:20:53PM +0800, yuhaocheng035@xxxxxxxxx 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@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/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