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

Israel Téllez García <[email protected]>
Newsgroups org.kernel.vger.bpf,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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 <[email protected]>
---
 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
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.