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

[Andrii Nakryiko]

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