From: Sandeep Penigalapati <[email protected]>

Shared UMEM is meant to be shared by a limited number of sockets,
governed by the mempool size (max_xsks). When the UMEM was already at
capacity (refcnt >= max_xsks), xdp_umem_configure() returned the UMEM
without incrementing its refcount, so the extra socket used it
unaccounted for.

This missing reference has two consequences. During queue setup the
fill-queue reservation is chosen from the refcount, so the sharing
socket reserves into its own uninitialised fill queue and crashes. At
close, the under-counted refcount reaches zero while the UMEM is still
in use, freeing it early and causing a use-after-free.

Reject sharing once the UMEM is at capacity by returning NULL, so queue
setup fails cleanly with -ENOMEM. This enforces the per-mempool socket
limit that shared UMEM was always intended to respect. Also document the
shared mempool sizing requirement (4096 mbufs per socket).

Fixes: 74b46340e2d4 ("net/af_xdp: support shared UMEM")
Cc: [email protected]

Signed-off-by: Sandeep Penigalapati <[email protected]>
---
 doc/guides/nics/af_xdp.rst          |  5 +++++
 drivers/net/af_xdp/rte_eth_af_xdp.c | 24 ++++++++++++++++++++----
 2 files changed, 25 insertions(+), 4 deletions(-)

diff --git a/doc/guides/nics/af_xdp.rst b/doc/guides/nics/af_xdp.rst
index c455b4c066..00ba89dc97 100644
--- a/doc/guides/nics/af_xdp.rst
+++ b/doc/guides/nics/af_xdp.rst
@@ -99,6 +99,11 @@ configured like so:
     --vdev net_af_xdp0,iface=ens786f1,shared_umem=1 \
     --vdev net_af_xdp1,iface=ens786f2,shared_umem=1
 
+The shared mempool must be large enough for every socket sharing the UMEM. Each
+socket requires 4096 mbufs, so a UMEM shared by ``N`` sockets needs at least
+``4096 * N`` mbufs. Port initialisation fails if the mempool is too small to
+add another socket to the UMEM.
+
 xdp_prog
 ~~~~~~~~
 
diff --git a/drivers/net/af_xdp/rte_eth_af_xdp.c 
b/drivers/net/af_xdp/rte_eth_af_xdp.c
index 2cdb533276..6ef76bd60a 100644
--- a/drivers/net/af_xdp/rte_eth_af_xdp.c
+++ b/drivers/net/af_xdp/rte_eth_af_xdp.c
@@ -1188,12 +1188,28 @@ xsk_umem_info *xdp_umem_configure(struct pmd_internals 
*internals,
                if (get_shared_umem(rxq, internals->if_name, &umem) < 0)
                        return NULL;
 
-               if (umem != NULL &&
-                       rte_atomic_load_explicit(&umem->refcnt, 
rte_memory_order_acquire) <
-                                       umem->max_xsks) {
+               if (umem != NULL) {
+                       uint32_t cnt = rte_atomic_load_explicit(&umem->refcnt,
+                                       rte_memory_order_acquire);
+
+                       /* Reject sharing once the UMEM is at capacity: sharing 
without
+                        * taking a reference corrupts the refcount and crashes 
later.
+                        */
+                       if (cnt >= umem->max_xsks) {
+                               AF_XDP_LOG_LINE(ERR,
+                                       "UMEM %s is shared by %u socket(s), max 
%u: "
+                                       "cannot share with %s,qid%i. "
+                                       "Increase the mempool size (%d mbufs 
per socket required).",
+                                       umem->mb_pool->name, cnt, 
umem->max_xsks,
+                                       internals->if_name, rxq->xsk_queue_idx,
+                                       ETH_AF_XDP_NUM_BUFFERS);
+                               return NULL;
+                       }
+
                        AF_XDP_LOG_LINE(INFO, "%s,qid%i sharing UMEM",
                                        internals->if_name, rxq->xsk_queue_idx);
-                       rte_atomic_fetch_add_explicit(&umem->refcnt, 1, 
rte_memory_order_acquire);
+                       rte_atomic_fetch_add_explicit(&umem->refcnt, 1,
+                                       rte_memory_order_acquire);
                }
        }
 
-- 
2.27.0

Reply via email to