Re: [PATCH v2] 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 exact dynamic allocation sizing in xdp_build_skb_from_zc() combined with a rounded-up memcpy() introduces a reliable out-of-bounds write into skb_shared_info.
--

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

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

This commit attempts to fix out of bounds writes during AF_XDP zero-copy
frame conversions. It introduces dynamic truesize calculations using
SKB_HEAD_ALIGN for the skb allocation and LARGEST_ALIGN for fragment
allocations to properly account for required headroom and packet length.

> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 9890a30584ba7..52546746378a0 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
[ ... ]
> @@ -738,9 +738,10 @@ static noinline bool xdp_copy_frags_from_zc(struct sk_buff *skb,
>   */
>  struct sk_buff *xdp_build_skb_from_zc(struct xdp_buff *xdp)
>  {
> +	u32 headroom = xdp->data_meta - xdp->data_hard_start;
>  	const struct xdp_rxq_info *rxq = xdp->rxq;
>  	u32 len = xdp->data_end - xdp->data_meta;
> -	u32 truesize = xdp->frame_sz;
> +	u32 truesize = SKB_HEAD_ALIGN(headroom + len);

[Severity: Critical]
Does this regression allocate enough linear space to handle a rounded-up
memcpy when the headroom is unaligned?

If an XDP program adjusts data_meta such that headroom is not a multiple of
64 (for example, 252 bytes), and a packet of 68 bytes is received, the
allocated linear size from SKB_HEAD_ALIGN(320) would be 320.

However, the later memcpy uses LARGEST_ALIGN(len) for the 68 bytes, which
could round up to 96 bytes.  This means 252 + 96 = 348 bytes could be
written into a 320 byte space.

[ ... ]
> @@ -762,7 +763,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));

[Severity: Critical]
Can this regression corrupt skb_shared_info?

Since the linear space allocation might only be 320 bytes as shown above,
but this memcpy could write up to 348 bytes, it appears this might write
past the end of the linear data region.

Could this lead to overwriting destructor_arg and cause issues upon SKB
free?

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