[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.linux-kernel,org.kernel.vger.bpf |
|---|---|
| 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