Re: [PATCH bpf-next v2] bpf: Fix mmap_lock leak in irq_work path

Andrii Nakryiko <[email protected]> Mon, 3 Aug 2026 10:44:04 -0700
Newsgroups org.kernel.vger.bpf,org.kernel.vger.linux-kernel
Message-ID <CAEf4BzZ703vZPOFobvSLskKA2DzQXpMcDd1jEaydGEwjx6O+sQ@mail.gmail.com>
On Mon, Aug 3, 2026 at 2:44=E2=80=AFAM Sanghyun Park
<[email protected]> wrote:
>
> > maybe we should make bpf_mmap_unlock_get_irq_work() return `struct
> > mmap_unlock_irq_work *` which would be ERR_PTR(), and return
> > ERR_PTR(-EBUSY) if (active || irq_work_is_busy) (see below)? I'd
> > probably call it bpf_mmap_unlock_guard_get(), though. And then we will
> > have bpf_mmap_unlock_guard_put() (instead of
> > bpf_mmap_unlock_put_irq_work) with the same logic.
>
> Yes, I think this would make the code a bit more straightforward.
>
> > we drop active to zero here, but irq_work_is_busy() can still return
> > true, I think we do need to check both
>
> Yes, BUSY can still be set after active is cleared. However, since the wo=
rk can
> be queued again while the callback is still running, I thought active alo=
ne was
> enough. I also tested it in a widened window, and didn't see a lock leak.
>
> But I may be missing something. Could requeueing the work here cause a pr=
oblem,
> or is this mainly to keep the original -EBUSY behavior?

I'm just not sure that re-queueing works if irq_work_is_busy() returns
true, please double check. If that doesn't work, then we can miss the
irq_work callback, which would be bad.