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

[Daniel Borkmann <daniel@iogearbox.net>]

On 9/17/26 8:24 PM, Daniel Borkmann wrote:
> 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@vger.kernel.org
>> Fixes: 52f278774e79 ("bpf: implement BPF_LWT_ENCAP_IP mode in bpf_lwt_push_encap")
>> Reported-by: Xiang Mei <xmei5@asu.edu>
>> Assisted-by: LLM
>> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
>> ---
>> 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@vger.kernel.org.
>> - Correct the patch author and reporter attribution.
>> v1:
>> - https://lore.kernel.org/bpf/20260915170147.3943392-2-bestswngs@gmail.com/
> 
> 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));

... also needs BPF selftests obviously; dropping the memcpy and a closer look wrt
freplace, whether we should reuse & propagate ->cb_access=1 also from there.

> +    if (!encap && skb->encapsulation)
> +        bpf_lwt_reset_cb(skb);
> 
>       switch (ret) {
>       case BPF_OK: