All of lore.kernel.org
 help / color / mirror / Atom feed
From: sashiko-bot@kernel.org
To: "Nick Hudson" <nhudson@akamai.com>
Cc: bpf@vger.kernel.org
Subject: Re: [PATCH bpf-next v7 4/6] bpf: allow new DECAP flags and add guard rails
Date: Wed, 12 Aug 2026 10:22:59 +0000	[thread overview]
Message-ID: <20260812102300.0BF711F000E9@smtp.kernel.org> (raw)
In-Reply-To: <20260812083115.73100-5-nhudson@akamai.com>

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 <nhudson@akamai.com>

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/20260812083115.73100-1-nhudson@akamai.com?part=4

  reply	other threads:[~2026-08-12 10:23 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-12  8:31 [PATCH bpf-next v7 0/6] bpf: decap flags and GSO state updates Nick Hudson
2026-08-12  8:31 ` [PATCH bpf-next v7 1/6] bpf: name the enum for BPF_FUNC_skb_adjust_room flags Nick Hudson
2026-08-12  8:31 ` [PATCH bpf-next v7 2/6] bpf: refactor masks for ADJ_ROOM flags and encap validation Nick Hudson
2026-08-12 10:33   ` bot+bpf-ci
2026-08-12  8:31 ` [PATCH bpf-next v7 3/6] bpf: add BPF_F_ADJ_ROOM_DECAP_* flags for tunnel decapsulation Nick Hudson
2026-08-12  8:31 ` [PATCH bpf-next v7 4/6] bpf: allow new DECAP flags and add guard rails Nick Hudson
2026-08-12 10:22   ` sashiko-bot [this message]
2026-08-12 12:13     ` Hudson, Nick
2026-08-12 10:33   ` bot+bpf-ci
2026-08-12  8:31 ` [PATCH bpf-next v7 5/6] bpf: clear decap state on skb_adjust_room shrink path Nick Hudson
2026-08-12 10:26   ` sashiko-bot
2026-08-12 12:30     ` Hudson, Nick
2026-08-12 10:33   ` bot+bpf-ci
2026-08-12  8:31 ` [PATCH bpf-next v7 6/6] selftests/bpf: tc_tunnel - validate decap GSO and encapsulation state Nick Hudson
2026-08-12 10:20   ` sashiko-bot
2026-08-12 12:23     ` Hudson, Nick
2026-08-12 10:48   ` bot+bpf-ci

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20260812102300.0BF711F000E9@smtp.kernel.org \
    --to=sashiko-bot@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=nhudson@akamai.com \
    --cc=sashiko-reviews@lists.linux.dev \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.