Re: [PATCH] xsk: fix memory corruptions in net/core/xdp.c

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] The new dynamic `truesize` calculation tightly bounds the skb linear buffer size but fails to account for the `LARGEST_ALIGN(len)` padding used in the subsequent `memcpy()`. This leads to an out-of-bounds write that corrupts the `skb_shared_info` structure.
--

commit f8cfaa75cd9fcfb65c25981d740b6eedecb19f7b
Author: Clément Lecigne <clecigne@google.com>

xsk: fix memory corruptions in net/core/xdp.c

This commit attempts to fix a memory corruption in AF_XDP zero-copy handling
by dynamically calculating the truesize for skb allocation. It also fixes a
mismatch in xdp_copy_frags_from_zc() between allocation and copy sizes.

> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 9890a30584ba7..f36d1fb875ab8 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
[ ... ]
> @@ -740,7 +740,9 @@ struct sk_buff *xdp_build_skb_from_zc(struct xdp_buff *xdp)
>  {
>  	const struct xdp_rxq_info *rxq = xdp->rxq;
>  	u32 len = xdp->data_end - xdp->data_meta;
> -	u32 truesize = xdp->frame_sz;
> +	u32 headroom = xdp->data_meta - xdp->data_hard_start;
> +	u32 truesize = SKB_DATA_ALIGN(headroom + len) +
> +		       SKB_DATA_ALIGN(sizeof(struct skb_shared_info));

[Severity: Critical]
Does this tight allocation size account for the LARGEST_ALIGN(len) padding
used during the memcpy() below?

If an XDP program modifies the packet data making headroom unaligned, and 
headroom + len is close to a multiple of SMP_CACHE_BYTES, napi_build_skb()
will place the skb_shared_info structure exactly at 
SKB_DATA_ALIGN(headroom + len).

Could the LARGEST_ALIGN(len) size in the memcpy() below write past this
linear data area and corrupt the adjacent skb_shared_info structure?

>  	struct sk_buff *skb = NULL;
>  	struct page_pool *pp;
>  	int metalen;
[ ... ]
> @@ -762,7 +764,7 @@ struct sk_buff *xdp_build_skb_from_zc(struct xdp_buff *xdp)
>  	}
>  
>  	skb_mark_for_recycle(skb);
> -	skb_reserve(skb, xdp->data_meta - xdp->data_hard_start);
> +	skb_reserve(skb, headroom);
>  
>  	memcpy(__skb_put(skb, len), xdp->data_meta, LARGEST_ALIGN(len));

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260624084130.2382335-1-clecigne@google.com?part=1