Re: [PATCH bpf 1/2] bpf: Fix bpf_skb_change_tail wrt csum partial skbs

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The checksum field space check in `bpf_skb_trim_rcsum()` assumes a 2-byte checksum (`sizeof(__sum16)`), but SCTP checksum offloads (`skb->csum_not_inet == 1`) require a 4-byte CRC32c. This allows BPF programs to trim SCTP packets such that only the first 2 bytes of the checksum field remain. The hardware will then attempt to process a 4-byte checksum, causing an out-of-bounds access.
--

commit 4cea525b891060e3211b5353724699e42d836d25
Author: Daniel Borkmann <daniel@iogearbox.net>
bpf: Fix bpf_skb_change_tail wrt csum partial skbs

This commit fixes an issue where bpf_skb_change_tail() rejects trims on skbs
carrying a checksum offload, such as those from GRO. It removes the minimum
length restriction from __bpf_skb_min_len() and modifies bpf_skb_trim_rcsum()
to downgrade the checksum to CHECKSUM_NONE if the new length no longer covers
the checksum field.

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 61940e7535523..8513167a858a8 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
[ ... ]
> @@ -3983,6 +3977,11 @@ static int bpf_skb_grow_rcsum(struct sk_buff *skb, unsigned int new_len)
>  
>  static int bpf_skb_trim_rcsum(struct sk_buff *skb, unsigned int new_len)
>  {
> +	if (skb->ip_summed == CHECKSUM_PARTIAL &&
> +	    new_len < skb_checksum_start_offset(skb) + skb->csum_offset +
> +		      sizeof(__sum16))
> +		skb->ip_summed = CHECKSUM_NONE;
> +

[Severity: High]
This is a pre-existing issue, but this patch replicates the flawed 2-byte
assumption from __bpf_skb_min_len(). The checksum field space check in
bpf_skb_trim_rcsum() assumes a 2-byte checksum (sizeof(__sum16)), but SCTP
checksum offloads (skb->csum_not_inet == 1) require a 4-byte CRC32c.

This allows BPF programs to trim SCTP packets such that only the first 2 bytes
of the checksum field remain. If trimmed so that exactly 2 or 3 bytes of the
4-byte CRC32c remain, the length check evaluates to false, and CHECKSUM_PARTIAL
is not cleared. The packet is then handed to the NIC driver.

Passing a truncated CHECKSUM_PARTIAL packet to the NIC hardware causes the
offload engine to access beyond the packet boundary, causing a hardware stall
or OOB write on the wire.

Could we check for skb->csum_not_inet and use a 4-byte length for the checksum
when processing SCTP packets here?

>  	return __skb_trim_rcsum(skb, new_len);
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260907121025.1923656-1-daniel@iogearbox.net?part=1