Re: [PATCH net v4 1/5] xsk: cache csum_start/csum_offset to fix TOCTOU in xsk_skb_metadata()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Truncation of `hr + csum_start` to a 16-bit unsigned integer on systems with large pages (e.g., 64KB) leads to a negative checksum offset, causing out-of-bounds memory accesses and potential hardware DMA faults.
--

commit 7a5d0ad0065df9dde9ff7b2ebb8561268805c4e9
Author: Jason Xing <kerneljasonxing@gmail.com>

xsk: cache csum_start/csum_offset to fix TOCTOU in xsk_skb_metadata()
    
This commit fixes a TOCTOU vulnerability in XSK TX metadata processing. It
reads csum_start and csum_offset from shared memory into local variables
once, then uses the local copies for both validation and assignment.
    
Closes: https://lore.kernel.org/all/20260503200927.73EA1C2BCB4@smtp.kernel.org/

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 5e5786cd9af55..f8c8a8c9dfba5 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -802,6 +802,7 @@ static int xsk_skb_metadata(struct sk_buff *skb, void *buffer,
[ ... ]
>  	if (meta->flags & XDP_TXMD_FLAGS_CHECKSUM) {
> -		if (unlikely(meta->request.csum_start +
> -			     meta->request.csum_offset +
> +		csum_start = READ_ONCE(meta->request.csum_start);
> +		csum_offset = READ_ONCE(meta->request.csum_offset);
> +
> +		if (unlikely(csum_start + csum_offset +
>  			     sizeof(__sum16) > desc->len))
>  			return -EINVAL;
>  
> -		skb->csum_start = hr + meta->request.csum_start;
> -		skb->csum_offset = meta->request.csum_offset;
> +		skb->csum_start = hr + csum_start;

[Severity: High]
This is a pre-existing issue, but does this addition risk overflowing the
16-bit skb->csum_start on systems with large pages?

If the AF_XDP chunk size and desc->len are large (e.g. up to 65536 on systems
with 64KB pages), csum_start can be near 65535. When headroom (hr) is added,
the result could overflow the 16-bit skb->csum_start and be truncated.

If this truncates to a smaller value, skb_checksum_start_offset(skb) could
subtract the headroom and produce a negative offset. In skb_checksum_help(),
this negative offset would be implicitly cast to an unsigned integer,
evaluating to true in the offset >= skb_headlen(skb) check and triggering a
WARN_ONCE while dropping the packet.

Additionally, if hardware checksum offload is used, drivers might pass this
wrapped negative offset to the hardware, instructing it to perform DMA
accesses massively out-of-bounds from the packet buffer. Could this result
in IOMMU faults or memory corruption?

> +		skb->csum_offset = csum_offset;
>  		skb->ip_summed = CHECKSUM_PARTIAL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260520004244.55663-1-kerneljasonxing@gmail.com?part=1