[PATCH bpf 0/2] bpf: Fix ring buffer checks on 32-bit position wrap

[Igor Malovitsa <igor.malovitsa@gmail.com>]

Two more places where ring buffer positions are compared in a way that
breaks once they wrap on a 32-bit kernel. Two earlier fixes of the same
kind are commit 6ff5b56a50c5
("bpf: Fix pending_pos walk on 32-bit ring position wrap") and
commit 3f611e9b820e
("bpf: Fix available-data accounting on 32-bit wrap in overwrite mode").
The remaining two:

- __bpf_ringbuf_reserve() reads consumer_pos before taking the lock, so
  a reservation that stalls long enough can pass the space check with a
  value about 2^32 bytes stale and overwrite records the consumer has
  not read.

- __bpf_user_ringbuf_peek() widens the positions to u64 and compares
  them, so once producer_pos wraps and consumer_pos has not, the ring
  reads as empty from then on and the user-space producer gets -ENOSPC.

Both need about 4 GiB of ring traffic on a 32-bit kernel. Neither
changes anything on 64-bit.

How they were found and tested: a formal model of the ring buffer
protocol, written in Lean on the ORC11 memory model with the help of an
LLM coding assistant, showed both executions. Each was then confirmed on
32-bit QEMU guests (i386, arm and riscv32) by a test-only initcall (not
part of this series) that puts the positions in the wrapped state and
calls the real __bpf_ringbuf_reserve() and __bpf_user_ringbuf_peek():
without the patches it reports both bugs, with them both cases behave
correctly. With each patch, kernel/bpf/ringbuf.c builds without warnings
with W=1 on x86_64 and i386.

No selftest is added: reaching the states needs about 4 GiB of traffic
or instrumentation that only a test build can have.

The ring buffer selftests pass with the series applied on x86_64,
aarch64 and riscv64 (vmtest.sh, test_progs -t ringbuf: ringbuf,
ringbuf_multi, user_ringbuf and verifier_ringbuf, 33 subtests), and on
x86_64 without it. On riscv64 QEMU ran with -cpu max: the cross-compiled
test_progs uses Zbb/Zcb instructions that the CPU vmtest.sh picks lacks.

Reproducer (test-only, not for merging): a late_initcall in ringbuf.c,
built for i386 (tinyconfig), arm and riscv32 (their defconfigs), each
with BPF_SYSCALL, and booted in QEMU. For the first bug, a hook between
the consumer_pos load and the lock in __bpf_ringbuf_reserve() stands in
for the stalled reservation: it moves the positions to where 4 GiB of
traffic would leave them, so the load before the lock still holds the
old 0. Condensed, without the message prefix:

  /* hook, called just before raw_res_spin_lock_irqsave() */
  WRITE_ONCE(rb->consumer_pos, ULONG_MAX - PAGE_SIZE + 1);
  WRITE_ONCE(rb->producer_pos, ULONG_MAX - 7);
  WRITE_ONCE(rb->pending_pos, ULONG_MAX - 7);

  /* initcall: the ring is full, so the reservation must fail */
  rb = bpf_ringbuf_alloc(PAGE_SIZE, NUMA_NO_NODE, false);
  if (__bpf_ringbuf_reserve(rb, 0))
      pr_emerg("cons-under-lock: BUG reproduced (reservation accepted)\n");
  else
      pr_emerg("cons-under-lock: fixed (reservation rejected)\n");

  /* one empty sample at consumer_pos, producer_pos just past the wrap */
  rb = bpf_ringbuf_alloc(PAGE_SIZE, NUMA_NO_NODE, false);
  rb->consumer_pos = ULONG_MAX - 7;
  rb->producer_pos = 0;
  hdr = (void *)rb->data + (rb->consumer_pos & rb->mask);
  hdr->len = 0;
  hdr->pg_off = 0;
  err = __bpf_user_ringbuf_peek(rb, &sample, &size);
  if (err == -ENODATA)
      pr_emerg("user-ringbuf-wrap: BUG reproduced (false empty)\n");
  else if (!err && size == 0)
      pr_emerg("user-ringbuf-wrap: fixed (sample found)\n");

Output without this series:

  BPF_RINGBUF_REPRO: cons-under-lock: BUG reproduced (reservation accepted)
  BPF_RINGBUF_REPRO: user-ringbuf-wrap: BUG reproduced (false empty)

and with it:

  BPF_RINGBUF_REPRO: cons-under-lock: fixed (reservation rejected)
  BPF_RINGBUF_REPRO: user-ringbuf-wrap: fixed (sample found)

Igor Malovitsa (2):
  bpf: Load ringbuf consumer_pos under the spinlock
  bpf: Fix user ringbuf empty check on 32-bit position wrap

 kernel/bpf/ringbuf.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)


base-commit: ff47652a4b66c067c765a7ad464d930b5a9367cc
-- 
2.53.0