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

[Ian Rogers]

On Mon, Mar 9, 2026 at 8:37 PM Qing Wang <wangqing7171@xxxxxxxxx> wrote:
>
> On Tue, 10 Mar 2026 at 02:59, Ian Rogers <irogers@xxxxxxxxxx> 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@xxxxxxxxxxxxxx/
> > 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@xxxxxxxxx/
> > 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@xxxxxxxxxxxxxx/
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