veth: Fix race with AF_XDP exposing old or uninitialized descriptors

Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=2164865
Upstream Status: net.git commit fa349e3

commit fa349e3
Author: Shawn Bohrer <sbohrer@cloudflare.com>
Date:   Tue Dec 20 12:59:03 2022 -0600

    veth: Fix race with AF_XDP exposing old or uninitialized descriptors

    When AF_XDP is used on on a veth interface the RX ring is updated in two
    steps.  veth_xdp_rcv() removes packet descriptors from the FILL ring
    fills them and places them in the RX ring updating the cached_prod
    pointer.  Later xdp_do_flush() syncs the RX ring prod pointer with the
    cached_prod pointer allowing user-space to see the recently filled in
    descriptors.  The rings are intended to be SPSC, however the existing
    order in veth_poll allows the xdp_do_flush() to run concurrently with
    another CPU creating a race condition that allows user-space to see old
    or uninitialized descriptors in the RX ring.  This bug has been observed
    in production systems.

    To summarize, we are expecting this ordering:

    CPU 0 __xsk_rcv_zc()
    CPU 0 __xsk_map_flush()
    CPU 2 __xsk_rcv_zc()
    CPU 2 __xsk_map_flush()

    But we are seeing this order:

    CPU 0 __xsk_rcv_zc()
    CPU 2 __xsk_rcv_zc()
    CPU 0 __xsk_map_flush()
    CPU 2 __xsk_map_flush()

    This occurs because we rely on NAPI to ensure that only one napi_poll
    handler is running at a time for the given veth receive queue.
    napi_schedule_prep() will prevent multiple instances from getting
    scheduled. However calling napi_complete_done() signals that this
    napi_poll is complete and allows subsequent calls to
    napi_schedule_prep() and __napi_schedule() to succeed in scheduling a
    concurrent napi_poll before the xdp_do_flush() has been called.  For the
    veth driver a concurrent call to napi_schedule_prep() and
    __napi_schedule() can occur on a different CPU because the veth xmit
    path can additionally schedule a napi_poll creating the race.

    The fix as suggested by Magnus Karlsson, is to simply move the
    xdp_do_flush() call before napi_complete_done().  This syncs the
    producer ring pointers before another instance of napi_poll can be
    scheduled on another CPU.  It will also slightly improve performance by
    moving the flush closer to when the descriptors were placed in the
    RX ring.

    Fixes: d139600 ("veth: Add XDP TX and REDIRECT")
    Suggested-by: Magnus Karlsson <magnus.karlsson@gmail.com>
    Signed-off-by: Shawn Bohrer <sbohrer@cloudflare.com>
    Link: https://lore.kernel.org/r/20221220185903.1105011-1-sbohrer@cloudflare.com
    Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Signed-off-by: Davide Caratti <dcaratti@redhat.com>

Author: dcaratti

drivers/net/veth.c

@@ -985,6 +985,9 @@ static int veth_poll(struct napi_struct *napi, int budget)
 	xdp_set_return_frame_no_direct();
 	done = veth_xdp_rcv(rq, budget, &bq, &stats);
 
+	if (stats.xdp_redirect > 0)
+		xdp_do_flush();
+
 	if (done < budget && napi_complete_done(napi, done)) {
 		/* Write rx_notify_masked before reading ptr_ring */
 		smp_store_mb(rq->rx_notify_masked, false);
@@ -998,8 +1001,6 @@ static int veth_poll(struct napi_struct *napi, int budget)
 
 	if (stats.xdp_tx > 0)
 		veth_xdp_flush(rq, &bq);
-	if (stats.xdp_redirect > 0)
-		xdp_do_flush();
 	xdp_clear_return_frame_no_direct();
 
 	return done;