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

[Stephen Hemminger <stephen@networkplumber.org>]

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.