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 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.