[PATCH bpf] xsk: Do not free ZC Rx buffer when the Rx ring is full

[PATCH bpf] xsk: Do not free ZC Rx buffer when the Rx ring is full

[Marcin Knap via B4 Relay]

From: Marcin Knap <marcin.knap@vecima.com>

When an XDP program redirects a zero-copy buffer to an XSK and the
socket's Rx ring is full, xsk_rcv_zc() frees the buffer and returns
-ENOBUFS to xdp_do_redirect().

A failed xdp_do_redirect() leaves the buffer owned by the caller, and
drivers rely on that: mlx5e treats the failure as XDP_DROP and releases
the buffer when the Rx WQE is refilled, and i40e and ice free it on
XDP_CONSUMED. The buffer is therefore freed twice. It gets reused for
another Rx descriptor, the second free puts an in-use buffer back on
the pool free list, and the same UMEM frame is later handed to user
space twice. On mlx5 with striding RQ this also ended in a NULL pointer
dereference in mlx5e_post_rx_wqes().

Leave the buffer with the caller on both the single-buffer and the
multi-buffer -ENOBUFS paths, as xsk_rcv() and xsk_rcv_check() already
do. i40e and ice relied on the core freeing the buffer on their
need_wakeup -ENOBUFS (XDP_EXIT) path, so make them free it there.

Seen on Mellanox ConnectX-6 DX MT2892 (FW 22.47.1026) with AF_XDP
zero-copy on 7.3.0-rc6 when the user-space consumer could not keep up
with the Rx ring. With this change no duplicate frames were seen with
striding RQ on or off.

Fixes: 269009893146 ("xsk: recycle buffer in case Rx queue was full")
Signed-off-by: Marcin Knap <marcin.knap@vecima.com>
---
Tested on mlx5 (ConnectX-6 DX) in zero-copy single-buffer mode.
The i40e and ice changes and the multi-buffer path are build-tested
only, as I don't have that hardware available.
---
 drivers/net/ethernet/intel/i40e/i40e_xsk.c |  1 +
 drivers/net/ethernet/intel/ice/ice_xsk.c   |  1 +
 net/xdp/xsk.c                              | 15 +++------------
 3 files changed, 5 insertions(+), 12 deletions(-)

diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
index 9f47388eaba5..d32c7255f0b3 100644
--- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
+++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
@@ -358,6 +358,7 @@ static void i40e_handle_xdp_result_zc(struct i40e_ring *rx_ring,
 		return;
 
 	if (xdp_res == I40E_XDP_EXIT) {
+		xsk_buff_free(xdp_buff);
 		*failure = true;
 		return;
 	}
diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c
index 0643017541c3..7c1dd4c87a71 100644
--- a/drivers/net/ethernet/intel/ice/ice_xsk.c
+++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
@@ -636,6 +636,7 @@ int ice_clean_rx_irq_zc(struct ice_rx_ring *rx_ring,
 		if (likely(xdp_res & (ICE_XDP_TX | ICE_XDP_REDIR))) {
 			xdp_xmit |= xdp_res;
 		} else if (xdp_res == ICE_XDP_EXIT) {
+			xsk_buff_free(first);
 			failure = true;
 			first = NULL;
 			break;
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index 33475b180ea6..0edc160c660f 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -224,21 +224,15 @@ static int xsk_rcv_zc(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
 	struct list_head *xskb_list;
 	u32 contd = 0;
 	u32 num_desc;
-	int err;
 
-	if (likely(!frags)) {
-		err = __xsk_rcv_zc(xs, xskb, len, contd);
-		if (err)
-			goto err;
-		return 0;
-	}
+	if (likely(!frags))
+		return __xsk_rcv_zc(xs, xskb, len, contd);
 
 	contd = XDP_PKT_CONTD;
 	num_desc = xdp_get_shared_info_from_buff(xdp)->nr_frags + 1;
 	if (xskq_prod_nb_free(xs->rx, num_desc) < num_desc) {
 		xs->rx_queue_full++;
-		err = -ENOBUFS;
-		goto err;
+		return -ENOBUFS;
 	}
 
 	__xsk_rcv_zc_safe(xs, xskb, len, contd);
@@ -252,9 +246,6 @@ static int xsk_rcv_zc(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
 	}
 
 	return 0;
-err:
-	xsk_buff_free(xdp);
-	return err;
 }
 
 static void *xsk_copy_xdp_start(struct xdp_buff *from)

---
base-commit: ff47652a4b66c067c765a7ad464d930b5a9367cc
change-id: 20261019-xsk-zc-rx-double-free-1a3b059cd41f

Best regards,
--  
Marcin Knap <marcin.knap@vecima.com>

Re: [PATCH bpf] xsk: Do not free ZC Rx buffer when the Rx ring is full

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 3233 bytes --]

> xsk: Do not free ZC Rx buffer when the Rx ring is full

TL;DR: Since xsk_rcv_zc() no longer frees the buffer on a full Rx ring, gve
DQO zero-copy Rx may leak one UMEM frame per dropped packet and eventually
stall AF_XDP Rx; it likely needs xsk_buff_free().

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 33475b180ea6..0edc160c660f 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c

[ ... ]

> @@ -252,9 +246,6 @@ static int xsk_rcv_zc(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
>  	}
>
>  	return 0;
> -err:
> -	xsk_buff_free(xdp);
> -	return err;
>  }
>
>  static void *xsk_copy_xdp_start(struct xdp_buff *from)

Can this leak the UMEM frame in the gve DQO zero-copy Rx path?

With this change xsk_rcv_zc() no longer frees the buffer when the Rx ring
is full and it returns -ENOBUFS, so every zero-copy driver has to free or
keep the buffer itself when xdp_do_redirect() fails.  i40e and ice are
updated here, but gve DQO also relied on the core free:

drivers/net/ethernet/google/gve/gve_rx_dqo.c:gve_xsk_done_dqo() {
    ...
	case XDP_REDIRECT:
		if (unlikely(xdp_do_redirect(priv->dev, xdp, xprog)))
			goto err;
		break;
    ...
err:
	u64_stats_update_begin(&rx->statss);
	...
	if (xdp_act == XDP_REDIRECT)
		rx->xdp_redirect_errors++;
	u64_stats_update_end(&rx->statss);
}

The err: path only updates stats and never calls xsk_buff_free(xdp).

The caller, gve_rx_xsk_dqo(), then calls gve_free_buf_state(rx, buf_state),
which puts the buf_state back on the free list.  gve_alloc_buf_state() later
sets buf_state->xsk_buff = NULL without freeing it, and the ring-stop cleanup
only frees xsk buffers attached to allocated buf_states, so nothing calls
xp_free() on this buffer.

The path that gets there is:

gve_rx_dqo()
  gve_rx_xsk_dqo()
    gve_xsk_done_dqo()
      xdp_do_redirect()
        __xdp_do_redirect_xsk()
          __xsk_map_redirect()
            xsk_rcv()
              xsk_rcv_zc()
                __xsk_rcv_zc()
                  xskq_prod_reserve_desc() returns -ENOBUFS

Before this patch the err: label in xsk_rcv_zc() freed the buffer in exactly
this case, which is the scenario the commit targets (user space not keeping
up with the Rx ring).  After this patch gve DQO leaks one UMEM frame for
each packet dropped because the ring is full.  The frame never returns to
the pool free list or to user space, so under sustained overload
xsk_buff_alloc() starts failing in gve_alloc_buffer() and AF_XDP Rx on that
queue stalls until the socket is torn down.

The commit message says "A failed xdp_do_redirect() leaves the buffer owned
by the caller, and drivers rely on that" and names only i40e and ice as
relying on the core free.  Does gve_xsk_done_dqo() need an xsk_buff_free(xdp)
on the XDP_REDIRECT failure path as part of this change?

The other zero-copy drivers that were checked (ixgbe, igb, igc, libeth/idpf,
gve GQI, mlx5, tsnep, fec, dpaa2, nfp, stmmac, icssg and virtio_net) already
free, recycle or keep the buffer on redirect failure.


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/39223384647

Re: [PATCH bpf] xsk: Do not free ZC Rx buffer when the Rx ring is full

[Maciej Fijalkowski]

On Mon, Oct 19, 2026 at 10:22:02AM +0200, Marcin Knap via B4 Relay wrote:
> From: Marcin Knap <marcin.knap@vecima.com>
> 
> When an XDP program redirects a zero-copy buffer to an XSK and the
> socket's Rx ring is full, xsk_rcv_zc() frees the buffer and returns
> -ENOBUFS to xdp_do_redirect().
> 
> A failed xdp_do_redirect() leaves the buffer owned by the caller, and
> drivers rely on that: mlx5e treats the failure as XDP_DROP and releases

Sashiko report shows that drivers are rather inconsistent here

> the buffer when the Rx WQE is refilled, and i40e and ice free it on
> XDP_CONSUMED. The buffer is therefore freed twice. It gets reused for
> another Rx descriptor, the second free puts an in-use buffer back on
> the pool free list, and the same UMEM frame is later handed to user
> space twice. 

Did you verify this or is this rather a speculation? xp_free() protects
against double-free cases.

> On mlx5 with striding RQ this also ended in a NULL pointer
> dereference in mlx5e_post_rx_wqes().

Please more info on this then.

> 
> Leave the buffer with the caller on both the single-buffer and the
> multi-buffer -ENOBUFS paths, as xsk_rcv() and xsk_rcv_check() already
> do. i40e and ice relied on the core freeing the buffer on their
> need_wakeup -ENOBUFS (XDP_EXIT) path, so make them free it there.
> 
> Seen on Mellanox ConnectX-6 DX MT2892 (FW 22.47.1026) with AF_XDP
> zero-copy on 7.3.0-rc6 when the user-space consumer could not keep up
> with the Rx ring. With this change no duplicate frames were seen with
> striding RQ on or off.

Have you thought about massaging mlx driver instead of intel + core?
Even when that would be correct approach ixgbe would miss being addressed.

> 
> Fixes: 269009893146 ("xsk: recycle buffer in case Rx queue was full")
> Signed-off-by: Marcin Knap <marcin.knap@vecima.com>
> ---
> Tested on mlx5 (ConnectX-6 DX) in zero-copy single-buffer mode.
> The i40e and ice changes and the multi-buffer path are build-tested
> only, as I don't have that hardware available.
> ---
>  drivers/net/ethernet/intel/i40e/i40e_xsk.c |  1 +
>  drivers/net/ethernet/intel/ice/ice_xsk.c   |  1 +
>  net/xdp/xsk.c                              | 15 +++------------
>  3 files changed, 5 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
> index 9f47388eaba5..d32c7255f0b3 100644
> --- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c
> +++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c
> @@ -358,6 +358,7 @@ static void i40e_handle_xdp_result_zc(struct i40e_ring *rx_ring,
>  		return;
>  
>  	if (xdp_res == I40E_XDP_EXIT) {
> +		xsk_buff_free(xdp_buff);
>  		*failure = true;
>  		return;
>  	}
> diff --git a/drivers/net/ethernet/intel/ice/ice_xsk.c b/drivers/net/ethernet/intel/ice/ice_xsk.c
> index 0643017541c3..7c1dd4c87a71 100644
> --- a/drivers/net/ethernet/intel/ice/ice_xsk.c
> +++ b/drivers/net/ethernet/intel/ice/ice_xsk.c
> @@ -636,6 +636,7 @@ int ice_clean_rx_irq_zc(struct ice_rx_ring *rx_ring,
>  		if (likely(xdp_res & (ICE_XDP_TX | ICE_XDP_REDIR))) {
>  			xdp_xmit |= xdp_res;
>  		} else if (xdp_res == ICE_XDP_EXIT) {
> +			xsk_buff_free(first);
>  			failure = true;
>  			first = NULL;
>  			break;
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 33475b180ea6..0edc160c660f 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -224,21 +224,15 @@ static int xsk_rcv_zc(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
>  	struct list_head *xskb_list;
>  	u32 contd = 0;
>  	u32 num_desc;
> -	int err;
>  
> -	if (likely(!frags)) {
> -		err = __xsk_rcv_zc(xs, xskb, len, contd);
> -		if (err)
> -			goto err;
> -		return 0;
> -	}
> +	if (likely(!frags))
> +		return __xsk_rcv_zc(xs, xskb, len, contd);
>  
>  	contd = XDP_PKT_CONTD;
>  	num_desc = xdp_get_shared_info_from_buff(xdp)->nr_frags + 1;
>  	if (xskq_prod_nb_free(xs->rx, num_desc) < num_desc) {
>  		xs->rx_queue_full++;
> -		err = -ENOBUFS;
> -		goto err;
> +		return -ENOBUFS;
>  	}
>  
>  	__xsk_rcv_zc_safe(xs, xskb, len, contd);
> @@ -252,9 +246,6 @@ static int xsk_rcv_zc(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
>  	}
>  
>  	return 0;
> -err:
> -	xsk_buff_free(xdp);
> -	return err;
>  }
>  
>  static void *xsk_copy_xdp_start(struct xdp_buff *from)
> 
> ---
> base-commit: ff47652a4b66c067c765a7ad464d930b5a9367cc
> change-id: 20261019-xsk-zc-rx-double-free-1a3b059cd41f
> 
> Best regards,
> --  
> Marcin Knap <marcin.knap@vecima.com>
> 
>