Skip to contentSkip to conversation

ipv6: sr: restore network header before forwarding

PATCH In review Jun Yang 2 participants 2 messages Aug 17 – Aug 20

Subsystems (3)
Networking Network drivers IPv4/IPv6
lore
Share & tools
lore

Jun Yang

author
#0 lore raw

PATCH 1/1 ipv6: sr: restore network header before forwarding

From: Jun Yang <[email protected]>

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.

Keep skb->data at the current extension header while rerouting and restore it by the exact network offset before each dst_input() handoff. Remove the fixed pull from the loopback path. This matches ipv6_rthdr_rcv() and handles any valid preceding extension header.

The trace was captured with kernel release
7.2.0-rc2-00023-g714b725feef4-dirty:

  BUG: KASAN: use-after-free in eth_header (net/ethernet/eth.c:86)
  Write of size 2 at addr ffff8881247b9a3e by task ipv6_srh_poc/9336
  Call Trace:
   eth_header (net/ethernet/eth.c:86)
   neigh_resolve_output (net/core/neighbour.c:1611)
   ip6_finish_output2 (net/ipv6/ip6_output.c:136)
   ip6_forward (net/ipv6/ip6_output.c:690)
   ipv6_rthdr_rcv (net/ipv6/exthdrs.c:832)
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]>

---
A KASAN reproducer for this issue is available if requested.

 net/ipv6/exthdrs.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)
net/ipv6/exthdrs.c +2 -3
index 9c677eb1d1a6..4c407520e9da 100644
@@ -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)
 	ip6_route_input(skb);
 
 	if (skb_dst(skb)->error) {
+		skb_push(skb, -skb_network_offset(skb));
 		dst_input(skb);
 		return -1;
 	}

@@ -476,10 +475,10 @@ static int ipv6_srh_rcv(struct sk_buff *skb)
 		}
 		ipv6_hdr(skb)->hop_limit--;
 
-		skb_pull(skb, sizeof(struct ipv6hdr));
 		goto looped_back;
 	}
 
+	skb_push(skb, -skb_network_offset(skb));
 	dst_input(skb);
 
 	return -1;
Signature · 1 line
2.55.0
reviewer
#1 lore raw

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]>
Quoted 14 lines
net/ipv6/exthdrs.c +0 -2
index 9c677eb1d1a61..4c407520e9daa 100644
@@ -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);

net/ipv6/exthdrs.c static int ipv6_srh_rcv(struct sk_buff *skb) · line 462

Quoted 3 lines
net/ipv6/exthdrs.c +0 -0
@@ -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?

[ ... ]