Re: [PATCH bpf-next v4 5/6] bpf: clear decap tunnel GSO state in skb_adjust_room

[sashiko-bot@kernel.org]

Sashiko AI review found 3 potential issue(s):
- [High] Non-GSO packets bypass decapsulation state clearing logic, leading to stale encapsulation state.
- [Medium] GRE decapsulation fails to clear `skb->encap_hdr_csum`.
- [Medium] `SKB_GSO_TUNNEL_REMCSUM` is not cleared from `gso_type` during UDP decapsulation.
--

commit 11e8db6b3cc9e077f7973f47c48689c42f700960
Author: Nick Hudson <nhudson@akamai.com>

bpf: clear decap tunnel GSO state in skb_adjust_room

On shrink in bpf_skb_adjust_room(), clear tunnel-specific GSO flags
according to the decapsulation flags. Handle ESP inside UDP, and clear
encap_hdr_csum and remcsum_offload if UDP decap is performed.

> diff --git a/net/core/filter.c b/net/core/filter.c
> index b87356ac05f75..54a478be04bb3 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c

[ ... ]

> @@ -3660,6 +3660,44 @@ static int bpf_skb_net_shrink(struct sk_buff *skb, u32 off, u32 len_diff,
>  		if (!(flags & BPF_F_ADJ_ROOM_FIXED_GSO))
>  			skb_increase_gso_size(shinfo, len_diff);
>  
> +		/* Selective GSO flag clearing based on decap type.
> +		 * Only clear the flags for the tunnel layer being removed.
> +		 */
> +		if ((flags & BPF_F_ADJ_ROOM_DECAP_L4_UDP) &&
> +		    (shinfo->gso_type & (SKB_GSO_UDP_TUNNEL |
> +					 SKB_GSO_UDP_TUNNEL_CSUM)))
> +			shinfo->gso_type &= ~(SKB_GSO_UDP_TUNNEL |
> +					      SKB_GSO_UDP_TUNNEL_CSUM);

Should SKB_GSO_TUNNEL_REMCSUM also be cleared here during UDP decapsulation?

Remote checksum offload properties are tied to the outer tunnel header, and 
stripping it without removing the associated REMCSUM GSO flag might leave
shinfo->gso_type in an invalid state.

> +		if ((flags & BPF_F_ADJ_ROOM_DECAP_L4_GRE) &&
> +		    (shinfo->gso_type & (SKB_GSO_GRE | SKB_GSO_GRE_CSUM)))
> +			shinfo->gso_type &= ~(SKB_GSO_GRE |
> +					      SKB_GSO_GRE_CSUM);
> +		if ((flags & BPF_F_ADJ_ROOM_DECAP_IPXIP4) &&
> +		    (shinfo->gso_type & SKB_GSO_IPXIP4))
> +			shinfo->gso_type &= ~SKB_GSO_IPXIP4;
> +		if ((flags & BPF_F_ADJ_ROOM_DECAP_IPXIP6) &&
> +		    (shinfo->gso_type & SKB_GSO_IPXIP6))
> +			shinfo->gso_type &= ~SKB_GSO_IPXIP6;
> +
> +		/* Clear encapsulation flag only when no tunnel GSO flags remain */
> +		if (flags & (BPF_F_ADJ_ROOM_DECAP_L4_MASK |
> +			     BPF_F_ADJ_ROOM_DECAP_IPXIP_MASK)) {
> +			if (!(shinfo->gso_type & (SKB_GSO_UDP_TUNNEL |
> +						  SKB_GSO_UDP_TUNNEL_CSUM |
> +						  SKB_GSO_GRE |
> +						  SKB_GSO_GRE_CSUM |
> +						  SKB_GSO_IPXIP4 |
> +						  SKB_GSO_IPXIP6 |
> +						  SKB_GSO_ESP)))
> +				if (skb->encapsulation)
> +					skb->encapsulation = 0;

Because all of this new clearing logic is added inside the if (skb_is_gso(skb))
block, do non-GSO packets bypass the state clearing? BPF programs can call
bpf_skb_adjust_room() to decapsulate non-GSO packets as well.

Could leaving skb->encapsulation set after stripping tunnel headers on non-GSO
packets cause downstream network stack components to misinterpret the packet
as still being encapsulated?

> +
> +			if (flags & BPF_F_ADJ_ROOM_DECAP_L4_UDP) {
> +				skb->encap_hdr_csum = 0;
> +				skb->remcsum_offload = 0;
> +			}

Does GRE decapsulation (BPF_F_ADJ_ROOM_DECAP_L4_GRE) also need to clear
skb->encap_hdr_csum? GRE tunnels utilize the encap_hdr_csum flag when
SKB_GSO_GRE_CSUM is active.

> +		}
> +
>  		/* Header must be checked, and gso_segs recomputed. */
>  		shinfo->gso_type |= SKB_GSO_DODGY;
>  		shinfo->gso_segs = 0;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260416075514.927101-1-nhudson@akamai.com?part=5