* [PATCH v1] net/af_xdp: fix shared UMEM refcount corruption
@ 2026-08-12 22:08 sandeep.penigalapati
2026-08-12 20:56 ` Stephen Hemminger
0 siblings, 1 reply; 2+ messages in thread
From: sandeep.penigalapati @ 2026-08-12 22:08 UTC (permalink / raw)
To: dev; +Cc: Ciara Loftus, Maryam Tahhan, stable, 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
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH v1] net/af_xdp: fix shared UMEM refcount corruption
2026-08-12 22:08 [PATCH v1] net/af_xdp: fix shared UMEM refcount corruption sandeep.penigalapati
@ 2026-08-12 20:56 ` Stephen Hemminger
0 siblings, 0 replies; 2+ messages in thread
From: Stephen Hemminger @ 2026-08-12 20:56 UTC (permalink / raw)
To: sandeep.penigalapati; +Cc: dev, Ciara Loftus, Maryam Tahhan, stable
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.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-08-12 20:57 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-12 22:08 [PATCH v1] net/af_xdp: fix shared UMEM refcount corruption sandeep.penigalapati
2026-08-12 20:56 ` Stephen Hemminger
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.