netdev - Re: [PATCH net v2 1/2] xsk: Fix fragment node deletion to prevent buffer leak

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <aYpRusvnFvzCmtpG@boxer>
Date: Mon, 9 Feb 2026 22:29:30 +0100
From: Maciej Fijalkowski <maciej.fijalkowski@...el.com>
To: "Nikhil P. Rao" <nikhil.rao@....com>
CC: <netdev@...r.kernel.org>, <magnus.karlsson@...el.com>, <sdf@...ichev.me>,
	<davem@...emloft.net>, <edumazet@...gle.com>, <kuba@...nel.org>,
	<pabeni@...hat.com>, <horms@...nel.org>, <kerneljasonxing@...il.com>
Subject: Re: [PATCH net v2 1/2] xsk: Fix fragment node deletion to prevent
 buffer leak

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@....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@...el.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
>