mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] bpf: clear stale IPv4 options after LWT encapsulation
@ 2026-09-16 17:04 Weiming Shi
  2026-09-17 18:24 ` Daniel Borkmann
  0 siblings, 1 reply; 4+ messages in thread
From: Weiming Shi @ 2026-09-16 17:04 UTC (permalink / raw)
  To: Alexei Starovoitov, Daniel Borkmann, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
	Ihor Solodrai, John Fastabend, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman
  Cc: bpf, linux-kernel, netdev, Peter Oskolkov, Xiang Mei,
	Weiming Shi, stable

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/

 include/linux/filter.h | 10 +++++
 net/core/lwt_bpf.c     | 96 ++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 106 insertions(+)

diff --git a/include/linux/filter.h b/include/linux/filter.h
index 39decde7fc730..0edd3e6ce563f 100644
--- a/include/linux/filter.h
+++ b/include/linux/filter.h
@@ -848,6 +848,15 @@ struct bpf_nh_params {
 #define BPF_RI_F_CPU_MAP_INIT	BIT(2)
 #define BPF_RI_F_DEV_MAP_INIT	BIT(3)
 #define BPF_RI_F_XSK_MAP_INIT	BIT(4)
+#define BPF_RI_F_LWT_IP_ENCAP	BIT(5)
+#define BPF_RI_F_LWT_RUN	BIT(6)
+
+struct bpf_lwt_ip_encap_state {
+	int iif;
+	__be16 cb_proto;
+	bool l3slave;
+	bool cb_access;
+};
 
 struct bpf_redirect_info {
 	u64 tgt_index;
@@ -858,6 +867,7 @@ struct bpf_redirect_info {
 	enum bpf_map_type map_type;
 	struct bpf_nh_params nh;
 	u32 kern_flags;
+	struct bpf_lwt_ip_encap_state lwt_ip_encap;
 };
 
 struct bpf_net_context {
diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
index da49364ec63de..4c0461b4dccf7 100644
--- a/net/core/lwt_bpf.c
+++ b/net/core/lwt_bpf.c
@@ -36,19 +36,103 @@ 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_ip_cb(struct sk_buff *skb, __be16 orig_proto,
+				int iif, bool l3slave, bool use_new_proto)
+{
+	__be16 cb_proto = orig_proto;
+
+	if (use_new_proto)
+		cb_proto = skb->protocol;
+
+	if (cb_proto == htons(ETH_P_IP)) {
+		if (orig_proto == htons(ETH_P_IP)) {
+			memset(&IPCB(skb)->opt, 0, sizeof(IPCB(skb)->opt));
+		} else {
+			memset(IPCB(skb), 0, sizeof(*IPCB(skb)));
+			IPCB(skb)->iif = iif;
+			if (l3slave)
+				IPCB(skb)->flags |= IPSKB_L3SLAVE;
+		}
+	} else if (cb_proto == htons(ETH_P_IPV6)) {
+		memset(IP6CB(skb), 0, sizeof(*IP6CB(skb)));
+		IP6CB(skb)->iif = iif;
+		IP6CB(skb)->nhoff = offsetof(struct ipv6hdr, nexthdr);
+		if (l3slave)
+			IP6CB(skb)->flags |= IP6SKB_L3SLAVE;
+	} else if (orig_proto == htons(ETH_P_IP)) {
+		memset(&IPCB(skb)->opt, 0, sizeof(IPCB(skb)->opt));
+	}
+}
+
 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;
+	struct bpf_lwt_ip_encap_state nested_lwt_ip_encap_state;
+	struct bpf_redirect_info *ri;
+	union {
+		struct inet_skb_parm ip4;
+		struct inet6_skb_parm ip6;
+	} saved_cb;
+	bool lwt_ip_encap, nested_lwt_ip_encap, nested_lwt_run;
+	__be16 orig_proto = skb->protocol;
+	bool use_new_proto;
+	bool l3slave = false;
+	int iif = 0;
 	int ret;
 
+	if (orig_proto == htons(ETH_P_IP)) {
+		iif = IPCB(skb)->iif;
+		l3slave = ipv4_l3mdev_skb(IPCB(skb)->flags);
+	} else if (orig_proto == htons(ETH_P_IPV6)) {
+		iif = IP6CB(skb)->iif;
+		l3slave = ipv6_l3mdev_skb(IP6CB(skb)->flags);
+	}
+
 	/* Disabling BH is needed to protect per-CPU bpf_redirect_info between
 	 * BPF prog and skb_do_redirect().
 	 */
 	local_bh_disable();
 	bpf_net_ctx = bpf_net_ctx_set(&__bpf_net_ctx);
+	ri = bpf_net_ctx_get_ri();
+	nested_lwt_run = ri->kern_flags & BPF_RI_F_LWT_RUN;
+	nested_lwt_ip_encap = ri->kern_flags & BPF_RI_F_LWT_IP_ENCAP;
+	if (nested_lwt_run)
+		nested_lwt_ip_encap_state = ri->lwt_ip_encap;
+	ri->kern_flags &= ~BPF_RI_F_LWT_IP_ENCAP;
+	ri->kern_flags |= BPF_RI_F_LWT_RUN;
+	ri->lwt_ip_encap.iif = iif;
+	ri->lwt_ip_encap.cb_proto = orig_proto;
+	ri->lwt_ip_encap.l3slave = l3slave;
+	ri->lwt_ip_encap.cb_access = lwt->prog->cb_access;
+	if (!ri->lwt_ip_encap.cb_access)
+		memcpy(&saved_cb, skb->cb, sizeof(saved_cb));
 	bpf_compute_data_pointers(skb);
 	ret = bpf_prog_run_save_cb(lwt->prog, skb);
+	lwt_ip_encap = ri->kern_flags & BPF_RI_F_LWT_IP_ENCAP;
+	ri->kern_flags &= ~BPF_RI_F_LWT_IP_ENCAP;
+
+	use_new_proto = (ret == BPF_LWT_REROUTE &&
+			 lwt->prog->type != BPF_PROG_TYPE_LWT_OUT) ||
+			(ret == BPF_REDIRECT && can_redirect);
+	if (lwt_ip_encap) {
+		__be16 cb_proto = orig_proto;
+
+		if (!ri->lwt_ip_encap.cb_access) {
+			if (use_new_proto)
+				cb_proto = ri->lwt_ip_encap.cb_proto;
+			else
+				memcpy(skb->cb, &saved_cb, sizeof(saved_cb));
+		}
+		bpf_lwt_reset_ip_cb(skb, cb_proto, iif, l3slave,
+				    use_new_proto);
+	}
+	if (nested_lwt_run)
+		ri->lwt_ip_encap = nested_lwt_ip_encap_state;
+	else
+		ri->kern_flags &= ~BPF_RI_F_LWT_RUN;
+	if (nested_lwt_ip_encap)
+		ri->kern_flags |= BPF_RI_F_LWT_IP_ENCAP;
 
 	switch (ret) {
 	case BPF_OK:
@@ -604,6 +688,7 @@ static int handle_gso_encap(struct sk_buff *skb, bool ipv4, int encap_len)
 
 int bpf_lwt_push_ip_encap(struct sk_buff *skb, void *hdr, u32 len, bool ingress)
 {
+	struct bpf_redirect_info *ri;
 	bool is_udp_tunnel;
 	struct iphdr *iph;
 	bool ipv4;
@@ -657,6 +742,7 @@ int bpf_lwt_push_ip_encap(struct sk_buff *skb, void *hdr, u32 len, bool ingress)
 	memcpy(skb_network_header(skb), hdr, len);
 	bpf_compute_data_pointers(skb);
 	skb_clear_hash(skb);
+	ri = bpf_net_ctx_get_ri();
 
 	if (ipv4) {
 		skb->protocol = htons(ETH_P_IP);
@@ -669,6 +755,16 @@ int bpf_lwt_push_ip_encap(struct sk_buff *skb, void *hdr, u32 len, bool ingress)
 		skb->protocol = htons(ETH_P_IPV6);
 	}
 
+	if (ri->kern_flags & BPF_RI_F_LWT_RUN) {
+		ri->kern_flags |= BPF_RI_F_LWT_IP_ENCAP;
+		if (!ri->lwt_ip_encap.cb_access) {
+			bpf_lwt_reset_ip_cb(skb, ri->lwt_ip_encap.cb_proto,
+					    ri->lwt_ip_encap.iif,
+					    ri->lwt_ip_encap.l3slave, true);
+			ri->lwt_ip_encap.cb_proto = skb->protocol;
+		}
+	}
+
 	if (skb_is_gso(skb))
 		return handle_gso_encap(skb, ipv4, len);
 
-- 
2.55.0

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] bpf: clear stale IPv4 options after LWT encapsulation
  2026-09-16 17:04 [PATCH v2] bpf: clear stale IPv4 options after LWT encapsulation Weiming Shi
@ 2026-09-17 18:24 ` Daniel Borkmann
  2026-09-17 18:54   ` Daniel Borkmann
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Borkmann @ 2026-09-17 18:24 UTC (permalink / raw)
  To: Weiming Shi, Alexei Starovoitov, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
	Ihor Solodrai, John Fastabend, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman
  Cc: bpf, linux-kernel, netdev, Peter Oskolkov, Xiang Mei, stable

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));
+	if (!encap && skb->encapsulation)
+		bpf_lwt_reset_cb(skb);
  
  	switch (ret) {
  	case BPF_OK:


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] bpf: clear stale IPv4 options after LWT encapsulation
  2026-09-17 18:24 ` Daniel Borkmann
@ 2026-09-17 18:54   ` Daniel Borkmann
  2026-09-19  8:22     ` Weiming Shi
  0 siblings, 1 reply; 4+ messages in thread
From: Daniel Borkmann @ 2026-09-17 18:54 UTC (permalink / raw)
  To: Weiming Shi, Alexei Starovoitov, Andrii Nakryiko,
	Eduard Zingerman, Kumar Kartikeya Dwivedi, Martin KaFai Lau,
	Song Liu, Yonghong Song, Jiri Olsa, Emil Tsalapatis,
	Ihor Solodrai, John Fastabend, David S . Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Simon Horman
  Cc: bpf, linux-kernel, netdev, Peter Oskolkov, Xiang Mei, stable

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:


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] bpf: clear stale IPv4 options after LWT encapsulation
  2026-09-17 18:54   ` Daniel Borkmann
@ 2026-09-19  8:22     ` Weiming Shi
  0 siblings, 0 replies; 4+ messages in thread
From: Weiming Shi @ 2026-09-19  8:22 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Alexei Starovoitov, Andrii Nakryiko, Eduard Zingerman,
	Kumar Kartikeya Dwivedi, Martin KaFai Lau, Song Liu,
	Yonghong Song, Jiri Olsa, Emil Tsalapatis, Ihor Solodrai,
	John Fastabend, David S . Miller, Eric Dumazet, Jakub Kicinski,
	Paolo Abeni, Simon Horman, bpf, linux-kernel, netdev,
	Peter Oskolkov, Xiang Mei, stable

Daniel Borkmann <daniel@iogearbox.net> 于2026年9月18日周五 02:54写道:
>
> 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:
>

Hi,
Thanks for the suggestion. I'll simplify the fix, handle cb_access for
freplace, add BPF selftests, and send a v3.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-19  8:23 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-16 17:04 [PATCH v2] bpf: clear stale IPv4 options after LWT encapsulation Weiming Shi
2026-09-17 18:24 ` Daniel Borkmann
2026-09-17 18:54   ` Daniel Borkmann
2026-09-19  8:22     ` Weiming Shi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®