All of lore.kernel.org
 help / color / mirror / Atom feed
From: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
To: "Nikhil P. Rao" <nikhil.rao@amd.com>
Cc: <netdev@vger.kernel.org>, <magnus.karlsson@intel.com>,
	<sdf@fomichev.me>, <davem@davemloft.net>, <edumazet@google.com>,
	<kuba@kernel.org>, <pabeni@redhat.com>, <horms@kernel.org>,
	<kerneljasonxing@gmail.com>
Subject: Re: [PATCH net v2 1/2] xsk: Fix fragment node deletion to prevent buffer leak
Date: Mon, 9 Feb 2026 22:29:30 +0100	[thread overview]
Message-ID: <aYpRusvnFvzCmtpG@boxer> (raw)
In-Reply-To: <20260209182642.237904-2-nikhil.rao@amd.com>

On Mon, Feb 09, 2026 at 06:24:50PM +0000, Nikhil P. Rao wrote:
> After commit b692bf9a7543 ("xsk: Get rid of xdp_buff_xsk::xskb_list_node"),
> the list_node field is reused for both the xskb pool list and the buffer
> free list, this causes a buffer leak as described below.
> 
> xp_free() checks if a buffer is already on the free list using
> list_empty(&xskb->list_node). When list_del() is used to remove a node
> from the xskb pool list, it doesn't reinitialize the node pointers.
> This means list_empty() will return false even after the node has been
> removed, causing xp_free() to incorrectly skip adding the buffer to the
> free list.
> 
> Fix this by using list_del_init() instead of list_del() in all fragment
> handling paths, this ensures the list node is reinitialized after removal,
> allowing the list_empty() to work correctly.
> 
> Fixes: b692bf9a7543 ("xsk: Get rid of xdp_buff_xsk::xskb_list_node")
> Signed-off-by: Nikhil P. Rao <nikhil.rao@amd.com>

Nice catch!

I'm curious how did you spot this. I assume it was a mix of XDP_DROP/PASS
action returned by your program in data path with mbuf as that's the path
that was affected. I wonder if we need to come up with a test case for
xskxceiver to cover this?

Acked-by: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

> ---
>  include/net/xdp_sock_drv.h | 6 +++---
>  net/xdp/xsk.c              | 2 +-
>  2 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/net/xdp_sock_drv.h b/include/net/xdp_sock_drv.h
> index 242e34f771cc..aefc368449d5 100644
> --- a/include/net/xdp_sock_drv.h
> +++ b/include/net/xdp_sock_drv.h
> @@ -122,7 +122,7 @@ static inline void xsk_buff_free(struct xdp_buff *xdp)
>  		goto out;
>  
>  	list_for_each_entry_safe(pos, tmp, xskb_list, list_node) {
> -		list_del(&pos->list_node);
> +		list_del_init(&pos->list_node);
>  		xp_free(pos);
>  	}
>  
> @@ -157,7 +157,7 @@ static inline struct xdp_buff *xsk_buff_get_frag(const struct xdp_buff *first)
>  	frag = list_first_entry_or_null(&xskb->pool->xskb_list,
>  					struct xdp_buff_xsk, list_node);
>  	if (frag) {
> -		list_del(&frag->list_node);
> +		list_del_init(&frag->list_node);
>  		ret = &frag->xdp;
>  	}
>  
> @@ -168,7 +168,7 @@ static inline void xsk_buff_del_frag(struct xdp_buff *xdp)
>  {
>  	struct xdp_buff_xsk *xskb = container_of(xdp, struct xdp_buff_xsk, xdp);
>  
> -	list_del(&xskb->list_node);
> +	list_del_init(&xskb->list_node);
>  }
>  
>  static inline struct xdp_buff *xsk_buff_get_head(struct xdp_buff *first)
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index f093c3453f64..f2ec4f78bbb6 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -186,7 +186,7 @@ static int xsk_rcv_zc(struct xdp_sock *xs, struct xdp_buff *xdp, u32 len)
>  		err = __xsk_rcv_zc(xs, pos, len, contd);
>  		if (err)
>  			goto err;
> -		list_del(&pos->list_node);
> +		list_del_init(&pos->list_node);
>  	}
>  
>  	return 0;
> -- 
> 2.43.0
> 

  reply	other threads:[~2026-02-09 21:29 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-09 18:24 [PATCH net v2 0/2] xsk: Fixes for AF_XDP fragment handling Nikhil P. Rao
2026-02-09 18:24 ` [PATCH net v2 1/2] xsk: Fix fragment node deletion to prevent buffer leak Nikhil P. Rao
2026-02-09 21:29   ` Maciej Fijalkowski [this message]
2026-02-09 18:24 ` [PATCH net v2 2/2] xsk: Fix zero-copy AF_XDP fragment drop Nikhil P. Rao
2026-02-09 21:55   ` Maciej Fijalkowski
2026-02-10 16:19     ` Maciej Fijalkowski
2026-02-10 21:10       ` Rao, Nikhil

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aYpRusvnFvzCmtpG@boxer \
    --to=maciej.fijalkowski@intel.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=horms@kernel.org \
    --cc=kerneljasonxing@gmail.com \
    --cc=kuba@kernel.org \
    --cc=magnus.karlsson@intel.com \
    --cc=netdev@vger.kernel.org \
    --cc=nikhil.rao@amd.com \
    --cc=pabeni@redhat.com \
    --cc=sdf@fomichev.me \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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.