Re: [PATCH bpf 3/6] libbpf: ringbuf: Handle position counter wrap

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

Pre-existing issues:
- [High] `__bpf_ringbuf_reserve` in the kernel permanently fails allocations after 32-bit position wrap-around.
- [High] Kernel-side `__bpf_user_ringbuf_peek` incorrectly handles 32-bit wrap-around, leading to permanent stalls and bypassing bounds checks.
- [High] 32-bit userspace running on a 64-bit kernel permanently breaks BPF ring buffers after 4GB of data is processed due to pointer size mismatches.
--

commit ce5e8d811804f9ff598195a7f394512f77c373b0
Author: Tamir Duberstein <tamird@kernel.org>

libbpf: ringbuf: Handle position counter wrap

Ring buffer positions are unsigned long counters and can wrap on 32-bit
systems. ringbuf_process_ring() stops consuming when producer_pos wraps
below consumer_pos because it compares the counters by magnitude.

> diff --git a/tools/lib/bpf/ringbuf.c b/tools/lib/bpf/ringbuf.c
> index ae7fa79b62170..b7adce37b5196 100644
> --- a/tools/lib/bpf/ringbuf.c
> +++ b/tools/lib/bpf/ringbuf.c
> @@ -268,7 +268,7 @@ static int64_t ringbuf_process_ring(struct ring *r, size_t n)
>  	do {
>  		got_new_data = false;
>  		prod_pos = smp_load_acquire(r->producer_pos);
> -		while (cons_pos < prod_pos) {
> +		while (cons_pos != prod_pos) {
>  			len_ptr = r->data + (cons_pos & r->mask);
>  			len = smp_load_acquire(len_ptr);
> 

[Severity: High]
This is a pre-existing issue, but does this fix fully resolve the behavior
for 32-bit userspace applications running on 64-bit kernels?

Looking further down in ringbuf_process_ring(), userspace updates the
consumer position:

    __atomic_store_n(r->consumer_pos, cons_pos, __ATOMIC_RELEASE);

On a 32-bit architecture, this writes 4 bytes. However, the 64-bit kernel
defines consumer_pos as an 8-byte unsigned long. When the 32-bit userspace
counter wraps at 4GB, it writes 0 to the lower 4 bytes while leaving the
upper 4 bytes unchanged.

Could this cause the kernel's bpf_ringbuf_has_space() to perceive an
instant drop in the consumer position, making it evaluate the ring buffer
as permanently full?

[Severity: High]
This is a pre-existing issue, but does the kernel side have a similar
wrap-around issue in __bpf_ringbuf_reserve()?

kernel/bpf/ringbuf.c:__bpf_ringbuf_reserve() {
    ...
    while (pend_pos < prod_pos) {
        ...
    }
    ...
}

Since pend_pos and prod_pos are unsigned longs, on 32-bit kernels, does
this magnitude comparison fail when producer_pos wraps around to a small
value? It appears pend_pos would freeze at its pre-wrap value, causing
bpf_ringbuf_has_space() to permanently return false and deny future
allocations.

[Severity: High]
This is also a pre-existing issue, but does __bpf_user_ringbuf_peek() have
a related wrap-around bug on 32-bit systems?

kernel/bpf/ringbuf.c:__bpf_user_ringbuf_peek() {
    u64 cons_pos, prod_pos;
    ...
    prod_pos = smp_load_acquire(&rb->producer_pos);
    ...
    cons_pos = smp_load_acquire(&rb->consumer_pos);
    if (cons_pos >= prod_pos)
        ...
}

Because rb->producer_pos and rb->consumer_pos are 32-bit unsigned longs on
32-bit systems, are they zero-extended to 64-bit u64 variables? If
producer_pos wraps around while consumer_pos is still large, does the
64-bit promotion cause the magnitude comparison to evaluate to true,
permanently stalling consumption by returning -ENODATA?

Additionally, would the 64-bit subtraction (prod_pos - cons_pos) yield a
massive value that defeats the bounds check?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260613-bpf-ringbuf-fixes-v1-0-e623481cb724@kernel.org?part=3