Re: [PATCH v2] bpf: clear stale IPv4 options after LWT encapsulation

[Daniel Borkmann]

On 9/16/26 7:04 PM, Weiming Shi wrote:
> bpf_lwt_push_ip_encap() rebases the network header after prepending an IP
> header, but leaves IPCB(skb)->opt describing the inner IPv4 header.  An
> ingress LWT route can consequently make an ICMP error interpret an
> inner-header byte as an option length and copy 255 bytes into 40 bytes of
> stack storage.  The trace decoded with scripts/decode_stacktrace.sh is:
>
>    BUG: KASAN: stack-out-of-bounds in __ip_options_echo
>    Write of size 255
>    Call Trace:
>     <IRQ>
>     __asan_memcpy (mm/kasan/shadow.c:106)
>     __ip_options_echo (net/ipv4/ip_options.c:96)
>     __icmp_send (net/ipv4/icmp.c:949)
>     ip_forward (net/ipv4/ip_forward.c:176)
>     lwtunnel_input (net/core/lwtunnel.c:465)
>     ip_rcv (net/ipv4/ip_input.c:612)
>     __netif_receive_skb_one_core (net/core/dev.c:6264)
>     process_backlog (net/core/dev.c:6728)
>     __napi_poll (net/core/dev.c:7787)
>     net_rx_action (net/core/dev.c:8007)
>     handle_softirqs (kernel/softirq.c:645)
>     do_softirq.part.0 (kernel/softirq.c:546)
>     </IRQ>
>     <TASK>
>     __local_bh_enable_ip (kernel/softirq.c:473)
>     __dev_queue_xmit (net/core/dev.c:4961)
>     packet_sendmsg (net/packet/af_packet.c:3143)
>     __sys_sendto (net/socket.c:2281)
>     __x64_sys_sendto (net/socket.c:2288)
>     do_syscall_64 (arch/x86/entry/syscall_64.c:84)
>     entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
>     </TASK>
>
> A helper-only reset can be restored by bpf_prog_run_save_cb(), while a
> program without ctx->cb[] access can clone-redirect the skb before a
> return-only reset.  Track active LWT runs and their control-block family
> in the BPF network context.  When the control block is not BPF scratch
> space, save its original contents and reset it immediately so clones see
> the new-family layout.  After the program returns, restore that snapshot
> when the verdict continues through the original protocol callback, then
> invalidate the stale header metadata.  Verdicts that reroute or redirect
> keep the new-family layout.
>
> Preserve the state across nested runs and retain the ingress interface and
> L3-slave state when the protocol family changes.
>
> Cc: stable@xxxxxxxxxxxxxxx
> Fixes: 52f278774e79 ("bpf: implement BPF_LWT_ENCAP_IP mode in bpf_lwt_push_encap")
> Reported-by: Xiang Mei <xmei5@xxxxxxx>
> Assisted-by: LLM
> Signed-off-by: Weiming Shi <bestswngs@xxxxxxxxx>
> ---
> v2:
> - Save the incoming protocol control block for programs without ctx->cb[]
>    access.
> - Restore that snapshot before a verdict continues through the original
>    protocol callback, then invalidate the rebased header metadata.
> - Keep the eager new-family reset for clones and final reroute/redirect
>    consumers.
> - Add Cc: stable@xxxxxxxxxxxxxxx.
> - Correct the patch author and reporter attribution.
> v1:
> - https://lore.kernel.org/bpf/20260915170147.3943392-2-bestswngs@xxxxxxxxx/

Hm, this is way too much fragile churn for a feature which I'm not sure is much
used (?). Can't we just save/restore the skb->cb when the BPF prog runs? Roughly
sth along these lines (untested) :

diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
index da49364ec63d..9cf04b44ecc1 100644
--- a/net/core/lwt_bpf.c
+++ b/net/core/lwt_bpf.c
@@ -36,10 +36,24 @@ static inline struct bpf_lwt *bpf_lwt_lwtunnel(struct lwtunnel_state *lwt)
 #define NO_REDIRECT false
 #define CAN_REDIRECT true
 
+static void bpf_lwt_reset_cb(struct sk_buff *skb)
+{
+	if (skb->protocol == htons(ETH_P_IP)) {
+		memset(IPCB(skb), 0, sizeof(*IPCB(skb)));
+		IPCB(skb)->iif = skb->skb_iif;
+	} else if (skb->protocol == htons(ETH_P_IPV6)) {
+		memset(IP6CB(skb), 0, sizeof(*IP6CB(skb)));
+		IP6CB(skb)->iif = skb->skb_iif;
+		IP6CB(skb)->nhoff = offsetof(struct ipv6hdr, nexthdr);
+	}
+}
+
 static int run_lwt_bpf(struct sk_buff *skb, struct bpf_lwt_prog *lwt,
 		       struct dst_entry *dst, bool can_redirect)
 {
 	struct bpf_net_context __bpf_net_ctx, *bpf_net_ctx;
+	bool encap = skb->encapsulation;
+	u8 cb_saved[BPF_SKB_CB_LEN];
 	int ret;
 
 	/* Disabling BH is needed to protect per-CPU bpf_redirect_info between
@@ -48,7 +62,12 @@ static int run_lwt_bpf(struct sk_buff *skb, struct bpf_lwt_prog *lwt,
 	local_bh_disable();
 	bpf_net_ctx = bpf_net_ctx_set(&__bpf_net_ctx);
 	bpf_compute_data_pointers(skb);
+
+	memcpy(cb_saved, bpf_skb_cb(skb), sizeof(cb_saved));
 	ret = bpf_prog_run_save_cb(lwt->prog, skb);
+	memcpy(bpf_skb_cb(skb), cb_saved, sizeof(cb_saved));
+	if (!encap && skb->encapsulation)
+		bpf_lwt_reset_cb(skb);
 
 	switch (ret) {
 	case BPF_OK: