netdev - Re: [PATCH 2/2 bpf] xsk: avoid data corruption on cq descriptor number

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] [day] [month] [year] [list]

Message-Id: <5b54b160eacd11f9b17b8fed22313aa0c7344843b2a4d0b1a33553545b1b9a8e@mail.kernel.org>
Date: Tue, 28 Oct 2025 16:45:40 +0000 (UTC)
From: bot+bpf-ci@...nel.org
To: fmancera@...e.de,bpf@...r.kernel.org
Cc: netdev@...r.kernel.org,magnus.karlsson@...el.com,maciej.fijalkowski@...el.com,sdf@...ichev.me,kerneljasonxing@...il.com,fw@...len.de,fmancera@...e.de,ast@...nel.org,andrii@...nel.org,daniel@...earbox.net,martin.lau@...nel.org,eddyz87@...il.com,yonghong.song@...ux.dev,clm@...a.com,ihor.solodrai@...ux.dev
Subject: Re: [PATCH 2/2 bpf] xsk: avoid data corruption on cq descriptor number

```
commit 517548717cd8f9023f71443add6daabf21ef7135
Author: Fernando Fernandez Mancera <fmancera@...e.de>

xsk: avoid data corruption on cq descriptor number

This patch fixes data corruption in AF_XDP by migrating descriptor
information from skb->cb (which can be overwritten by other subsystems
after transmit) to skb XDP extension.

Closes: https://lore.kernel.org/netdev/0435b904-f44f-48f8-afb0-68868474bf1c@nop.hu/

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 7b0c68a70..4f3fc005d 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c

[ ... ]

> @@ -727,16 +730,32 @@ static struct sk_buff *xsk_build_skb_zerocopy(struct xdp_sock *xs,
>  				return ERR_PTR(err);
>  		}
>  	} else {
> +		struct xdp_skb_ext *ext;
> +
>  		xsk_addr = kmem_cache_zalloc(xsk_tx_generic_cache, GFP_KERNEL);
>  		if (!xsk_addr)
>  			return ERR_PTR(-ENOMEM);
>
> +		ext = skb_ext_find(skb, SKB_EXT_XDP);
> +		if (!ext) {
> +			ext = skb_ext_add(skb, SKB_EXT_XDP);
> +			if (!ext)
> +				return ERR_PTR(-ENOMEM);
                                ^^^^

In xsk_build_skb_zerocopy(), when skb_ext_add() fails after the
kmem_cache_zalloc() succeeds, does this leak the xsk_addr allocation?

The xsk_addr node is allocated first, then if skb_ext_add() returns
NULL, the function returns ERR_PTR(-ENOMEM) without freeing xsk_addr.
It hasn't been added to any list yet, so nothing will clean it up.

> +			memset(ext, 0, sizeof(*ext));
> +			INIT_LIST_HEAD(&ext->addrs_list);
> +			ext->num_descs = 1;
> +		} else if (ext->num_descs == 0) {
> +			INIT_LIST_HEAD(&ext->addrs_list);
> +			ext->num_descs = 1;
> +		}
> +
>  		/* in case of -EOVERFLOW that could happen below,
>  		 * xsk_consume_skb() will release this node as whole skb
>  		 * would be dropped, which implies freeing all list elements
>  		 */
>  		xsk_addr->addr = desc->addr;
> -		list_add_tail(&xsk_addr->addr_node, &XSKCB(skb)->addrs_list);
> +		list_add_tail(&xsk_addr->addr_node, &ext->addrs_list);
> +		xsk_inc_num_desc(skb);
>  	}

[ ... ]

> @@ -804,6 +823,11 @@ static struct sk_buff *xsk_build_skb(struct xdp_sock *xs,
>  			if (unlikely(err))
>  				goto free_err;
>
> +			if (!skb_ext_add(skb, SKB_EXT_XDP)) {
> +				err = -ENOMEM;
> +				goto free_err;
> +			}
> +
                ^^^^

In xsk_build_skb() on the first-descriptor path, when skb_ext_add()
succeeds, does the extension get initialized?

The code adds the extension but doesn't capture the returned pointer
or initialize it. Later, xsk_destruct_skb()->xsk_cq_submit_addr_locked()
will read ext->num_descs, and for multi-buffer packets, subsequent
descriptors will find this extension and try to use ext->addrs_list.

Without initialization, ext->num_descs contains garbage which could be
interpreted as > 1, and ext->addrs_list is an uninitialized list_head.
This can cause the destructor to iterate a corrupt list, or
list_add_tail() in the multi-buffer path to corrupt memory.

Compare with the subsequent-descriptor path at lines 856-870 where
the extension is properly initialized with memset() and INIT_LIST_HEAD()
when added.

>  			xsk_skb_init_misc(skb, xs, desc->addr);
>  			if (desc->options & XDP_TX_METADATA) {
>  				err = xsk_skb_metadata(skb, buffer, desc,

[ ... ]

```

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

In-Reply-To-Subject: `xsk: avoid data corruption on cq descriptor number`
CI run summary: https://github.com/kernel-patches/bpf/actions/runs/18881598580