From: sashiko-bot@kernel.org
To: "Alexei Starovoitov" <alexei.starovoitov@gmail.com>, bpf@vger.kernel.org
Subject: Re: [PATCH bpf 26/27] bpf: Use the inner transport header after a decap
Date: Sun, 01 Nov 2026 19:57:29 +0000 [thread overview]
Message-ID: <sashiko-outbox-192641@kernel.org> (raw)
In-Reply-To: <20261101190535.1454146-27-alexei.starovoitov@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: mishandled unset ~0U transport header in bpf_skb_clear_encap()
--
commit 907dfd6423a9766152f03a831bcc864117a68b99
Author: Chris Mason <mason@kernel.org>
bpf: Use the inner transport header after a decap
This commit adds bpf_skb_clear_encap() to update skb->transport_header from
skb->inner_transport_header when decapsulation in bpf_skb_net_shrink() clears
skb->encapsulation.
> diff --git a/net/core/filter.c b/net/core/filter.c
> index b5534dac9e6dc..21fcdc7cd950d 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -3769,6 +3769,25 @@ static int bpf_skb_net_grow(struct sk_buff *skb, u32 off, u32 len_diff,
> return 0;
> }
>
> +/*
> + * Code that went by the inner transport header while skb->encapsulation was set
> + * goes by the transport header from now on. So take the inner one over, unless
> + * it is the smaller one, as it is if nobody has set it. Do not leave the result
> + * in front of @off, where the bytes were removed.
> + */
> +static void bpf_skb_clear_encap(struct sk_buff *skb, u32 off)
> +{
> + int thoff;
> +
> + if (!skb->encapsulation)
> + return;
> +
> + thoff = max(skb->inner_transport_header, skb->transport_header) -
> + skb_headroom(skb);
> + skb_set_transport_header(skb, max_t(int, thoff, off));
> + skb->encapsulation = 0;
> +}
[Severity: High]
Can max(skb->inner_transport_header, skb->transport_header) in
bpf_skb_clear_encap() in net/core/filter.c select the unset sentinel (~0U)
instead of a valid transport header offset?
In include/linux/skbuff.h, an unset transport header is represented by
(__u16)~0U (0xffff):
include/linux/skbuff.h:skb_transport_header_was_set() {
return skb->transport_header != (typeof(skb->transport_header))~0U;
}
Because 0xffff is the largest possible __u16 value rather than the smallest,
taking max(skb->inner_transport_header, skb->transport_header) will pick
0xffff whenever either field holds the unset sentinel:
1. If skb->transport_header is unset (0xffff) while
skb->inner_transport_header holds a valid offset (for example, after
gro_cells_receive() calls skb_unset_transport_header(), or after
encapsulating via bpf_skb_net_grow() with BPF_F_ADJ_ROOM_ENCAP_L3_IPV4/IPV6
without L4 flags), max() chooses 0xffff and overwrites
skb->transport_header with 0xffff instead of copying
skb->inner_transport_header.
2. If an skb starts with skb->transport_header unset (0xffff) and is
encapsulated with BPF_F_ADJ_ROOM_ENCAP_L4_GRE or
BPF_F_ADJ_ROOM_ENCAP_L4_UDP, bpf_skb_net_grow() copies 0xffff into
skb->inner_transport_header while setting skb->transport_header to the
outer L4 header:
net/core/filter.c:bpf_skb_net_grow() {
if (encap) {
...
inner_trans = skb->transport_header;
}
...
if (encap) {
...
skb->inner_transport_header = inner_trans;
...
if (flags & BPF_F_ADJ_ROOM_ENCAP_L4_GRE ||
flags & BPF_F_ADJ_ROOM_ENCAP_L4_UDP) {
...
skb_set_transport_header(skb, mac_len + nh_len);
}
}
When that packet is later decapsulated in bpf_skb_net_shrink(),
bpf_skb_net_hdr_pop() clamps the outer skb->transport_header to off, while
skb_inner_transport_offset(skb) is 0xffff - skb_headroom(skb) >= (int)off
so skb->inner_transport_header stays 0xffff:
net/core/filter.c:bpf_skb_net_hdr_pop() {
} else if (skb_transport_header_was_set(skb) &&
skb_transport_offset(skb) < (int)off) {
/* It was in the removed bytes or before skb->data. */
skb_set_transport_header(skb, off);
}
/* Do not leave the inner transport header in front of @off. */
if (skb->encapsulation &&
skb_inner_transport_offset(skb) < (int)off)
skb_set_inner_transport_header(skb, off);
}
Then bpf_skb_clear_encap() evaluates max(0xffff, skb->transport_header) and
sets skb->transport_header back to 0xffff.
For a GSO packet where bpf_skb_net_shrink() sets SKB_GSO_DODGY, resets
gso_segs = 0, and clears skb->encapsulation, leaving skb->transport_header at
0xffff causes qdisc_pkt_len_segs_init() to skip GSO segment calculation and
causes skb_gso_validate_mac_len() -> skb_gso_transport_seglen() ->
tcp_hdrlen() -> skb_transport_header() to trigger
DEBUG_NET_WARN_ON_ONCE(!skb_transport_header_was_set(skb)) and read from
skb->head + 0xffff.
Should bpf_skb_clear_encap() check whether the transport headers are set
(or ignore values equal to (__u16)~0U) before comparing them?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261101190535.1454146-1-alexei.starovoitov@gmail.com?part=26
next prev parent reply other threads:[~2026-11-01 19:57 UTC|newest]
Thread overview: 38+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-01 19:05 [PATCH bpf 00/27] bpf: Fixes for the verifier, arena, skb and lwt helpers Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 01/27] bpf: Clamp the trace length in __bpf_get_stack() again Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 02/27] bpf: Clamp the trace length before the skip test in stackid_init() Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 03/27] bpf: Refuse to map an arena at address 0 Alexei Starovoitov
2026-11-01 19:19 ` sashiko-bot
2026-11-01 19:05 ` [PATCH bpf 04/27] bpf: Don't let bpf_arena_free_pages() see a half-set user range Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 05/27] bpf: Refuse an arena mmap() longer than max_entries pages Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 06/27] bpf: Remove the WARN from arena_get_unmapped_area() Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 07/27] selftests/bpf: Test the length and the address of an arena mmap() Alexei Starovoitov
2026-11-01 19:38 ` bot+bpf-ci
2026-11-01 19:05 ` [PATCH bpf 08/27] bpf: Reject memory for kfunc projection type args Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 09/27] bpf: Don't turn struct arguments of KF_ACQUIRE kfuncs into memory Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 10/27] bpf: Don't turn kfunc arguments of an empty struct type " Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 11/27] bpf: Compare the arguments of an extension with those of its target Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 12/27] selftests/bpf: Test the arguments of an extension prog Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 13/27] bpf: Require prog fd or capability for cgroup detach Alexei Starovoitov
2026-11-01 19:38 ` bot+bpf-ci
2026-11-02 0:12 ` Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 14/27] bpf, cgroup: Keep the skb network offset signed Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 15/27] bpf: Don't let bpf_skb_adjust_room() raise gso_size to GSO_BY_FRAGS Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 16/27] bpf: Reject non-IPv6 packets in BPF_LWT_ENCAP_SEG6 Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 17/27] bpf: Set a dst for test runs of lwt_in progs Alexei Starovoitov
2026-11-01 19:38 ` bot+bpf-ci
2026-11-02 0:12 ` Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 18/27] bpf: Limit the SRH length in bpf_lwt_seg6_adjust_srh() Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 19/27] bpf: Don't look for the SRH again in the End.BPF helpers Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 20/27] bpf: Move the MAC header in bpf_lwt_seg6_adjust_srh() Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 21/27] bpf: Don't leave a stale MAC header in bpf_lwt_push_ip_encap() Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 22/27] bpf: Reset skb->cb after an lwt prog pushes an IP header Alexei Starovoitov
2026-11-01 19:51 ` sashiko-bot
2026-11-01 19:05 ` [PATCH bpf 23/27] bpf: Require BPF_LWT_REROUTE when an lwt_in prog changes the protocol Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 24/27] bpf: Don't let lwt_in progs call into IPv6 when it is disabled Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 25/27] bpf: Move the transport header with the moved headers Alexei Starovoitov
2026-11-01 19:05 ` [PATCH bpf 26/27] bpf: Use the inner transport header after a decap Alexei Starovoitov
2026-11-01 19:57 ` sashiko-bot [this message]
2026-11-01 19:05 ` [PATCH bpf 27/27] bpf: Check the offset in bpf_skb_generic_{push,pop}() Alexei Starovoitov
2026-11-01 20:00 ` sashiko-bot
2026-11-02 0:20 ` [PATCH bpf 00/27] bpf: Fixes for the verifier, arena, skb and lwt helpers patchwork-bot+netdevbpf
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=sashiko-outbox-192641@kernel.org \
--to=sashiko-bot@kernel.org \
--cc=alexei.starovoitov@gmail.com \
--cc=bpf@vger.kernel.org \
--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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox