lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [thread-next>] [day] [month] [year] [list]
Date:   Fri, 29 Nov 2019 10:51:10 +0100
From:   Magnus Karlsson <magnus.karlsson@...el.com>
To:     magnus.karlsson@...el.com, bjorn.topel@...el.com, ast@...nel.org,
        daniel@...earbox.net, netdev@...r.kernel.org,
        jonathan.lemon@...il.com
Cc:     maximmi@...lanox.com, bpf@...r.kernel.org
Subject: [PATCH bpf] xsk: add missing memory barrier in xskq_has_addrs()

The rings in AF_XDP between user space and kernel space have the
following semantics:

producer                         consumer

if (LOAD ->consumer) {           LOAD ->producer
                   (A)           smp_rmb()       (C)
   STORE $data                   LOAD $data
   smp_wmb()       (B)           smp_mb()        (D)
   STORE ->producer              STORE ->consumer
}

The consumer function xskq_has_addrs() below loads the producer
pointer and updates the locally cached copy of it. However, it does
not issue the smp_rmb() operation required by the lockless ring. This
would have been ok had the function not updated the locally cached
copy, as that could not have resulted in new data being read from the
ring. But as it updates the local producer pointer, a subsequent peek
operation, such as xskq_peek_addr(), might load data from the ring
without issuing the required smp_rmb() memory barrier.

static inline bool xskq_has_addrs(struct xsk_queue *q, u32 cnt)
{
        u32 entries = q->prod_tail - q->cons_tail;

        if (entries >= cnt)
                return true;

        /* Refresh the local pointer. */
        q->prod_tail = READ_ONCE(q->ring->producer);
	*** MISSING MEMORY BARRIER ***
        entries = q->prod_tail - q->cons_tail;

        return entries >= cnt;
}

Fix this by adding the missing memory barrier at the indicated point
above.

Fixes: d57d76428ae9 ("Add API to check for available entries in FQ")
Signed-off-by: Magnus Karlsson <magnus.karlsson@...el.com>
---
 net/xdp/xsk_queue.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/xdp/xsk_queue.h b/net/xdp/xsk_queue.h
index eddae46..b5492c3 100644
--- a/net/xdp/xsk_queue.h
+++ b/net/xdp/xsk_queue.h
@@ -127,6 +127,7 @@ static inline bool xskq_has_addrs(struct xsk_queue *q, u32 cnt)
 
 	/* Refresh the local pointer. */
 	q->prod_tail = READ_ONCE(q->ring->producer);
+	smp_rmb(); /* C, matches B */
 	entries = q->prod_tail - q->cons_tail;
 
 	return entries >= cnt;
-- 
2.7.4

Powered by blists - more mailing lists