[PATCH v1] net/af_xdp: fix shared UMEM refcount corruption
Stephen Hemminger
stephen at networkplumber.org
Wed Aug 12 22:56:56 CEST 2026
On Wed, 12 Aug 2026 18:08:43 -0400
sandeep.penigalapati at intel.com wrote:
> From: Sandeep Penigalapati <sandeep.penigalapati at 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 at dpdk.org
>
> Signed-off-by: Sandeep Penigalapati <sandeep.penigalapati at 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.
More information about the dev
mailing list