From: sashiko-bot@kernel.org
To: "Alexei Starovoitov" <alexei.starovoitov@gmail.com>, bpf@vger.kernel.org
Subject: Re: [PATCH bpf 27/27] bpf: Check the offset in bpf_skb_generic_{push,pop}()
Date: Sun, 01 Nov 2026 20:00:05 +0000 [thread overview]
Message-ID: <sashiko-outbox-192643@kernel.org> (raw)
In-Reply-To: <20261101190535.1454146-28-alexei.starovoitov@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: u32 wraparound of off in bpf_skb_adjust_room() bypasses bounds check
--
commit 294bb6515ed77f7d0955cf5e521eacaeed3e3e77
Author: Chris Mason <mason@kernel.org>
bpf: Check the offset in bpf_skb_generic_{push,pop}()
This commit adds bounds checks in bpf_skb_generic_push() and
bpf_skb_generic_pop() to ensure off does not exceed skb->len and pulls the
first off bytes into the linear part of the skb before moving data.
> diff --git a/net/core/filter.c b/net/core/filter.c
> index 21fcdc7cd950d..9245009d9f1eb 100644
> --- a/net/core/filter.c
> +++ b/net/core/filter.c
> @@ -3369,6 +3369,10 @@ static const struct bpf_func_proto bpf_skb_vlan_pop_proto = {
>
> static int bpf_skb_generic_push(struct sk_buff *skb, u32 off, u32 len)
> {
> + /* The bytes in front of @off are moved with skb->data. */
> + if (unlikely(off > skb->len || !pskb_may_pull(skb, off)))
> + return -ENOMEM;
> +
> /* Caller already did skb_cow() with meta_len+len as headroom,
> * so no need to do it here.
> */
> @@ -3389,9 +3393,10 @@ static int bpf_skb_generic_pop(struct sk_buff *skb, u32 off, u32 len)
> void *old_data;
>
> /* skb_ensure_writable() is not needed here, as we're
> - * already working on an uncloned skb.
> + * already working on an uncloned skb. The first test
> + * keeps off + len from wrapping.
> */
> - if (unlikely(!pskb_may_pull(skb, off + len)))
> + if (unlikely(off > skb->len || !pskb_may_pull(skb, off + len)))
> return -ENOMEM;
[Severity: High]
Can off wrap in bpf_skb_adjust_room() before reaching bpf_skb_generic_push()
or bpf_skb_generic_pop()?
In net/core/filter.c, bpf_skb_adjust_room() computes off as:
off = skb_mac_header_len(skb);
switch (mode) {
case BPF_ADJ_ROOM_NET:
off += bpf_skb_net_base_len(skb);
break;
When skb->network_header < skb->mac_header by 1 to 20 bytes (for ETH_P_IP) or
1 to 40 bytes (for ETH_P_IPV6), skb_mac_header_len(skb) returns a u32 near
UINT_MAX (0xffffffd8..0xffffffff).
When mode is BPF_ADJ_ROOM_NET, adding bpf_skb_net_base_len(skb) (20 or 40)
wraps the 32-bit unsigned off to a small value in 0..39 before
bpf_skb_net_grow() or bpf_skb_net_shrink() calls bpf_skb_generic_push() or
bpf_skb_generic_pop().
Because off has already wrapped to 0..39, the off > skb->len check in
bpf_skb_generic_push() and bpf_skb_generic_pop() passes whenever
skb->len >= off. When growing with encapsulation flags in bpf_skb_net_grow(),
u16 mac_len = skb->network_header - skb->mac_header also wraps to
65496..65535, and skb_set_network_header(skb, mac_len) sets
skb->network_header far past the end of the packet.
Would it make sense to validate skb->network_header >= skb->mac_header (or
check skb_mac_header_len(skb) before adding bpf_skb_net_base_len(skb)) in
bpf_skb_adjust_room()?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261101190535.1454146-1-alexei.starovoitov@gmail.com?part=27
next prev parent reply other threads:[~2026-11-01 20:00 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
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 [this message]
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-192643@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