Re: [PATCH net] calipso: update payload_len when removing the CALIPSO option

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] ipv6: calipso_skbuff_delattr corrupts IPv6 Jumbograms via payload_len underflow
- [Medium] ipv6: calipso: calipso_skbuff_delattr() fails to invalidate CHECKSUM_COMPLETE
--

commit 8fb4651bac6a1dd9a8a2f7c1f3e21afbeec3108b
Author: Joas Antonio dos Santos <joasantonio108@gmail.com>

calipso: update payload_len when removing the CALIPSO option

calipso_skbuff_delattr() removes the CALIPSO option from the hop-by-hop
extension header, or the whole extension header if CALIPSO is its only
option, and pulls the skb by the removed length. It never updates
ipv6hdr->payload_len, so after a successful removal the header still
claims the original length.

> diff --git a/net/ipv6/calipso.c b/net/ipv6/calipso.c
> index c6a34334e657e..c072eca501070 100644
> --- a/net/ipv6/calipso.c
> +++ b/net/ipv6/calipso.c
> @@ -1428,6 +1428,9 @@ static int calipso_skbuff_delattr(struct sk_buff *skb)
>  		skb_pull(skb, delta);
>  		memmove((char *)ip6_hdr + delta, ip6_hdr, size);
>  		skb_reset_network_header(skb);

[Severity: Medium]
When delta bytes are removed from the packet, should skb->csum also be
updated (e.g., via skb_postpull_rcsum()), or should skb->ip_summed be
cleared to CHECKSUM_NONE?

An IPv6 multicast packet arriving with CHECKSUM_COMPLETE can reach the
NF_INET_FORWARD hook with its checksum state intact, since ip6mr_forward2()
does not invoke skb_forward_csum().

If SELinux intercepts this via selinux_ip_forward() and triggers
calipso_skbuff_delattr(), the packet is left with an invalid
CHECKSUM_COMPLETE state. If the packet is subsequently forwarded into a
virtual interface like a veth pair, the receiver will drop it due to a
spurious checksum mismatch.

> +		ip6_hdr = ipv6_hdr(skb);
> +		ip6_hdr->payload_len =
> +			htons(ntohs(ip6_hdr->payload_len) - delta);

[Severity: Medium]
Can this underflow when ip6_hdr->payload_len is 0, corrupting the IPv6
Jumbogram length calculation?

For Jumbograms, payload_len is explicitly 0 and the actual length is stored
in a Hop-by-Hop option. Subtracting delta from 0 underflows the 16-bit
integer, resulting in a large positive value.

Subsequent IPv6 stack processing will misinterpret this packet as a standard
IPv6 packet with a large payload rather than reading the Jumbogram option,
completely corrupting the packet.

While calipso_skbuff_setattr() also incorrectly modifies Jumbogram lengths,
this underflow logic on the delete path is newly introduced here.

>  	}
>  
>  	return 0;

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