The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v2 bpf 0/4] bpf: Fix ring buffer position wrap handling on 32-bit
@ 2026-08-14 12:48 Israel Téllez García
  2026-08-14 12:48 ` [PATCH v2 bpf 1/4] bpf: Fix pending_pos walk on 32-bit ring position wrap Israel Téllez García
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Israel Téllez García @ 2026-08-14 12:48 UTC (permalink / raw)
  To: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann
  Cc: Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
	Emil Tsalapatis, Bing-Jhong Billy Jheng, bpf, linux-kernel,
	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


^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH v2 bpf 1/4] bpf: Fix pending_pos walk on 32-bit ring position wrap
  2026-08-14 12:48 [PATCH v2 bpf 0/4] bpf: Fix ring buffer position wrap handling on 32-bit Israel Téllez García
@ 2026-08-14 12:48 ` Israel Téllez García
  2026-08-14 13:31   ` bot+bpf-ci
  2026-08-14 12:48 ` [PATCH v2 bpf 2/4] bpf: Fix available-data accounting on 32-bit wrap in overwrite mode Israel Téllez García
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Israel Téllez García @ 2026-08-14 12:48 UTC (permalink / raw)
  To: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann
  Cc: Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
	Emil Tsalapatis, Bing-Jhong Billy Jheng, bpf, linux-kernel,
	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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 bpf 2/4] bpf: Fix available-data accounting on 32-bit wrap in overwrite mode
  2026-08-14 12:48 [PATCH v2 bpf 0/4] bpf: Fix ring buffer position wrap handling on 32-bit Israel Téllez García
  2026-08-14 12:48 ` [PATCH v2 bpf 1/4] bpf: Fix pending_pos walk on 32-bit ring position wrap Israel Téllez García
@ 2026-08-14 12:48 ` Israel Téllez García
  2026-08-14 12:48 ` [PATCH v2 bpf 3/4] bpf: Read producer_pos before overwrite_pos in ringbuf_avail_data_sz() Israel Téllez García
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 11+ messages in thread
From: Israel Téllez García @ 2026-08-14 12:48 UTC (permalink / raw)
  To: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann
  Cc: Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
	Emil Tsalapatis, Bing-Jhong Billy Jheng, bpf, linux-kernel,
	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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 bpf 3/4] bpf: Read producer_pos before overwrite_pos in ringbuf_avail_data_sz()
  2026-08-14 12:48 [PATCH v2 bpf 0/4] bpf: Fix ring buffer position wrap handling on 32-bit Israel Téllez García
  2026-08-14 12:48 ` [PATCH v2 bpf 1/4] bpf: Fix pending_pos walk on 32-bit ring position wrap Israel Téllez García
  2026-08-14 12:48 ` [PATCH v2 bpf 2/4] bpf: Fix available-data accounting on 32-bit wrap in overwrite mode Israel Téllez García
@ 2026-08-14 12:48 ` Israel Téllez García
  2026-08-14 13:30   ` bot+bpf-ci
                     ` (2 more replies)
  2026-08-14 12:48 ` [PATCH v2 bpf 4/4] libbpf: Fix ring buffer consumer loop on 32-bit position wrap Israel Téllez García
  2026-08-14 22:30 ` [PATCH v2 bpf 0/4] bpf: Fix ring buffer position wrap handling on 32-bit patchwork-bot+netdevbpf
  4 siblings, 3 replies; 11+ messages in thread
From: Israel Téllez García @ 2026-08-14 12:48 UTC (permalink / raw)
  To: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann
  Cc: Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
	Emil Tsalapatis, Bing-Jhong Billy Jheng, bpf, linux-kernel,
	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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH v2 bpf 4/4] libbpf: Fix ring buffer consumer loop on 32-bit position wrap
  2026-08-14 12:48 [PATCH v2 bpf 0/4] bpf: Fix ring buffer position wrap handling on 32-bit Israel Téllez García
                   ` (2 preceding siblings ...)
  2026-08-14 12:48 ` [PATCH v2 bpf 3/4] bpf: Read producer_pos before overwrite_pos in ringbuf_avail_data_sz() Israel Téllez García
@ 2026-08-14 12:48 ` Israel Téllez García
  2026-08-14 13:31   ` bot+bpf-ci
  2026-08-14 22:30 ` [PATCH v2 bpf 0/4] bpf: Fix ring buffer position wrap handling on 32-bit patchwork-bot+netdevbpf
  4 siblings, 1 reply; 11+ messages in thread
From: Israel Téllez García @ 2026-08-14 12:48 UTC (permalink / raw)
  To: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann
  Cc: Andrii Nakryiko, Eduard Zingerman, Kumar Kartikeya Dwivedi,
	Martin KaFai Lau, Song Liu, Yonghong Song, Jiri Olsa,
	Emil Tsalapatis, Bing-Jhong Billy Jheng, bpf, linux-kernel,
	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


^ permalink raw reply related	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 bpf 3/4] bpf: Read producer_pos before overwrite_pos in ringbuf_avail_data_sz()
  2026-08-14 12:48 ` [PATCH v2 bpf 3/4] bpf: Read producer_pos before overwrite_pos in ringbuf_avail_data_sz() Israel Téllez García
@ 2026-08-14 13:30   ` bot+bpf-ci
  2026-08-14 14:09   ` Israel Téllez
  2026-08-14 22:20   ` Andrii Nakryiko
  2 siblings, 0 replies; 11+ messages in thread
From: bot+bpf-ci @ 2026-08-14 13:30 UTC (permalink / raw)
  To: i.tellez, andrii.nakryiko, ast, daniel
  Cc: andrii, eddyz87, memxor, martin.lau, song, yonghong.song, jolsa,
	emil, billy, bpf, linux-kernel, i.tellez, ast, andrii, daniel,
	martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai

[-- 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 bpf 4/4] libbpf: Fix ring buffer consumer loop on 32-bit position wrap
  2026-08-14 12:48 ` [PATCH v2 bpf 4/4] libbpf: Fix ring buffer consumer loop on 32-bit position wrap Israel Téllez García
@ 2026-08-14 13:31   ` bot+bpf-ci
  0 siblings, 0 replies; 11+ messages in thread
From: bot+bpf-ci @ 2026-08-14 13:31 UTC (permalink / raw)
  To: i.tellez, andrii.nakryiko, ast, daniel
  Cc: andrii, eddyz87, memxor, martin.lau, song, yonghong.song, jolsa,
	emil, billy, bpf, linux-kernel, i.tellez, ast, andrii, daniel,
	martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai

[-- 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 bpf 1/4] bpf: Fix pending_pos walk on 32-bit ring position wrap
  2026-08-14 12:48 ` [PATCH v2 bpf 1/4] bpf: Fix pending_pos walk on 32-bit ring position wrap Israel Téllez García
@ 2026-08-14 13:31   ` bot+bpf-ci
  0 siblings, 0 replies; 11+ messages in thread
From: bot+bpf-ci @ 2026-08-14 13:31 UTC (permalink / raw)
  To: i.tellez, andrii.nakryiko, ast, daniel
  Cc: andrii, eddyz87, memxor, martin.lau, song, yonghong.song, jolsa,
	emil, billy, bpf, linux-kernel, i.tellez, ast, andrii, daniel,
	martin.lau, eddyz87, yonghong.song, clm, ihor.solodrai

[-- 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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 bpf 3/4] bpf: Read producer_pos before overwrite_pos in ringbuf_avail_data_sz()
  2026-08-14 12:48 ` [PATCH v2 bpf 3/4] bpf: Read producer_pos before overwrite_pos in ringbuf_avail_data_sz() Israel Téllez García
  2026-08-14 13:30   ` bot+bpf-ci
@ 2026-08-14 14:09   ` Israel Téllez
  2026-08-14 22:20   ` Andrii Nakryiko
  2 siblings, 0 replies; 11+ messages in thread
From: Israel Téllez @ 2026-08-14 14:09 UTC (permalink / raw)
  To: Andrii Nakryiko, Alexei Starovoitov, Daniel Borkmann
  Cc: Andrii Nakryiko, bpf, linux-kernel

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.

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 bpf 3/4] bpf: Read producer_pos before overwrite_pos in ringbuf_avail_data_sz()
  2026-08-14 12:48 ` [PATCH v2 bpf 3/4] bpf: Read producer_pos before overwrite_pos in ringbuf_avail_data_sz() Israel Téllez García
  2026-08-14 13:30   ` bot+bpf-ci
  2026-08-14 14:09   ` Israel Téllez
@ 2026-08-14 22:20   ` Andrii Nakryiko
  2 siblings, 0 replies; 11+ messages in thread
From: Andrii Nakryiko @ 2026-08-14 22:20 UTC (permalink / raw)
  To: Israel Téllez García
  Cc: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
	Bing-Jhong Billy Jheng, bpf, linux-kernel

On Fri, Aug 14, 2026 at 5:49 AM Israel Téllez García <i.tellez@btesa.com> wrote:
>
> 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);

by reading over_pos after prod_pos, we can see over_pos logically
after prod_pos, which will lead to bogus result. With existing
ordering we at most will report stale amount of data, which is much
more benign. I'm dropping this change


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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* Re: [PATCH v2 bpf 0/4] bpf: Fix ring buffer position wrap handling on 32-bit
  2026-08-14 12:48 [PATCH v2 bpf 0/4] bpf: Fix ring buffer position wrap handling on 32-bit Israel Téllez García
                   ` (3 preceding siblings ...)
  2026-08-14 12:48 ` [PATCH v2 bpf 4/4] libbpf: Fix ring buffer consumer loop on 32-bit position wrap Israel Téllez García
@ 2026-08-14 22:30 ` patchwork-bot+netdevbpf
  4 siblings, 0 replies; 11+ messages in thread
From: patchwork-bot+netdevbpf @ 2026-08-14 22:30 UTC (permalink / raw)
  To: =?utf-8?b?SXNyYWVsIFTDqWxsZXogPGkudGVsbGV6QGJ0ZXNhLmNvbT4=?=
  Cc: andrii.nakryiko, ast, daniel, andrii, eddyz87, memxor, martin.lau,
	song, yonghong.song, jolsa, emil, billy, bpf, linux-kernel

Hello:

This series was applied to bpf/bpf-next.git (master)
by Andrii Nakryiko <andrii@kernel.org>:

On Fri, 14 Aug 2026 14:48:39 +0200 you wrote:
> 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.
> 
> [...]

Here is the summary with links:
  - [v2,bpf,1/4] bpf: Fix pending_pos walk on 32-bit ring position wrap
    https://git.kernel.org/bpf/bpf-next/c/6ff5b56a50c5
  - [v2,bpf,2/4] bpf: Fix available-data accounting on 32-bit wrap in overwrite mode
    https://git.kernel.org/bpf/bpf-next/c/3f611e9b820e
  - [v2,bpf,3/4] bpf: Read producer_pos before overwrite_pos in ringbuf_avail_data_sz()
    (no matching commit)
  - [v2,bpf,4/4] libbpf: Fix ring buffer consumer loop on 32-bit position wrap
    https://git.kernel.org/bpf/bpf-next/c/fdd4fad0bbbd

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2026-08-14 22:31 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-14 12:48 [PATCH v2 bpf 0/4] bpf: Fix ring buffer position wrap handling on 32-bit Israel Téllez García
2026-08-14 12:48 ` [PATCH v2 bpf 1/4] bpf: Fix pending_pos walk on 32-bit ring position wrap Israel Téllez García
2026-08-14 13:31   ` bot+bpf-ci
2026-08-14 12:48 ` [PATCH v2 bpf 2/4] bpf: Fix available-data accounting on 32-bit wrap in overwrite mode Israel Téllez García
2026-08-14 12:48 ` [PATCH v2 bpf 3/4] bpf: Read producer_pos before overwrite_pos in ringbuf_avail_data_sz() Israel Téllez García
2026-08-14 13:30   ` bot+bpf-ci
2026-08-14 14:09   ` Israel Téllez
2026-08-14 22:20   ` Andrii Nakryiko
2026-08-14 12:48 ` [PATCH v2 bpf 4/4] libbpf: Fix ring buffer consumer loop on 32-bit position wrap Israel Téllez García
2026-08-14 13:31   ` bot+bpf-ci
2026-08-14 22:30 ` [PATCH v2 bpf 0/4] bpf: Fix ring buffer position wrap handling on 32-bit patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox