[PATCH v2 bpf 0/4] bpf: Fix ring buffer position wrap handling on 32-bit

[PATCH v2 bpf 0/4] bpf: Fix ring buffer position wrap handling on 32-bit

[Israel Téllez García]

Patch 1 is v1 with the (long) cast dropped, as requested: pending_pos never
runs ahead of producer_pos, so the unsigned difference is the real distance.

While looking for other places where these positions are compared as values
rather than as distances, three more turned up; patches 2-4 fix those. All
four are the same class of bug: the positions are unsigned long, they wrap at
2^32 on 32-bit architectures, Documentation/bpf/ringbuf.rst says that wrap is
expected, and therefore every comparison between them has to be written as a
difference.

Patch 1 is the one that bites in the field: four armv7 devices stopped
delivering events after exactly 4295491360 bytes had passed through a 512 KiB
ring, and with it applied one of them has since taken 10 GiB through the same
ring with no stall.

Patches 2 and 3 are both in ringbuf_avail_data_sz(). The first replaces the
max() of two positions with a comparison of their distances to producer_pos;
the second reads producer_pos before overwrite_pos, which is the order that
__bpf_ringbuf_reserve() documents as the one the consumer must use. Patch 4 is
the userspace counterpart of patch 1, in libbpf's consumer loop.

The review also pointed at __bpf_user_ringbuf_peek(), where cons_pos and
prod_pos are u64 locals loaded from unsigned long fields, so on 32-bit they
never wrap and 'cons_pos >= prod_pos' stops working. That one is not part of
this series: it is the user-space-producer ring, where producer_pos is
untrusted input, so making it wrap-safe also means re-deriving the bounds
checks that keep the kernel safe there, which is a change of a different
nature from these four.

v1: https://lore.kernel.org/bpf/20260806130214.66028-1-i.tellez@btesa.com/

v2:
 - patch 1: drop the (long) cast (Andrii Nakryiko)
 - patches 2-4: new, the other 32-bit wrap sites found while reviewing the
   rest of the file and libbpf

Israel Téllez García (4):
  bpf: Fix pending_pos walk on 32-bit ring position wrap
  bpf: Fix available-data accounting on 32-bit wrap in overwrite mode
  bpf: Read producer_pos before overwrite_pos in ringbuf_avail_data_sz()
  libbpf: Fix ring buffer consumer loop on 32-bit position wrap

 kernel/bpf/ringbuf.c    | 12 +++++++++---
 tools/lib/bpf/ringbuf.c |  2 +-
 2 files changed, 10 insertions(+), 4 deletions(-)

-- 
2.39.5

[PATCH v2 bpf 1/4] bpf: Fix pending_pos walk on 32-bit ring position wrap

[Israel Téllez García]

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 difference instead. pending_pos never
runs ahead of producer_pos, so the unsigned difference is the real
distance between them and stays correct across the wrap.

Fixes: cfa1a2329a69 ("bpf: Fix overrunning reservations in ringbuf")
Signed-off-by: Israel Téllez García <i.tellez@btesa.com>
---
 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..06d3cc192601 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 (prod_pos - pend_pos > 0) {
 		hdr = (void *)rb->data + (pend_pos & rb->mask);
 		hdr_len = READ_ONCE(hdr->len);
 		if (hdr_len & BPF_RINGBUF_BUSY_BIT)
-- 
2.39.5

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

[sashiko-bot]

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

Pre-existing issues:
- [High] Missing wrap-around handling in BPF_MAP_TYPE_USER_RINGBUF kernel consumer
- [High] Missing wrap-around handling in libbpf ringbuffer consumer
- [Medium] Incorrect wrap-around handling and memory barrier usage in `ringbuf_avail_data_sz`
--

--- Patch [1]: [PATCH v2 bpf 1/4] bpf: Fix pending_pos walk on 32-bit ring position wrap ---
commit 3d10b943efa00590ce8d5db29b823f30ea00dbbf
Author: Israel Téllez García <i.tellez@btesa.com>

bpf: Fix pending_pos walk on 32-bit ring position wrap

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. Consumer, producer, and pending positions are 32-bit on
32-bit architectures and can wrap around, which causes an ordered comparison
to incorrectly stall reservations permanently. This patch changes the
ordered comparison to an unsigned difference, which stays correct across the
wrap-around.

> diff --git a/kernel/bpf/ringbuf.c b/kernel/bpf/ringbuf.c
> index 35ae64ade36b8..06d3cc192601f 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 (prod_pos - pend_pos > 0) {

[Severity: High]
This isn't a bug introduced by this patch, but since you are fixing the
32-bit wrap around issues, does this code in __bpf_user_ringbuf_peek() also
have the same problem?

kernel/bpf/ringbuf.c:__bpf_user_ringbuf_peek() {
    ...
    cons_pos = smp_load_acquire(&rb->consumer_pos);
    if (cons_pos >= prod_pos)
        return -ENODATA;
    ...
    if (total_len > prod_pos - cons_pos)
        return -EINVAL;
    ...
}

Because cons_pos and prod_pos are read from 32-bit unsigned longs into 64-bit
u64 variables, will the implicit zero-extension cause cons_pos >= prod_pos
to incorrectly evaluate to true when prod_pos wraps around 4GB?

Also, wouldn't the subtraction total_len > prod_pos - cons_pos underflow to a
massive positive value on 32-bit architecture wrap-around, bypassing the size
check?


[Severity: Medium]
This is a pre-existing issue, but is the wrap-around handling in
ringbuf_avail_data_sz() also affected?

kernel/bpf/ringbuf.c:ringbuf_avail_data_sz() {
    ...
    if (unlikely(rb->overwrite_mode)) {
        over_pos = smp_load_acquire(&rb->overwrite_pos);
        prod_pos = smp_load_acquire(&rb->producer_pos);
        return prod_pos - max(cons_pos, over_pos);
    }
    ...
}

Could the max() macro unsafely select the logically older cons_pos over
over_pos during a 32-bit wrap-around, because max() uses an ordered comparison
rather than modular arithmetic?

Additionally, is it safe to read overwrite_pos before producer_pos here,
or does this break the acquire-release pairing and allow reading a stale
overwrite_pos relative to producer_pos?


[Severity: High]
This is a pre-existing issue, but does the libbpf consumer loop have a similar
wrap-around problem on 32-bit architectures?

tools/lib/bpf/ringbuf.c:ringbuf_process_ring() {
    ...
    cons_pos = smp_load_acquire(r->consumer_pos);
    do {
        got_new_data = false;
        prod_pos = smp_load_acquire(r->producer_pos);
        while (cons_pos < prod_pos) {
    ...
}

If producer_pos wraps around the 32-bit boundary and becomes smaller than
consumer_pos, will the ordered loop condition cons_pos < prod_pos become false
and permanently stall the libbpf consumer?

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

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814124843.22041-1-i.tellez@btesa.com?part=1

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

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1060 bytes --]

> diff --git a/kernel/bpf/ringbuf.c b/kernel/bpf/ringbuf.c
> index 35ae64ade36b..06d3cc192601 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 (prod_pos - pend_pos > 0) {
>  		hdr = (void *)rb->data + (pend_pos & rb->mask);
>  		hdr_len = READ_ONCE(hdr->len);
>  		if (hdr_len & BPF_RINGBUF_BUSY_BIT)

This isn't a bug, but since both operands are unsigned long,
`prod_pos - pend_pos > 0` is the same as `pend_pos != prod_pos`.

Would spelling it that way (or `(long)(prod_pos - pend_pos) > 0` if an
ordering test is meant) make the intent clearer next to the
`- ... > rb->mask` comparisons above?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31802836964

[PATCH v2 bpf 2/4] bpf: Fix available-data accounting on 32-bit wrap in overwrite mode

[Israel Téllez García]

In overwrite mode ringbuf_avail_data_sz() picks the newer of the consumer
and overwrite positions before measuring how much data is available:

	return prod_pos - max(cons_pos, over_pos);

max() is an ordering comparison, and consumer_pos, producer_pos and
overwrite_pos are unsigned long, i.e. 32-bit on 32-bit architectures,
where Documentation/bpf/ringbuf.rst allows them to wrap. Once one of the
two positions has wrapped and the other has not, max() returns the older
one: the result is then a modular difference close to 2^32, so the
function reports far more available data than the ring can hold. Pollers
using BPF_RB_AVAIL_DATA get a bogus figure, and epoll consumers can be
woken with nothing to read.

Compare distances rather than positions. prod_pos - X is the amount of
data produced since X for either position, wrap or no wrap, so the newer
position is simply the one with the smaller distance, which is also the
value the function wants to return.

64-bit hosts are unaffected in practice: their counters would need
16 EiB to wrap. Found by review of the same class of bug fixed in
"bpf: Fix pending_pos walk on 32-bit ring position wrap".

Signed-off-by: Israel Téllez García <i.tellez@btesa.com>
---
 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 06d3cc192601..0fefa89039be 100644
--- a/kernel/bpf/ringbuf.c
+++ b/kernel/bpf/ringbuf.c
@@ -321,7 +321,7 @@ static unsigned long ringbuf_avail_data_sz(struct bpf_ringbuf *rb)
 	if (unlikely(rb->overwrite_mode)) {
 		over_pos = smp_load_acquire(&rb->overwrite_pos);
 		prod_pos = smp_load_acquire(&rb->producer_pos);
-		return prod_pos - max(cons_pos, over_pos);
+		return min(prod_pos - cons_pos, prod_pos - over_pos);
 	} else {
 		prod_pos = smp_load_acquire(&rb->producer_pos);
 		return prod_pos - cons_pos;
-- 
2.39.5

Re: [PATCH v2 bpf 2/4] bpf: Fix available-data accounting on 32-bit wrap in overwrite mode

[sashiko-bot]

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

Pre-existing issues:
- [High] 32-bit wrap-around bug in `__bpf_user_ringbuf_peek` deadlocks the user ringbuf.
--

--- Patch [2]: [PATCH v2 bpf 2/4] bpf: Fix available-data accounting on 32-bit wrap in overwrite mode ---
commit def0b55b3ee1635d80af8891542c5b83a8fe238e
Author: Israel Téllez García <i.tellez@btesa.com>

bpf: Fix available-data accounting on 32-bit wrap in overwrite mode

In overwrite mode ringbuf_avail_data_sz() picks the newer of the consumer
and overwrite positions before measuring how much data is available. This
commit fixes a 32-bit wrap issue by comparing distances rather than
positions.

> diff --git a/kernel/bpf/ringbuf.c b/kernel/bpf/ringbuf.c
> index 06d3cc192601f..0fefa89039beb 100644
> --- a/kernel/bpf/ringbuf.c
> +++ b/kernel/bpf/ringbuf.c
> @@ -321,7 +321,7 @@ static unsigned long ringbuf_avail_data_sz(struct bpf_ringbuf *rb)
>  	if (unlikely(rb->overwrite_mode)) {
>  		over_pos = smp_load_acquire(&rb->overwrite_pos);
>  		prod_pos = smp_load_acquire(&rb->producer_pos);
> -		return prod_pos - max(cons_pos, over_pos);
> +		return min(prod_pos - cons_pos, prod_pos - over_pos);
>  	} else {
>  		prod_pos = smp_load_acquire(&rb->producer_pos);
>  		return prod_pos - cons_pos;

[Severity: High]
This is a pre-existing issue, but does a similar 32-bit wrap-around bug
exist in __bpf_user_ringbuf_peek() within the same file?

Looking at __bpf_user_ringbuf_peek(), it loads the 32-bit producer and
consumer positions into u64 variables:

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)
        return -ENODATA;
    ...
    if (total_len > prod_pos - cons_pos)
        return -EINVAL;
    ...
}

Because these are zero-extended into u64 on 32-bit architectures, does the
ordering comparison (cons_pos >= prod_pos) evaluate to true when the 32-bit
producer_pos wraps around to 0 (e.g., 0xFFFFFFF8 >= 0x00000008)?

If so, would this cause __bpf_user_ringbuf_peek() to incorrectly return
-ENODATA, failing to advance cons_pos and permanently deadlocking the user
ringbuffer once more than 4GB of data is pushed?

Additionally, does the u64 zero-extension break the modulo arithmetic in the
distance calculation (prod_pos - cons_pos)?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814124843.22041-1-i.tellez@btesa.com?part=2

[PATCH v2 bpf 3/4] bpf: Read producer_pos before overwrite_pos in ringbuf_avail_data_sz()

[Israel Téllez García]

The producer documents the order the consumer has to use, in
__bpf_ringbuf_reserve():

	 * smp_store_release(&rb->producer_pos, new_prod_pos) at
	 * the end of the function ensures that when consumer sees
	 * the updated rb->producer_pos, it always sees the updated
	 * rb->overwrite_pos, so when consumer reads overwrite_pos
	 * after smp_load_acquire(r->producer_pos), the overwrite_pos
	 * will always be valid.

ringbuf_avail_data_sz() reads them the other way round, so the acquire
load of producer_pos no longer orders the load of overwrite_pos. Nothing
then prevents the pair from being observed as a stale overwrite_pos next
to an already advanced producer_pos, which overstates the amount of
available data by however far the producer moved in between.

Swap the two loads so the documented contract holds, and say why in a
comment.

Signed-off-by: Israel Téllez García <i.tellez@btesa.com>
---
 kernel/bpf/ringbuf.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/kernel/bpf/ringbuf.c b/kernel/bpf/ringbuf.c
index 0fefa89039be..89a9c15a260f 100644
--- a/kernel/bpf/ringbuf.c
+++ b/kernel/bpf/ringbuf.c
@@ -319,8 +319,14 @@ static unsigned long ringbuf_avail_data_sz(struct bpf_ringbuf *rb)
 	cons_pos = smp_load_acquire(&rb->consumer_pos);
 
 	if (unlikely(rb->overwrite_mode)) {
-		over_pos = smp_load_acquire(&rb->overwrite_pos);
+		/*
+		 * Read producer_pos first: its release store in
+		 * __bpf_ringbuf_reserve() is what publishes the matching
+		 * overwrite_pos.
+		 */
 		prod_pos = smp_load_acquire(&rb->producer_pos);
+		/* Ordered after the acquire load above, per that contract. */
+		over_pos = smp_load_acquire(&rb->overwrite_pos);
 		return min(prod_pos - cons_pos, prod_pos - over_pos);
 	} else {
 		prod_pos = smp_load_acquire(&rb->producer_pos);
-- 
2.39.5

Re: [PATCH v2 bpf 3/4] bpf: Read producer_pos before overwrite_pos in ringbuf_avail_data_sz()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] Swapping the load order of producer_pos and overwrite_pos introduces a TOCTOU race that causes an integer underflow in `prod_pos - over_pos`, resulting in a massively overstated available data size.
--

--- Patch [3]: [PATCH v2 bpf 3/4] bpf: Read producer_pos before overwrite_pos in ringbuf_avail_data_sz() ---
commit efaae45af0e3891c65188df6e4677342c6fb0c41
Author: Israel Téllez García <i.tellez@btesa.com>

bpf: Read producer_pos before overwrite_pos in ringbuf_avail_data_sz()

This commit changes the order in which producer_pos and overwrite_pos are
read in ringbuf_avail_data_sz() to match the documented memory ordering
contract. It swaps the reads so that the acquire load of producer_pos
orders the subsequent load of overwrite_pos.

> diff --git a/kernel/bpf/ringbuf.c b/kernel/bpf/ringbuf.c
> index 0fefa89039beb..89a9c15a260f3 100644
> --- a/kernel/bpf/ringbuf.c
> +++ b/kernel/bpf/ringbuf.c
> @@ -319,8 +319,14 @@ static unsigned long ringbuf_avail_data_sz(struct bpf_ringbuf *rb)
>  	cons_pos = smp_load_acquire(&rb->consumer_pos);
>  
>  	if (unlikely(rb->overwrite_mode)) {
> -		over_pos = smp_load_acquire(&rb->overwrite_pos);
> +		/*
> +		 * Read producer_pos first: its release store in
> +		 * __bpf_ringbuf_reserve() is what publishes the matching
> +		 * overwrite_pos.
> +		 */
>  		prod_pos = smp_load_acquire(&rb->producer_pos);
> +		/* Ordered after the acquire load above, per that contract. */
> +		over_pos = smp_load_acquire(&rb->overwrite_pos);
>  		return min(prod_pos - cons_pos, prod_pos - over_pos);

[Severity: High]
Does this sequence allow an integer underflow in prod_pos - over_pos?

If the reader is preempted after reading prod_pos, and a concurrent producer
advances overwrite_pos beyond the stale prod_pos snapshot:

Reader:
    prod_pos = smp_load_acquire(&rb->producer_pos);
    <preempted>

Producer:
    // advances overwrite_pos beyond the snapshotted prod_pos
    rb->overwrite_pos = ...;
    smp_store_release(&rb->producer_pos, ...);

Reader resumes:
    over_pos = smp_load_acquire(&rb->overwrite_pos);
    return min(prod_pos - cons_pos, prod_pos - over_pos);

Could over_pos now be greater than prod_pos, causing the subtraction to
underflow to a massive unsigned value near ULONG_MAX?

This would result in min() returning prod_pos - cons_pos, which could be
extremely large and break EPOLLOUT for user-space producers by returning
incorrectly large available data sizes.

>  	} else {
>  		prod_pos = smp_load_acquire(&rb->producer_pos);
>  		return prod_pos - cons_pos;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814124843.22041-1-i.tellez@btesa.com?part=3

Re: [PATCH v2 bpf 3/4] bpf: Read producer_pos before overwrite_pos in ringbuf_avail_data_sz()

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1416 bytes --]

> bpf: Read producer_pos before overwrite_pos in ringbuf_avail_data_sz()
>
> The producer documents the order the consumer has to use, in
> __bpf_ringbuf_reserve():
>
>          * smp_store_release(&rb->producer_pos, new_prod_pos) at
>          * the end of the function ensures that when consumer sees
>          * the updated rb->producer_pos, it always sees the updated
>          * rb->overwrite_pos, so when consumer reads overwrite_pos
>          * after smp_load_acquire(r->producer_pos), the overwrite_pos
>          * will always be valid.
>
> ringbuf_avail_data_sz() reads them the other way round, so the acquire
> load of producer_pos no longer orders the load of overwrite_pos. Nothing
> then prevents the pair from being observed as a stale overwrite_pos next
> to an already advanced producer_pos, which overstates the amount of
> available data by however far the producer moved in between.
>
> Swap the two loads so the documented contract holds, and say why in a
> comment.
>
> Signed-off-by: Israel Téllez García <i.tellez@btesa.com>

This looks like a bug fix for a memory ordering issue. Should this include:

  Fixes: b73246ff875b ("adding ci files")


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31802836964

Re: [PATCH v2 bpf 3/4] bpf: Read producer_pos before overwrite_pos in ringbuf_avail_data_sz()

[Israel Téllez]

Please drop this patch from the series; patches 1, 2 and 4 stand on their own.

Reading producer_pos first does follow the contract documented in
__bpf_ringbuf_reserve(), but it also allows overwrite_pos to be observed
ahead of the producer_pos snapshot, and then prod_pos - over_pos underflows
and min() falls back to prod_pos - cons_pos, which in overwrite mode is not
bounded by the ring size. That trades a bounded staleness for an unbounded
overestimate, which is worse than what it fixes.

Making the documented order safe needs an additional guard for over_pos
running ahead of the snapshot, so it belongs in its own patch rather than
here.

[PATCH v2 bpf 4/4] libbpf: Fix ring buffer consumer loop on 32-bit position wrap

[Israel Téllez García]

ringbuf_process_ring() walks the records between the consumer and the
producer with an ordering comparison:

	while (cons_pos < prod_pos) {

cons_pos and prod_pos mirror the kernel's ring positions and are
unsigned long here too, so on 32-bit they wrap at 2^32 bytes of traffic.
When producer_pos has wrapped and consumer_pos has not, prod_pos is the
smaller of the two, the loop body never runs and no record is consumed.
Since consumer_pos only advances inside that loop, it never wraps either
and the consumer stops delivering samples for good, with no error
returned to the caller: ring_buffer__poll() keeps reporting zero
records while the kernel side fills up and starts dropping.

Compare the distance instead. The consumer never runs ahead of the
producer, so prod_pos - cons_pos is the amount of unconsumed data and
stays correct across the wrap.

64-bit hosts are unaffected in practice: the counters would need 16 EiB
to wrap. This is the userspace counterpart of the kernel-side walk fixed
in "bpf: Fix pending_pos walk on 32-bit ring position wrap"; a 32-bit
consumer hits whichever of the two comes first.

Signed-off-by: Israel Téllez García <i.tellez@btesa.com>
---
 tools/lib/bpf/ringbuf.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/lib/bpf/ringbuf.c b/tools/lib/bpf/ringbuf.c
index 00ec4837a06d..c8fe57401a8b 100644
--- a/tools/lib/bpf/ringbuf.c
+++ b/tools/lib/bpf/ringbuf.c
@@ -244,7 +244,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 (prod_pos - cons_pos > 0) {
 			len_ptr = r->data + (cons_pos & r->mask);
 			len = smp_load_acquire(len_ptr);
 
-- 
2.39.5

Re: [PATCH v2 bpf 4/4] libbpf: Fix ring buffer consumer loop on 32-bit position wrap

[sashiko-bot]

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

New issues:
- [High] The loop condition `prod_pos - cons_pos > 0` uses unsigned arithmetic, turning it into a simple inequality check (`!=`) which causes an infinite loop if `cons_pos` oversteps `prod_pos`.

Pre-existing issues:
- [High] 32-bit user space on 64-bit kernel will experience permanent ringbuffer stall after 4GB of traffic.
- [High] The kernel's `__bpf_user_ringbuf_peek` implementation suffers from a 32-bit position wrap bug that permanently breaks the map on 32-bit architectures.
--

--- Patch [4]: [PATCH v2 bpf 4/4] libbpf: Fix ring buffer consumer loop on 32-bit position wrap ---
commit fb935226d411d583439a72026e8126519f36b208
Author: Israel Téllez García <i.tellez@btesa.com>

libbpf: Fix ring buffer consumer loop on 32-bit position wrap

ringbuf_process_ring() walks the records between the consumer and the
producer with an ordering comparison. This patch fixes a 32-bit position
wrap bug by comparing the distance instead, as the consumer never runs
ahead of the producer.

> diff --git a/tools/lib/bpf/ringbuf.c b/tools/lib/bpf/ringbuf.c
> index 00ec4837a06de..c8fe57401a8b5 100644
> --- a/tools/lib/bpf/ringbuf.c
> +++ b/tools/lib/bpf/ringbuf.c
> @@ -244,7 +244,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);

[Severity: High]
This isn't a bug introduced by this patch, but on a 32-bit userspace running
on a 64-bit kernel, does this lead to a permanent ring buffer stall?

Because unsigned long is 32-bit in userspace but the kernel uses 64-bit
positions, won't producer_pos in the kernel exceed 2^32 after 4GB of
traffic, while the 32-bit userspace only updates the lower 32 bits of
consumer_pos?

Does this cause prod_pos - cons_pos in the kernel to permanently exceed the
ring buffer mask, causing bpf_ringbuf_has_space() to reject all future
records?

> -		while (cons_pos < prod_pos) {
> +		while (prod_pos - cons_pos > 0) {

[Severity: High]
Does this condition risk an infinite loop in the consumer?

Since prod_pos and cons_pos are unsigned long, their subtraction is
unsigned and strictly >= 0, making > 0 act identically to != 0.

If cons_pos oversteps prod_pos due to reading a torn or corrupted record
length (such as when the kernel overwrites unconsumed records in
BPF_F_RB_OVERWRITE mode), won't cons_pos bypass prod_pos?

Will the loop then spin endlessly, processing garbage memory as records
until cons_pos coincidentally aligns perfectly with prod_pos again?

>  			len_ptr = r->data + (cons_pos & r->mask);
>  			len = smp_load_acquire(len_ptr);
>  

While reviewing this 32-bit position wrap behavior, I noticed a similar
pre-existing issue in the kernel's USER_RINGBUF implementation.

[Severity: High]
This is a pre-existing issue, but does __bpf_user_ringbuf_peek() permanently
break maps on 32-bit architectures?

kernel/bpf/ringbuf.c:__bpf_user_ringbuf_peek() {
    ...
    u64 cons_pos, prod_pos;
    ...
    if (cons_pos >= prod_pos)
        return -ENODATA;
    ...
}

On 32-bit kernels, rb->producer_pos and rb->consumer_pos are 32-bit
variables read into 64-bit u64 variables. When producer_pos wraps past
32-bit, its zero-extended 64-bit value becomes small, while consumer_pos
remains large.

Does this absolute ordering check falsely evaluate to true, permanently
returning -ENODATA and stalling the map until user space pushes another 4GB
of data?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260814124843.22041-1-i.tellez@btesa.com?part=4

Re: [PATCH v2 bpf 4/4] libbpf: Fix ring buffer consumer loop on 32-bit position wrap

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 1912 bytes --]

> libbpf: Fix ring buffer consumer loop on 32-bit position wrap
>
> ringbuf_process_ring() walks the records between the consumer and the
> producer with an ordering comparison:
>
> 	while (cons_pos < prod_pos) {
>
> cons_pos and prod_pos mirror the kernel's ring positions and are
> unsigned long here too, so on 32-bit they wrap at 2^32 bytes of traffic.
> When producer_pos has wrapped and consumer_pos has not, prod_pos is the
> smaller of the two, the loop body never runs and no record is consumed.
> Since consumer_pos only advances inside that loop, it never wraps either
> and the consumer stops delivering samples for good, with no error
> returned to the caller: ring_buffer__poll() keeps reporting zero
> records while the kernel side fills up and starts dropping.
>
> Compare the distance instead. The consumer never runs ahead of the
> producer, so prod_pos - cons_pos is the amount of unconsumed data and
> stays correct across the wrap.
>
> 64-bit hosts are unaffected in practice: the counters would need 16 EiB
> to wrap. This is the userspace counterpart of the kernel-side walk fixed
> in "bpf: Fix pending_pos walk on 32-bit ring position wrap"; a 32-bit
> consumer hits whichever of the two comes first.
>
> Signed-off-by: Israel Téllez García <i.tellez@btesa.com>

This looks like a bug fix for ring buffer support. Should this include:

  Fixes: bf99c936f947 ("libbpf: Add BPF ring buffer support")

The buggy comparison 'while (cons_pos < prod_pos)' was introduced when
ring buffer support was first added to libbpf. On 32-bit systems, this
comparison fails when producer_pos wraps, causing the consumer to stop
delivering samples permanently.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/31802836964