[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

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] xsk: missing xsk_buff_free() on redirect error in gve and octeontx2
--

commit 2859b3e778dcb29dabcd4c998977aac223d207b7
Author: Marcin Knap <marcin.knap@vecima.com>

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

This commit stops xsk_rcv_zc() from calling xsk_buff_free() when the XSK Rx
ring is full (-ENOBUFS) so that callers of xdp_do_redirect() do not
double-free the zero-copy buffer. It also updates i40e and ice to free the
buffer on their XDP_EXIT paths.

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 33475b180ea6f..0edc160c660fd 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;
>  }

[Severity: High]
Does removing xsk_buff_free(xdp) here leak xdp_buffs in gve and octeontx2
when xdp_do_redirect() fails with -ENOBUFS?

In drivers/net/ethernet/google/gve/gve_rx_dqo.c, the call chain
gve_rx_poll_dqo() -> gve_rx_dqo() -> gve_rx_xsk_dqo() -> gve_xsk_done_dqo()
handles XDP_REDIRECT on zero-copy buffers without freeing xdp on error:

	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_TX)
		rx->xdp_tx_errors++;
	if (xdp_act == XDP_REDIRECT)
		rx->xdp_redirect_errors++;
	u64_stats_update_end(&rx->statss);

Its caller gve_rx_xsk_dqo() then releases buf_state via
gve_free_buf_state(rx, buf_state) without freeing xdp, leaking it from
rx->xsk_pool.

Similarly, in drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c, the
call chain otx2_napi_handler() -> otx2_rx_napi_handler() ->
otx2_rcv_pkt_handler() -> otx2_xdp_rcv_pkt_handler() pops xsk_buff from
pool->xdp[--cq->rbpool->xdp_top] and does not free it if xdp_do_redirect()
fails:

	case XDP_REDIRECT:
		cq->pool_ptrs++;
		if (xsk_buff) {
			err = xdp_do_redirect(pfvf->netdev, xsk_buff, prog);
			if (!err) {
				*need_xdp_flush = true;
				return true;
			}
			return false;
		}

Should gve_xsk_done_dqo() and otx2_xdp_rcv_pkt_handler() also be updated
to free the zero-copy buffer when xdp_do_redirect() fails?

>  static void *xsk_copy_xdp_start(struct xdp_buff *from)

[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019-xsk-zc-rx-double-free-v1-1-2aa19f6628fb@vecima.com?part=1

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>
> 
> 

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

[Magnus Karlsson]

On Tue, 20 Oct 2026 at 00:14, Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> 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

A note to ourselves, we should write a section in the documentation
for AF_XDP that defines the expected driver behavior. There are very
likely more inconsistencies out there since this was never explicitly
specified anywhere.

> > 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>
> >
> >
>

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

[Marcin Knap]

> Sashiko report shows that drivers are rather inconsistent here
Yes I missed it, I am working on fixes.

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

So in my case I received kernel panic when I turned of striding rq off and the rx ring was full, but the panic itself was mitigated by the fixes from @Jerome Tollet in the mlx driver. I described the problem with linked fixes here:
https://forums.developer.nvidia.com/t/ofed-backport-of-mainline-mlx5e-xsk-stale-buffer-release-fix-kernel-panic-in-mlx5e-post-rx-wqes/384752/5 
But the real root cause is the problem I'm trying to fix.

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

I verified this, the origin bug on our own was that we received the same UMEM frame several twice even we haven't push it to the ring, that was causing serious damages in our system, we need to add special detection duplicated frames after receiving frames on rx ring, when I removed this additional xdp free, the problem was gone.


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

I was considering and I did as workaround, but IMO the final fix should be placed in the AF XDP, because there is inconsistency between different kind of errors, look at xsk_rcv_check() there isn't any freeing buffer, so why only ENOBUFS causes that? Maybe there is some reason that I don't know. Moreover, if I fix it only in mlx, the problem in the other driver at certain path will persist including Intel, only ZC with needs_wakup flag path is properly handled there. Yes ixgbe is my gap, I missed it and I can apply the patch there as well.  But eventually this is your call where to fix it, of course. 

> A note to ourselves, we should write a section in the documentation
> for AF_XDP that defines the expected driver behavior. There are very
> likely more inconsistencies out there since this was never explicitly
> specified anywhere

This is good point, some documented unification of behavor across happy path and different kind of errors would be nice.


________________________________________
From: Magnus Karlsson <magnus.karlsson@gmail.com>
Sent: Tuesday, 20 October 2026 08:09
To: Maciej Fijalkowski
Cc: Marcin Knap; Tony Nguyen; Przemek Kitszel; Andrew Lunn; David S. Miller; Eric Dumazet; Jakub Kicinski; Paolo Abeni; Magnus Karlsson; Stanislav Fomichev; Simon Horman; Alexei Starovoitov; Daniel Borkmann; Jesper Dangaard Brouer; John Fastabend; jtollet@cisco.com; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; bpf@vger.kernel.org
Subject: Re: [PATCH bpf] xsk: Do not free ZC Rx buffer when the Rx ring is full

CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.


[You don't often get email from magnus.karlsson@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]

On Tue, 20 Oct 2026 at 00:14, Maciej Fijalkowski
<maciej.fijalkowski@intel.com> wrote:
>
> 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

A note to ourselves, we should write a section in the documentation
for AF_XDP that defines the expected driver behavior. There are very
likely more inconsistencies out there since this was never explicitly
specified anywhere.

> > 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>
> >
> >
>

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

[Maciej Fijalkowski]

On Tue, Oct 20, 2026 at 07:00:47AM +0000, Marcin Knap wrote:
> > Sashiko report shows that drivers are rather inconsistent here
> Yes I missed it, I am working on fixes.

Please don't top-post on MLs and configure your mail client to wrap your
lines. Kernel people get triggered by this :P

> 
> > On mlx5 with striding RQ this also ended in a NULL pointer
> > dereference in mlx5e_post_rx_wqes(). 
> 
> So in my case I received kernel panic when I turned of striding rq off and the rx ring was full, but the panic itself was mitigated by the fixes from @Jerome Tollet in the mlx driver. I described the problem with linked fixes here:
> https://forums.developer.nvidia.com/t/ofed-backport-of-mainline-mlx5e-xsk-stale-buffer-release-fix-kernel-panic-in-mlx5e-post-rx-wqes/384752/5 
> But the real root cause is the problem I'm trying to fix.
> 
> > Did you verify this or is this rather a speculation? xp_free() protects
> > against double-free cases.
> 
> I verified this, the origin bug on our own was that we received the same UMEM frame several twice even we haven't push it to the ring, that was causing serious damages in our system, we need to add special detection duplicated frames after receiving frames on rx ring, when I removed this additional xdp free, the problem was gone.
> 
> 
> > Have you thought about massaging mlx driver instead of intel + core?
> > Even when that would be correct approach ixgbe would miss being addressed.
> 
> I was considering and I did as workaround, but IMO the final fix should be placed in the AF XDP, because there is inconsistency between different kind of errors, look at xsk_rcv_check() there isn't any freeing buffer, so why only ENOBUFS causes that? Maybe there is some reason that I don't know. Moreover, if I fix it only in mlx, the problem in the other driver at certain path will persist including Intel, only ZC with needs_wakup flag path is properly handled there. Yes ixgbe is my gap, I missed it and I can apply the patch there as well.  But eventually this is your call where to fix it, of course. 

ZC drivers are supposed to break processing and yield cpu to application
when xsk rxq gets full. We didn't want to expose particular packet
multiple times to XDP progs as that might confuse internal logic of progs.
Therefore we drop the packet in the core and let driver side know about
this via ENOBUFS return value.

Intel side needs fixing for non-need-wakeup case as you point out.

> 
> > A note to ourselves, we should write a section in the documentation
> > for AF_XDP that defines the expected driver behavior. There are very
> > likely more inconsistencies out there since this was never explicitly
> > specified anywhere
> 
> This is good point, some documented unification of behavor across happy path and different kind of errors would be nice.

+1 also, I assume rxq full case needs documentation.

Marcin, could you re-iterate why Jerome's fixes were not enough for you?
Would also be nice to align mlx-5 to -ENOBUFS case.

Thanks,
Maciej

> 
> 
> ________________________________________
> From: Magnus Karlsson <magnus.karlsson@gmail.com>
> Sent: Tuesday, 20 October 2026 08:09
> To: Maciej Fijalkowski
> Cc: Marcin Knap; Tony Nguyen; Przemek Kitszel; Andrew Lunn; David S. Miller; Eric Dumazet; Jakub Kicinski; Paolo Abeni; Magnus Karlsson; Stanislav Fomichev; Simon Horman; Alexei Starovoitov; Daniel Borkmann; Jesper Dangaard Brouer; John Fastabend; jtollet@cisco.com; intel-wired-lan@lists.osuosl.org; netdev@vger.kernel.org; linux-kernel@vger.kernel.org; bpf@vger.kernel.org
> Subject: Re: [PATCH bpf] xsk: Do not free ZC Rx buffer when the Rx ring is full
> 
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you recognize the sender and know the content is safe.
> 
> 
> [You don't often get email from magnus.karlsson@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
> 
> On Tue, 20 Oct 2026 at 00:14, Maciej Fijalkowski
> <maciej.fijalkowski@intel.com> wrote:
> >
> > 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
> 
> A note to ourselves, we should write a section in the documentation
> for AF_XDP that defines the expected driver behavior. There are very
> likely more inconsistencies out there since this was never explicitly
> specified anywhere.
> 
> > > 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>
> > >
> > >
> >

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

[Marcin Knap]

On Tue, Oct 20, 2026 at 07:00:47AM +0000, Maciej Fijalkowski wrote:

> Please don't top-post on MLs and configure your mail client to wrap your
> lines. Kernel people get triggered by this :P

Sorry for that, I was trying but not good enough... 
cannot setup this on web outlook and I have blocked every other client.

> ZC drivers are supposed to break processing and yield cpu to
> application when xsk rxq gets full. We didn't want to expose
> particular packet multiple times to XDP progs as that might confuse
> internal logic of progs. Therefore we drop the packet in the core and
> let driver side know about this via ENOBUFS return value.

Thanks, that explains it and it makes sense to me. 
I will drop the core change and fix the
drivers that free the buffer again after -ENOBUFS instead.

> Marcin, could you re-iterate why Jerome's fixes were not enough for
> you?

This is how I understand this but I don't know this code well and I can be
wrong.
Jerome's patches stop mlx5e from releasing the same WQE buffer twice
when a refill is retried. They don't cover the case where the core
has already freed the buffer on -ENOBUFS. mlx5e_xdp_handle() treats
the failed redirect as XDP_DROP, skip-release isn't set, and the
buffer is released again when its slot is refilled.

xp_free() only catches this if the buffer is still on the free list.
The free list is LIFO and refill allocates replacement buffers in
bulk, so the buffer freed by the core is usually handed to another
WQE before its old slot is refilled. The second free then puts a
buffer that is still in use back on the free list, and the same UMEM
frame is later published twice.

> Would also be nice to align mlx-5 to -ENOBUFS case.

Will do, I have potential mlx5 fix (this was my workaround).

I did quick analysis with AI tool and it sounds like all drivers have 
similar problem and they don't handle -ENOBUFS correctly except 
Intel drivers needs_wakup path. Given that, would you prefer the driver 
fixes, or do you have any other ideas?

Thanks,
Marcin