Re: [PATCH bpf] bpf: Fix pending_pos walk on 32-bit ring position wrap

On Thu, Aug 6, 2026 at 6:02 AM Israel Téllez García <[email protected]> wrote:
>
> The reservation path caches the position of the oldest not-yet-committed
> record in rb->pending_pos and advances it past already committed records
> on every reservation:
>
>         while (pend_pos < prod_pos) {
>
> consumer_pos, producer_pos and pending_pos are unsigned long, i.e.
> 32-bit on 32-bit architectures, and Documentation/bpf/ringbuf.rst states
> that these counters may wrap around there. Every other comparison in the
> file is written as a difference, so modular arithmetic keeps them
> correct across the wrap. This one is an ordering comparison, and it is
> not wrap-safe.
>
> Once producer_pos wraps past 2^32, prod_pos is small while pend_pos
> still holds its pre-wrap value, so the loop condition is false and
> pending_pos is never advanced again. Reservations keep succeeding for a
> while, because bpf_ringbuf_has_space() uses differences, but
> new_prod_pos - pend_pos grows as the producer advances, and once it
> exceeds rb->mask every subsequent __bpf_ringbuf_reserve() call fails:
> the kernel believes a pending record spans the whole buffer. The ring
> never recovers, bpf_ringbuf_output() drops every event from then on, and
> nothing is logged.
>
> Observed on four armv7 devices (i.MX7 Dual, 6.6.52) running a
> tracepoint-based collector with a 512 KiB ring and 160-byte records.
> Every one of them stopped delivering after exactly 26846821 records and
> 4295491360 bytes had passed through the ring, at event rates between 441
> and 862 records/s, that is after 8 h to 17 h of uptime: the trigger is
> the byte count, not time or load. That figure is 2^32 plus 524064 bytes,
> and the excess is one ring's worth of grace period, as expected while
> new_prod_pos - pend_pos is still below rb->mask. The last reservation
> that fits is the largest record boundary X with X + 160 <= 524287, and
> since 2^32 mod 160 = 96 the boundaries after the wrap sit at
> X = 64 (mod 160), giving X = 524064. Userspace kept consuming normally
> until the producer stopped, then read zero records for good. With this
> patch applied, one of the four devices took 10 GiB through the same ring
> with no stall, while the three unpatched ones kept wedging at the same
> byte count.
>
> 64-bit hosts are unaffected in practice: their counters would need
> 16 EiB to wrap.
>
> Compare the two positions as a signed difference, which is wrap-safe
> here: the real distance between pend_pos and prod_pos is bounded by the
> ring size, far below the point where the sign would become ambiguous.
>
> Fixes: cfa1a2329a69 ("bpf: Fix overrunning reservations in ringbuf")
> Signed-off-by: Israel Téllez García <[email protected]>
> ---
>  kernel/bpf/ringbuf.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/kernel/bpf/ringbuf.c b/kernel/bpf/ringbuf.c
> index 35ae64ade36b..2c08a98370d5 100644
> --- a/kernel/bpf/ringbuf.c
> +++ b/kernel/bpf/ringbuf.c
> @@ -482,7 +482,7 @@ static void *__bpf_ringbuf_reserve(struct bpf_ringbuf *rb, u64 size)
>         prod_pos = rb->producer_pos;
>         new_prod_pos = prod_pos + len;
>
> -       while (pend_pos < prod_pos) {
> +       while ((long)(prod_pos - pend_pos) > 0) {

prod_pos should logically always be greater or equal to pend_pos, so
there is no need for casting here, please drop (long) part. See
sashiko's reply also, maybe you can try addressing other similar
issues that would bite you on 32-bit architectures.

pw-bot: cr

>                 hdr = (void *)rb->data + (pend_pos & rb->mask);
>                 hdr_len = READ_ONCE(hdr->len);
>                 if (hdr_len & BPF_RINGBUF_BUSY_BIT)
> --
> 2.43.0
>