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

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] bpf: incorrect SKB CB state when cb_access is false
--

commit 81b40ecbc1d9468e9497a57756458b9bd7e91f03
Author: Xiang Mei <xmei5@asu.edu>

bpf: clear stale IPv4 options after LWT encapsulation

This commit addresses an out-of-bounds stack write caused by a stale IPv4
header length being interpreted as an option length. It tracks active LWT
runs and resets the control block state to preserve consistency across
nested runs and protocol family changes.

> diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
> --- a/net/core/lwt_bpf.c
> +++ b/net/core/lwt_bpf.c
> @@ -80,6 +80,31 @@ static int run_lwt_bpf(struct sk_buff *skb, struct bpf_lwt_prog *lwt,
[ ... ]
>  	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;
> +
> +	if (lwt_ip_encap && 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,
> +				    (ret == BPF_LWT_REROUTE &&
> +				     lwt->prog->type != BPF_PROG_TYPE_LWT_OUT) ||
> +				    (ret == BPF_REDIRECT && can_redirect));

[Severity: High]
Does skipping the control block reset when cb_access is false cause issues if
the packet is returned to the old protocol handler?

If a BPF program with cb_access set to false changes the IP encapsulation
family via bpf_lwt_push_ip_encap() and then returns BPF_OK, it seems the
original protocol stack could end up reading a control block formatted for the
wrong protocol family. Could this result in corrupted packet delivery state,
such as lost routing flags like IPSKB_L3SLAVE or reading a zeroed nhoff in
IPv6?

[ ... ]
> @@ -669,6 +740,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;
> +		}
> +	}

[Severity: High]
Is it safe to eagerly convert the control block to the new protocol here when
cb_access is false?

This appears to assume the packet will always be subsequently processed as the
new protocol. Since the restoration logic in run_lwt_bpf() is completely
bypassed when cb_access is false, this might leave the control block in an
inconsistent layout when the packet is handled by the original protocol handler.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260915170147.3943392-2-bestswngs@gmail.com?part=1