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@xxxxxxxxxx> 于2026年3月25日周三 02:38写道：
>
> On Mon, Mar 9, 2026 at 9:45 PM Ian Rogers <irogers@xxxxxxxxxx> wrote:
> >
> > 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.
>
> 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