From: sashiko-bot@kernel.org
To: "Alexei Starovoitov" <alexei.starovoitov@gmail.com>, bpf@vger.kernel.org
Subject: Re: [PATCH bpf 22/27] bpf: Reset skb->cb after an lwt prog pushes an IP header
Date: Sun, 01 Nov 2026 19:51:52 +0000 [thread overview]
Message-ID: <sashiko-outbox-192638@kernel.org> (raw)
In-Reply-To: <20261101190535.1454146-23-alexei.starovoitov@gmail.com>
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] bpf: incomplete skb->cb reset and VRF flag loss in bpf_lwt_reset_cb()
--
commit 40c1b82f0bd0c6a0826ad31c22402c9c073712bd
Author: Alexei Starovoitov <ast@kernel.org>
bpf: Reset skb->cb after an lwt prog pushes an IP header
This commit adds bpf_lwt_reset_cb() to reset skb->cb in bpf_input() after
an lwt program pushes an encapsulation header and in bpf_xmit() before
rerouting. This avoids interpreting stale inner IP option offsets or
cross-family control block fields against the outer header.
> diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
> index b9270a08cc6d1..3a946e8709b16 100644
> --- a/net/core/lwt_bpf.c
> +++ b/net/core/lwt_bpf.c
> @@ -85,6 +85,32 @@ static int run_lwt_bpf(struct sk_buff *skb, struct bpf_lwt_prog *lwt,
> return ret;
> }
>
> +/*
> + * skb->cb still describes the IP header that was the first one before the
> + * prog ran. The option offsets in IPCB(skb)->opt are wrong for a pushed
> + * header, and a header of the other family needs the other layout. iif
> + * stays: behind a VRF skb->skb_iif is the L3 master and not the port.
> + * bpf_lwt_push_encap() cannot do this, because bpf_prog_run_save_cb() can
> + * restore skb->cb after the prog.
> + */
> +static void bpf_lwt_reset_cb(struct sk_buff *skb, __be16 proto)
> +{
> + int iif;
> +
> + if (skb->protocol == proto) {
> + if (proto == htons(ETH_P_IP))
> + memset(&IPCB(skb)->opt, 0, sizeof(IPCB(skb)->opt));
> + } else if (skb->protocol == htons(ETH_P_IP)) {
> + iif = IP6CB(skb)->iif;
> + memset(IPCB(skb), 0, sizeof(*IPCB(skb)));
> + IPCB(skb)->iif = iif;
> + } else {
> + iif = IPCB(skb)->iif;
> + memset(IP6CB(skb), 0, sizeof(*IP6CB(skb)));
> + IP6CB(skb)->iif = iif;
> + }
> +}
[Severity: Critical]
Does bpf_lwt_reset_cb() leave several fields in IPCB(skb) and IP6CB(skb) in
an inconsistent state after encapsulation or rerouting?
1. When transitioning from IPv4 to IPv6, memset(IP6CB(skb), 0, ...) leaves
IP6CB(skb)->nhoff as 0 instead of offsetof(struct ipv6hdr, nexthdr). If
bpf_lwt_input_reroute() routes the encapsulated IPv6 packet to local
delivery, ip6_input() -> ip6_protocol_deliver_rcu() reads:
net/ipv6/ip6_input.c:ip6_protocol_deliver_rcu() {
...
nhoff = IP6CB(skb)->nhoff;
if (!have_final) {
if (!pskb_pull(skb, skb_transport_offset(skb)))
goto discard;
nexthdr = skb_network_header(skb)[nhoff];
}
}
With nhoff == 0, this reads byte 0 of struct ipv6hdr (version and traffic
class) as the next header protocol instead of ipv6_hdr(skb)->nexthdr.
2. For IPv6-in-IPv6 encapsulation or rerouting (skb->protocol == proto ==
htons(ETH_P_IPV6)), bpf_lwt_reset_cb() does nothing. However, IP6CB(skb)
holds extension header offsets relative to skb_network_header(skb) (ra,
dst0, srcrt, dst1, lastopt, nhoff, dsthao, srhoff) as well as flags
(IP6SKB_HOPBYHOP, IP6SKB_ROUTERALERT, IP6SKB_FRAGMENTED) and frag_max_size
parsed from the inner IPv6 packet before bpf_input() or bpf_xmit() ran.
After bpf_lwt_push_ip_encap() moves skb->network_header to the newly pushed
outer IPv6 header (or an lwt_xmit program modifies headers and returns
BPF_LWT_REROUTE):
- With CONFIG_IPV6_MIP6, if icmpv6_send() is triggered on the outer packet,
mip6_addr_swap() uses the stale IP6CB(skb)->dsthao offset relative to the
new outer network header:
net/ipv6/icmp.c:mip6_addr_swap() {
...
if (opt->dsthao) {
off = ipv6_find_tlv(skb, opt->dsthao, IPV6_TLV_HAO);
if (likely(off >= 0)) {
hao = (struct ipv6_destopt_hao *)
(skb_network_header(skb) + off);
swap(iph->saddr, hao->addr);
}
}
}
Can this corrupt the packet or swap 16 bytes out of bounds?
- On local delivery via ip6_protocol_deliver_rcu() or
ip6_datagram_recv_specific_ctl(), the stack will index from the new
network header using the inner packet's nhoff, lastopt, dst0, dst1, and
srcrt offsets.
- In ip6_forward() and __ip6_finish_output(), stale IP6SKB_ROUTERALERT or
frag_max_size from the inner packet will divert or fragment the outer
packet.
3. When a packet arrives on a port enslaved to a VRF device, vrf_ip_rcv()
and vrf_ip6_rcv() set IPSKB_L3SLAVE or IP6SKB_L3SLAVE in cb->flags while
keeping the slave port ifindex in cb->iif and setting skb->skb_iif to the
VRF master ifindex. Because bpf_lwt_reset_cb() clears flags without
translating IPSKB_L3SLAVE and IP6SKB_L3SLAVE across family transitions,
inet6_iif() and inet_sdif() lose the VRF association:
include/linux/ipv6.h:inet6_iif() {
bool l3_slave = ipv6_l3mdev_skb(IP6CB(skb)->flags);
return l3_slave ? skb->skb_iif : IP6CB(skb)->iif;
}
4. For IPv4-in-IPv4, only IPCB(skb)->opt is zeroed, leaving
IPCB(skb)->frag_max_size and non-VRF flags from the inner packet set, which
causes __ip_finish_output() to fragment the outer packet to the inner
packet's reassembled fragment size.
Should bpf_lwt_reset_cb() reset the full control block for both families
while preserving iif, translating IPSKB_L3SLAVE / IP6SKB_L3SLAVE, and
initializing IP6CB(skb)->nhoff = offsetof(struct ipv6hdr, nexthdr), similar
to decap_and_validate() in net/ipv6/seg6_local.c?
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261101190535.1454146-1-alexei.starovoitov@gmail.com?part=22
next prev parent reply other threads:[~2026-11-01 19:51 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 [this message]
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
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-192638@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