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.