Re: [RFC net-next 11/15] xdp: Copy provider buffers on pass and redirect

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] net: xdp: payload truncation in xdp_build_skb_from_zc() when metadata unsupported
--

commit a36f92358b032a7e20669d832f27102e55f9f4eb
Author: Björn Töpel <bjorn@kernel.org>

xdp: Copy provider buffers on pass and redirect

This commit introduces logic to copy provider buffers into kernel pages
on pass and redirects, handling fragments appropriately. It splits the
copy from the release for redirects, maintaining the classic zero-copy
path while resolving frag addresses through xdp_frag_address().

> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 386240bd24c98..b7f16f44dac22 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
[ ... ]
> @@ -638,6 +718,9 @@ struct sk_buff *xdp_build_skb_from_buff(const struct xdp_buff *xdp)
>  	u32 nr_frags = 0;
>  	int metalen;
>  
> +	if (xdp_buff_has_netmem(xdp))
> +		return xdp_build_skb_from_zc(xdp);
> +
>  	if (unlikely(xdp_buff_has_frags(xdp))) {

[Severity: High]
Does generalizing xdp_build_skb_from_zc() to handle all netmem buffers cause
a payload truncation when metadata is unsupported?

Previously, xdp_build_skb_from_zc() only handled pure XSK buffers, where
data_meta == data is guaranteed. Now, it can receive generic provider-backed
netmem buffers.

If a netmem provider driver does not support metadata and initializes the
buffer via xdp_prepare_buff() with false, xdp->data_meta is set to
xdp->data + 1.

Looking at the length calculation and copy in xdp_build_skb_from_zc():

struct sk_buff *xdp_build_skb_from_zc(struct xdp_buff *xdp)
{
        u32 headroom = xdp->data_meta - xdp->data_hard_start;
        ...
        u32 len = xdp->data_end - xdp->data_meta;
        ...
        memcpy(__skb_put(skb, len), xdp->data_meta, len);
}

Without checking xdp_data_meta_unsupported(xdp), it blindly uses
xdp->data_meta to determine the length and as the source for memcpy().

Could this cause the first byte of every packet to be truncated and the
payload to be shifted by one byte, leading to silent data corruption on
XDP_PASS?

Should xdp_build_skb_from_zc() check xdp_data_meta_unsupported(xdp) similar
to how xdp_copy_zc_to_xdp_frame() handles it?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261002190018.696925-1-bjorn@kernel.org?part=11