Re: [PATCH bpf-next v7 4/6] bpf: allow new DECAP flags and add guard rails

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] The patch exposes new decapsulation flags without implementing the required kernel side effects to clear tunnel-specific GSO flags and `skb->encapsulation` status, leading to stale tunnel metadata on decapsulated packets.
- [High] The `decap` flag check in `bpf_skb_net_shrink()` was not updated to include the new L4 and IPXIP decapsulation masks, causing critical packet state updates (like dropping the outer `dst_entry`) to be skipped.
--

--- Patch [4]: [PATCH bpf-next v7 4/6] bpf: allow new DECAP flags and add guard rails ---
commit cdd433d824d20f40752fca9cc2db82451d364a2c
Author: Nick Hudson <[email protected]>

bpf: allow new DECAP flags and add guard rails

Add checks to require shrink-only decap, reject conflicting decap flag
combinations, and verify removed length is sufficient for claimed header
decapsulation.

> diff --git a/net/core/filter.c b/net/core/filter.c
> index 89e516049c852..981b75941e7f7 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
[ ... ]
> @@ -3579,7 +3586,9 @@ static u32 bpf_skb_net_base_len(const struct sk_buff *skb)
>  					 BPF_F_ADJ_ROOM_ENCAP_L2( \
>  					  BPF_ADJ_ROOM_ENCAP_L2_MASK))
>  
> -#define BPF_F_ADJ_ROOM_DECAP_MASK	(BPF_F_ADJ_ROOM_DECAP_L3_MASK)
> +#define BPF_F_ADJ_ROOM_DECAP_MASK	(BPF_F_ADJ_ROOM_DECAP_L3_MASK | \
> +					 BPF_F_ADJ_ROOM_DECAP_L4_MASK | \
> +					 BPF_F_ADJ_ROOM_DECAP_IPXIP_MASK)

[Severity: High]
Does this mask update also need to be reflected in the decap check in
bpf_skb_net_shrink()?

Currently, bpf_skb_net_shrink() evaluates decap based only on the L3 mask:

net/core/filter.c:bpf_skb_net_shrink() {
	bool decap = flags & BPF_F_ADJ_ROOM_DECAP_L3_MASK;
	...
	if (decap) {
		...
		if (skb_valid_dst(skb))
			skb_dst_drop(skb);
	}
	...
}

If a BPF program uses BPF_F_ADJ_ROOM_DECAP_L4_UDP to decapsulate an L4 or
IPXIP tunnel but does not provide BPF_F_ADJ_ROOM_DECAP_L3_MASK, decap
evaluates to false.

As a result, skb_dst_drop(skb) is skipped, causing the inner packet to
incorrectly inherit the outer tunnel's routing cache. This can route the
inner packet to the tunnel endpoint instead of its true destination.

[Severity: High]
Does this code miss clearing tunnel-specific GSO flags and skb->encapsulation
status?

The UAPI documentation promises that using BPF_F_ADJ_ROOM_DECAP_L4_UDP will
clear UDP tunnel GSO flags (SKB_GSO_UDP_TUNNEL and SKB_GSO_UDP_TUNNEL_CSUM)
when decapsulating a UDP tunnel.

However, bpf_skb_net_shrink() omits the logic to do this:

net/core/filter.c:bpf_skb_net_shrink() {
	...
	ret = bpf_skb_net_hdr_pop(skb, off, len_diff);
	if (unlikely(ret < 0))
		return ret;

	if (decap) {
		/* Match skb->protocol to new outer l3 protocol */
	...
}

Leaving stale tunnel metadata on the packet can cause downstream drivers to
parse incorrect header offsets, resulting in skb_over_panic or
skb_under_panic.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=4