[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAEf4BzbtzHsa8DASzOg-Xqp8_-vG5ekC7JXhwuyZqPhrckU1hA@mail.gmail.com>
Date: Mon, 20 Oct 2025 09:17:18 -0700
From: Andrii Nakryiko <andrii.nakryiko@...il.com>
To: Noorain Eqbal <nooraineqbal@...il.com>
Cc: alexei.starovoitov@...il.com, andrii@...nel.org, ast@...nel.org,
bpf@...r.kernel.org, daniel@...earbox.net, david.hunter@...uxfoundation.org,
eddyz87@...il.com, haoluo@...gle.com, john.fastabend@...il.com,
jolsa@...nel.org, kpsingh@...nel.org, linux-kernel-mentees@...ts.linux.dev,
linux-kernel@...r.kernel.org, martin.lau@...ux.dev, sdf@...ichev.me,
skhan@...uxfoundation.org, song@...nel.org,
syzbot+2617fc732430968b45d2@...kaller.appspotmail.com,
yonghong.song@...ux.dev
Subject: Re: [PATCH] bpf: sync pending IRQ work before freeing ring buffer
On Sun, Oct 19, 2025 at 3:30 PM Noorain Eqbal <nooraineqbal@...il.com> wrote:
>
> On Sat, Oct 19, 2025 at 1:13 UTC, Alexei Starovoitov wrote:
> > Why do you think irq_work_run_list() processes bpf ringbuf in
> > the above splat?
>
> In the syzbot reproducer, GDB shows that when bpf_ringbuf_free() is entered
> the ring buffer's irq_work was still pending when the map was being freed.
>
> (gdb) p rb->work
> $5 = {
> node = {llist = {next = 0xffffffff8dc055c0 <wake_up_kfence_timer_work>},
> {u_flags = 35, a_flags = {counter = 35}}},
> func = 0xffffffff8223ac60 <bpf_ringbuf_notify>,
> irqwait = {task = 0x0}
> }
>
> Here, `u_flags = 0x23` indicates IRQ_WORK_PENDING and IRQ_WORK_BUSY
> are set, which shows that irq_work for the ring buffer was still queued
> at the time of free. This confirms that `irq_work_run_list()` could
> process the ring buffer after memory was freed.
>
> On Sat, Oct 19, 2025 at 1:13 UTC, Alexei Starovoitov wrote:
> > Sort-of kind-of makes sense, but bpf_ringbuf_free() is called
> > when no references to bpf map are left. User space and bpf progs
> > are not using it anymore, so irq_work callbacks should have completed
> > long ago.
>
> You're correct that normally all irq_work callbacks should have completed
> by the time bpf_ringbuf_free() is called. However, there is a small
> race window. In the syzbot reproducer (https://syzkaller.appspot.com/text?tag=ReproC&x=17a24b34580000),
> the BPF program is attached to sched_switch and it also writes to the
> ring buffer on every context switch. Each forked child creates the
> BPF program and quickly drops the last reference after bpf_ringbuf_commit()
> queues an irq_work. Because the irq_work runs asynchronously, it may still
> be pending when bpf_ringbuf_free() executes, thus creating a small race
> window that can lead to use-after-free.
>
> Adding `irq_work_sync(&rb->work)` ensures that all pending notifications
> complete before freeing the buffer.
I think this all makes sense and the fix should be good. Please add
the above details (perhaps in a bit more condensed form) to the commit
message.
>
> Thanks,
> Noorain Eqbal
Powered by blists - more mailing lists