Re: [PATCH net] xsk: reset the mac header in the generic Tx path

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Incomplete fix leaves skb->network_header and skb->transport_header uninitialized in generic XDP TX, causing out-of-bounds reads during checksum offload.
--

--- Patch [1]: [PATCH net] xsk: reset the mac header in the generic Tx path ---
commit 28a4ae00fcc18f8550bef3cea318942f60fe0720
Author: Petr Oros <poros@redhat.com>

xsk: reset the mac header in the generic Tx path

This commit fixes a bug where AF_XDP generic Tx leaves the MAC header
uninitialized, leading to out-of-bounds reads in drivers. It sets the
MAC header in xsk_skb_init_misc() to ensure the pointer stays in bounds.

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 7855ee09c4b64..885427392cb6b 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -931,6 +931,7 @@ static int xsk_skb_init_misc(struct sk_buff *skb, struct xdp_sock *xs,
>  	skb->priority = READ_ONCE(xs->sk.sk_priority);
>  	skb->mark = READ_ONCE(xs->sk.sk_mark);
>  	skb->destructor = xsk_destruct_skb;
> +	skb_reset_mac_header(skb);

[Severity: High]
This is a pre-existing issue and was not introduced by the patch under review,
but does this incomplete fix leave skb->network_header and
skb->transport_header uninitialized?

If a userspace application uses an AF_XDP socket in generic TX mode with
the XDP_TXMD_FLAGS_CHECKSUM metadata flag set, xsk_skb_metadata() will set
skb->ip_summed = CHECKSUM_PARTIAL without initializing the network or
transport headers.

When the packet is passed to a NIC driver (like ice) that processes
CHECKSUM_PARTIAL, it reads skb_network_header() and skb_transport_header():

drivers/net/ethernet/intel/ice/ice_txrx.c:ice_tx_csum() {
    ...
		ip.hdr = skb_network_header(skb);
		l4.hdr = skb_transport_header(skb);
    ...
}

Because these headers are never initialized, network_header remains 0
(pointing to uninitialized SKB headroom) and transport_header remains ~0U
(65535 bytes past skb->head).

Could this lead to out-of-bounds reads when drivers dereference these
pointers, requiring us to safely anchor or parse the headers here as well
(for example, via skb_probe_transport_header() as was previously done for
AF_PACKET)?

>  	return 0;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260817153957.3177627-1-poros@redhat.com?part=1