Fix a possible vmalloc out-of-bounds access caused by pending IRQ work
by ensuring all pending IRQ work completes before freeing.

Fixes: 457f44363a88 ("bpf: Implement BPF ring buffer and verifier support for it")
Reported-by: syzbot+2617fc732430968b45d2@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=2617fc732430968b45d2
Tested-by: syzbot+2617fc732430968b45d2@syzkaller.appspotmail.com
Signed-off-by: neqbal <nooraineqbal@gmail.com>
---
 kernel/bpf/ringbuf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/bpf/ringbuf.c b/kernel/bpf/ringbuf.c
index 719d73299397..d706c4b7f532 100644
--- a/kernel/bpf/ringbuf.c
+++ b/kernel/bpf/ringbuf.c
@@ -216,6 +216,8 @@ static struct bpf_map *ringbuf_map_alloc(union bpf_attr *attr)
 
 static void bpf_ringbuf_free(struct bpf_ringbuf *rb)
 {
+	irq_work_sync(&rb->work);
+
 	/* copy pages pointer and nr_pages to local variable, as we are going
 	 * to unmap rb itself with vunmap() below
 	 */
-- 
2.51.0

On Sat, Oct 18, 2025 at 11:12 AM neqbal <nooraineqbal@gmail.com> wrote:
>
> Fix a possible vmalloc out-of-bounds access caused by pending IRQ work
> by ensuring all pending IRQ work completes before freeing.
>
> Fixes: 457f44363a88 ("bpf: Implement BPF ring buffer and verifier support for it")
> Reported-by: syzbot+2617fc732430968b45d2@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=2617fc732430968b45d2

Why do you think irq_work_run_list() processes bpf ringbuf in
the above splat?

> Tested-by: syzbot+2617fc732430968b45d2@syzkaller.appspotmail.com
> Signed-off-by: neqbal <nooraineqbal@gmail.com>

Use your real name.

> ---
>  kernel/bpf/ringbuf.c | 2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/kernel/bpf/ringbuf.c b/kernel/bpf/ringbuf.c
> index 719d73299397..d706c4b7f532 100644
> --- a/kernel/bpf/ringbuf.c
> +++ b/kernel/bpf/ringbuf.c
> @@ -216,6 +216,8 @@ static struct bpf_map *ringbuf_map_alloc(union bpf_attr *attr)
>
>  static void bpf_ringbuf_free(struct bpf_ringbuf *rb)
>  {
> +       irq_work_sync(&rb->work);

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.

pw-bot: cr

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.

Thanks,
Noorain Eqbal

On Sun, Oct 19, 2025 at 3:30 PM Noorain Eqbal <nooraineqbal@gmail.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

Fix a race where irq_work can be queued in bpf_ringbuf_commit()
but the ring buffer is freed before the work executes.
In the syzbot reproducer, a BPF program attached to sched_switch
triggers bpf_ringbuf_commit(), queuing an irq_work. If the ring buffer
is freed before this work executes, the irq_work thread may accesses
freed memory.
Calling `irq_work_sync(&rb->work)` ensures that all pending irq_work
complete before freeing the buffer

Fixes: 457f44363a88 ("bpf: Implement BPF ring buffer and verifier support for it")
Reported-by: syzbot+2617fc732430968b45d2@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=2617fc732430968b45d2
Tested-by: syzbot+2617fc732430968b45d2@syzkaller.appspotmail.com
Signed-off-by: Noorain Eqbal <nooraineqbal@gmail.com>
---
v2: Updated the commit message for clarity.
---
 kernel/bpf/ringbuf.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/kernel/bpf/ringbuf.c b/kernel/bpf/ringbuf.c
index 719d73299397..d706c4b7f532 100644
--- a/kernel/bpf/ringbuf.c
+++ b/kernel/bpf/ringbuf.c
@@ -216,6 +216,8 @@ static struct bpf_map *ringbuf_map_alloc(union bpf_attr *attr)
 
 static void bpf_ringbuf_free(struct bpf_ringbuf *rb)
 {
+	irq_work_sync(&rb->work);
+
 	/* copy pages pointer and nr_pages to local variable, as we are going
 	 * to unmap rb itself with vunmap() below
 	 */
-- 
2.51.0