[PATCH bpf 1/2] bpf: Load ringbuf consumer_pos under the spinlock

[Igor Malovitsa]

__bpf_ringbuf_reserve() loads consumer_pos before taking the spinlock.
Nothing bounds how stale that value is when the lock is finally taken:
other producers may have reserved, and the consumer consumed, any
amount of data meanwhile. bpf_ringbuf_has_space() compares
new_prod_pos - cons_pos as unsigned long, so on 32-bit, once positions
wrap, a value close to 2^32 bytes stale passes the check and the new
record is written over records the consumer has not read yet.

Load consumer_pos after taking the lock. producer_pos cannot move while
the lock is held and the consumer never passes producer_pos, so the
value is less than a ring behind and the difference is exact.

This needs a reservation to stall for about 4 GiB of traffic on a
32-bit kernel; it was found with a formal model of the protocol,
and reproduced using late_initcall.

Fixes: 457f44363a88 ("bpf: Implement BPF ring buffer and verifier support for it")
Assisted-by: LLM
Signed-off-by: Igor Malovitsa <igor.malovitsa@xxxxxxxxx>
---
 kernel/bpf/ringbuf.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/kernel/bpf/ringbuf.c b/kernel/bpf/ringbuf.c
index 3f1013d80544..6b86c4db5108 100644
--- a/kernel/bpf/ringbuf.c
+++ b/kernel/bpf/ringbuf.c
@@ -473,11 +473,12 @@ static void *__bpf_ringbuf_reserve(struct bpf_ringbuf *rb, u64 size)
 	if (len > ringbuf_total_data_sz(rb))
 		return NULL;
 
-	cons_pos = smp_load_acquire(&rb->consumer_pos);
-
 	if (raw_res_spin_lock_irqsave(&rb->spinlock, flags))
 		return NULL;
 
+	/* Under the lock, so at most one ring behind producer_pos. */
+	cons_pos = smp_load_acquire(&rb->consumer_pos);
+
 	pend_pos = rb->pending_pos;
 	prod_pos = rb->producer_pos;
 	new_prod_pos = prod_pos + len;
-- 
2.53.0