Re: [PATCH v1] net/af_xdp: fix shared UMEM refcount corruption

Re: [PATCH v1] net/af_xdp: fix shared UMEM refcount corruption

[Stephen Hemminger]

On Wed, 12 Aug 2026 18:08:43 -0400
sandeep.penigalapati@intel.com wrote:

> From: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
> 
> 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: stable@dpdk.org
> 
> Signed-off-by: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
> ---

Detailed AI review found some issues:

Review of [PATCH v1] net/af_xdp: fix shared UMEM refcount corruption

Verified against DPDK main (26.11.0-rc0).  Applies cleanly; the driver
builds with -Dwerror=true at debugoptimized and minsize.  Fixes tag
74b46340e2d4 checks out and that commit does introduce the flaw.

Error:

1. drivers/net/af_xdp/rte_eth_af_xdp.c

   The new "return NULL" leaves rxq->mb_pool set while rxq->umem stays
   NULL: eth_rx_queue_setup() assigns rxq->mb_pool before calling
   xsk_configure(), and nothing clears it on the error path.

   get_shared_umem() then walks that stale entry and dereferences the
   NULL umem:

	if (mb_pool == internals->rx_queues[i].mb_pool) {
		if (ctx_exists(...))
			...
		if (rte_atomic_load_explicit(&internals->rx_queues[i].umem->refcnt,

   Any later queue setup using the same mempool crashes as soon as the
   failed rxq is the first match in the scan -- for example after the
   port that owns the UMEM is closed and removed from internal_list.

   The same scan can also reach a freed or over-shared pointer through
   the existing out_umem path in xsk_configure(), which calls
   xdp_umem_destroy(rxq->umem) without clearing rxq->umem, and leaves
   rxq->umem pointing at a still-live UMEM this rxq no longer holds a
   reference to.

   Both are latent today, but this patch makes reaching that state a
   routine outcome rather than an unusual one, so it should be closed
   here:

	/* in get_shared_umem() */
	if (internals->rx_queues[i].umem == NULL)
		continue;

   and clear rxq->umem unconditionally on the xsk_configure() error
   path, not only when the refcount reaches zero.

Warning:

2. doc/guides/nics/af_xdp.rst

   The new paragraph runs three sentences together across wrapped
   lines.  doc/guides/contributing/documentation.rst asks for one
   sentence per line, wrapped at punctuation points.

Info:

3. This is a behaviour change on a stable branch: setups that appeared
   to work (until close, or until the fill-queue crash) now fail at
   queue setup with -ENOMEM.  That is the right trade, but it is worth
   stating explicitly in the commit message for the stable maintainers.

4. The reflow of the rte_atomic_fetch_add_explicit() call is unrelated
   churn.  While the line is being touched: rte_memory_order_acquire on
   a refcount increment orders nothing useful; relaxed is sufficient
   there, and the matching decrement in eth_dev_close() wants release
   plus an acquire fence before xdp_umem_destroy().  Pre-existing, so
   only worth folding in if you are already rewriting the line.

5. The load and the increment are still not atomic with respect to each
   other -- get_shared_umem() releases internal_list_lock before
   returning, so two ports configured concurrently on the same mempool
   can both observe cnt < max_xsks and both increment past the cap.
   Control path, so the exposure is small, but a compare-exchange loop
   (or doing the check while holding internal_list_lock) is what
   actually enforces the limit the commit message describes.

6. refcnt is uint8_t while max_xsks is uint32_t.  A mempool of
   256 * 4096 mbufs or more produces max_xsks > 255, and the refcount
   wraps before the cap is ever reached.  Pre-existing.

7. "Port initialisation fails if the mempool is too small" -- it is the
   Rx queue setup that fails; "Queue setup fails" would be more precise
   and matches the -ENOMEM the caller returns.

[PATCH v1] net/af_xdp: fix shared UMEM refcount corruption

[sandeep.penigalapati]

From: Sandeep Penigalapati <sandeep.penigalapati@intel.com>

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: stable@dpdk.org

Signed-off-by: Sandeep Penigalapati <sandeep.penigalapati@intel.com>
---
 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