Re: [PATCH net] ipv6: sr: restore network header before forwarding

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.
Full review at: https://sashiko.dev/#/patchset/[email protected]?part=1
---
--- Patch [1]: [PATCH net] ipv6: sr: restore network header before forwarding ---
commit afc1a14387ef1e89c0ab5549e08d7004f0646b94
Author: Jun Yang <[email protected]>

ipv6: sr: restore network header before forwarding

ipv6_srh_rcv() runs with skb->data at the Segment Routing Header while
skb_network_header() points at the fixed IPv6 header. It restores the
data position by pushing sizeof(struct ipv6hdr) before dst_input(),
which assumes the SRH immediately follows the IPv6 header. If another
extension header precedes the SRH, skb_network_offset() remains negative
and neighbour output passes it to __skb_pull() as an unsigned length.

Fixes: 1ababeba4a21 ("ipv6: implement dataplane support for rthdr type 4 (Segment Routing Header)")
Cc: [email protected]
Reported-by: TencentOS Corvus AI <[email protected]>
Assisted-by: tencentos-corvus-ai:kimi-k3
Signed-off-by: Jun Yang <[email protected]>

> diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
> index 9c677eb1d1a61..4c407520e9daa 100644
> --- a/net/ipv6/exthdrs.c
> +++ b/net/ipv6/exthdrs.c
> @@ -452,8 +452,6 @@ static int ipv6_srh_rcv(struct sk_buff *skb)
>  	hdr->segments_left--;
>  	addr = hdr->segments + hdr->segments_left;
>  
> -	skb_push(skb, sizeof(struct ipv6hdr));
> -
>  	if (skb->ip_summed == CHECKSUM_COMPLETE)
>  		seg6_update_csum(skb);
>  
> @@ -462,6 +460,7 @@ static int ipv6_srh_rcv(struct sk_buff *skb)
>  	ipv6_hdr(skb)->daddr = *addr;
>  
>  	ip6_route_input(skb);

[Severity: High]
This is a pre-existing issue, but does removing this push expose a BPF flow
dissection bug for all SRH packets?

When the skb data pointer is left at the SRH instead of the network header,
skb_network_offset(skb) becomes negative.

ip6_route_input() passes the skb to fib6_rules_early_flow_dissect(), which
eventually forwards this negative offset as nhoff to bpf_flow_dissect() in
net/core/flow_dissector.c:

bpf_flow_dissect() {
    ...
    flow_keys->nhoff = nhoff;
    flow_keys->thoff = flow_keys->nhoff;
    ...
}

Because flow_keys->nhoff is a u16, the negative offset wraps around to a
huge positive offset before BPF execution.

Later in the same function:

    flow_keys->nhoff = clamp_t(u16, flow_keys->nhoff, nhoff, hlen);

After BPF execution, clamp_t() operates on the wrapped u16 values,
permanently capping the offsets to hlen.

While this previously affected packets with multiple extension headers, does
delaying the push until after routing cause BPF flow dissection to fail for
all SRH packets?

[ ... ]