Re: [PATCH bpf v2 3/4] bpf: Update transport_header when encapsulating UDP tunnel in lwt

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch only updates `skb->transport_header` for UDP tunnels, leaving it stale for GRE encapsulation, which breaks hardware offloads (e.g., TSO/checksums) on modern NICs like `ice`.
- [High] `handle_gso_encap()` hardcodes the IPv4 header length to 20 bytes, failing to account for IP options, which causes out-of-bounds reads when parsing the encapsulated L4 header.
--

commit 069ecde1299856dcf6cef2dee89cb71a921e3024
Author: Leon Hwang <leon.hwang@linux.dev>

bpf: Update transport_header when encapsulating UDP tunnel in lwt

This commit fixes checksum offloading for encapsulated UDP tunnels in the
bpf_lwt_push_ip_encap() helper by updating the stale skb->transport_header.
Without this, hardware NICs like ice may compute incorrect checksums and
drop packets.

> diff --git a/net/core/lwt_bpf.c b/net/core/lwt_bpf.c
> index c306120e11d29..1d556dec94b4f 100644
> --- a/net/core/lwt_bpf.c
> +++ b/net/core/lwt_bpf.c
[ ... ]
> @@ -615,10 +616,16 @@ int bpf_lwt_push_ip_encap(struct sk_buff *skb, void *hdr, u32 len, bool ingress)
>  		ipv4 = true;
>  		if (unlikely(iph->ihl < 5 || len < iph->ihl * 4))
>  			return -EINVAL;
> +		is_udp_tunnel = iph->protocol == IPPROTO_UDP;
> +		if (unlikely(is_udp_tunnel && len < iph->ihl * 4 + sizeof(struct udphdr)))
> +			return -EINVAL;

[Severity: High]
This is a pre-existing issue, but does handle_gso_encap() handle IPv4
options correctly when parsing the encapsulated L4 header?

bpf_lwt_push_ip_encap() explicitly allows IPv4 headers with options by
checking iph->ihl < 5. When a GSO packet is encapsulated,
handle_gso_encap() is called.

In handle_gso_encap(), the offset to the outer L4 header is hardcoded to 20
bytes (sizeof(struct iphdr)), ignoring the actual iph->ihl.

If a BPF program pushes an encapsulation header with IPv4 options, wouldn't
the next_hdr pointer incorrectly point to the IP options payload instead of
the GRE or UDP header?

This could lead to out-of-bounds reads or misparsing the L4 fields,
potentially misclassifying the GSO type (e.g., wrongly setting
SKB_GSO_UDP_TUNNEL_CSUM) which leads to incorrect offload processing or
dropped packets.

[ ... ]
> @@ -641,6 +648,10 @@ int bpf_lwt_push_ip_encap(struct sk_buff *skb, void *hdr, u32 len, bool ingress)
>  		skb_postpush_rcsum(skb, iph, len);
>  	skb_reset_network_header(skb);
>  	memcpy(skb_network_header(skb), buff, len);
> +	if (ipv4 && is_udp_tunnel)
> +		skb_set_transport_header(skb, skb_network_offset(skb) + iph->ihl * 4);
> +	else if (!ipv4 && is_udp_tunnel)
> +		skb_set_transport_header(skb, skb_network_offset(skb) + sizeof(struct ipv6hdr));

[Severity: High]
This isn't a bug introduced by this patch, but does this leave the
transport_header stale for GRE encapsulation?

bpf_lwt_push_ip_encap() permits pushing IP encapsulation headers with
IPPROTO_GRE, but this only updates skb->transport_header when is_udp_tunnel
is true.

For GRE packets, the transport_header remains stale and points to the inner
packet's transport header. The ice driver (and others) handles GRE and UDP
tunnel GSO offloads by calculating the outer L4 header length as:

skb_inner_network_header(skb) - skb_transport_header(skb)

With a stale transport_header, this driver length calculation evaluates to
(inner_network_offset) - (inner_transport_offset), yielding a negative
value. This large unsigned length exceeds ICE_TXD_L4LEN_MAX, causing
hardware offloads to fail and the packet to be dropped.

>  	bpf_compute_data_pointers(skb);
>  	skb_clear_hash(skb);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529151351.69911-1-leon.hwang@linux.dev?part=3