On Fri, Oct 30, 2026 at 09:14:17AM +0100, Marcin Knap via B4 Relay wrote: > From: Marcin Knap <[email protected]> > > 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 > most drivers rely on that: mlx5e treats the failure as XDP_DROP and > releases the buffer when the Rx WQE is refilled, while i40e, ice, > ixgbe, igb, igc, stmmac, idpf and others free it themselves. 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. ixgbe and igb keep the buffer in the ring on their need_wakeup > -ENOBUFS (XDP_EXIT) path and run it through XDP again on the next poll, > after the core has freed it. > > 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. A few drivers relied on the core freeing the buffer, so make them > free it: > > - i40e and ice consume the descriptor on XDP_EXIT without freeing it. > - gve DQO counts the redirect error without freeing the buffer. > - octeontx2 returns false on a failed redirect, so the caller goes on > to build an skb from the already freed buffer. Free the buffer and > report it as consumed instead. > > 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 <[email protected]> > --- > 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. > --- > Changes in v2: > - Also free the buffer on zero-copy redirect failure in gve DQO and > octeontx2, which relied on the core freeing it (AI review). > - Audited the remaining zero-copy drivers against Björn's table. > ixgbe and igb no longer run XDP on a freed buffer after > XDP_EXIT; making them drop and consume the descriptor (R2) is > left for a follow-up unless you prefer it in this patch. > - Rebased on current bpf. > - Link to v1: > https://patch.msgid.link/[email protected] > > To: Joshua Washington <[email protected]> > To: Harshitha Ramamurthy <[email protected]> > To: Andrew Lunn <[email protected]> > To: "David S. Miller" <[email protected]> > To: Eric Dumazet <[email protected]> > To: Jakub Kicinski <[email protected]> > To: Paolo Abeni <[email protected]> > To: Tony Nguyen <[email protected]> > To: Przemek Kitszel <[email protected]> > To: Sunil Goutham <[email protected]> > To: Geetha sowjanya <[email protected]> > To: Ratheesh Kannoth <[email protected]> > To: Subbaraya Sundeep <[email protected]> > To: Bharat Bhushan <[email protected]> > To: Magnus Karlsson <[email protected]> > To: Maciej Fijalkowski <[email protected]> > To: Stanislav Fomichev <[email protected]> > To: Simon Horman <[email protected]> > To: Alexei Starovoitov <[email protected]> > To: Daniel Borkmann <[email protected]> > To: Jesper Dangaard Brouer <[email protected]> > To: John Fastabend <[email protected]> > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > Cc: [email protected] > --- > drivers/net/ethernet/google/gve/gve_rx_dqo.c | 4 +++- > drivers/net/ethernet/intel/i40e/i40e_xsk.c | 1 + > drivers/net/ethernet/intel/ice/ice_xsk.c | 1 + > drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c | 3 ++- > net/xdp/xsk.c | 15 +++------------ > 5 files changed, 10 insertions(+), 14 deletions(-) > > diff --git a/drivers/net/ethernet/google/gve/gve_rx_dqo.c > b/drivers/net/ethernet/google/gve/gve_rx_dqo.c > index 82ed5e48b00a..cc544420df11 100644 > --- a/drivers/net/ethernet/google/gve/gve_rx_dqo.c > +++ b/drivers/net/ethernet/google/gve/gve_rx_dqo.c > @@ -686,8 +686,10 @@ static void gve_xsk_done_dqo(struct gve_priv *priv, > struct gve_rx_ring *rx, > goto err; > break; > case XDP_REDIRECT: > - if (unlikely(xdp_do_redirect(priv->dev, xdp, xprog))) > + if (unlikely(xdp_do_redirect(priv->dev, xdp, xprog))) { > + xsk_buff_free(xdp); > goto err; > + } > break; > } > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_xsk.c > b/drivers/net/ethernet/intel/i40e/i40e_xsk.c > index 51a05ce4c7ce..a11ee1f3fa38 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_xsk.c > +++ b/drivers/net/ethernet/intel/i40e/i40e_xsk.c > @@ -360,6 +360,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);
This seems to be enough to avoid double-free on non-need_wakeup case when core was releasing the buffer. Reviewed-by: Maciej Fijalkowski <[email protected]> One nit is AF_XDP patches are routed via net/net-next since some time, but I don't know if you have to resend or not. > failure = true; > first = NULL; > break; > diff --git a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c > b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c > index 90af9b9044aa..6d10c132f61c 100644 > --- a/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c > +++ b/drivers/net/ethernet/marvell/octeontx2/nic/otx2_txrx.c > @@ -1555,7 +1555,8 @@ static bool otx2_xdp_rcv_pkt_handler(struct otx2_nic > *pfvf, > *need_xdp_flush = true; > return true; > } > - return false; > + xsk_buff_free(xsk_buff); > + return true; > } > > err = xdp_do_redirect(pfvf->netdev, &xdp, prog); > diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c > index 781ba46eb152..89bac2feeff2 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: fb7bd3a2aad70a76232a7168a505386d0a1396af > change-id: 20261019-xsk-zc-rx-double-free-1a3b059cd41f > > Best regards, > -- > Marcin Knap <[email protected]> > >
