Re: [PATCH net] ipv6: rpl: unclone the skb before modifying the packet

[Kuniyuki Iwashima]

From: Andrea Mayer <andrea.mayer@xxxxxxxxxxx>
Date: Thu,  1 Oct 2026 20:21:36 +0200
> ipv6_rpl_srh_rcv() modifies the packet data, such as Segments Left and
> the destination address, without checking whether the skb is cloned.
> The skb may be cloned, for example by an AF_PACKET socket receiving on
> the ingress device. ipv6_rpl_srh_rcv() then writes into the packet data
> shared with the clone. A read from that socket can return the modified
> packet instead of the received one.
> 
> The only pskb_expand_head() in the function runs after Segments Left and
> the destination address are written, and only when Segments Left reaches
> 0 or there is not enough headroom.
> 
> Call pskb_expand_head() on a cloned skb before the packet is modified
> (i.e., before Segments Left is decremented), as ipv6_srh_rcv() does.
> On failure, drop the packet with SKB_DROP_REASON_NOMEM.
> 
> Fixes: a2f4c143d76b ("ipv6: rpl: Fix Route of Death.")
> Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
> Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925133807.32-1-andrea.mayer%40uniroma2.it
> Signed-off-by: Andrea Mayer <andrea.mayer@xxxxxxxxxxx>
> ---
>  net/ipv6/exthdrs.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
> index 09a4552f7f08..8fcf18e6114d 100644
> --- a/net/ipv6/exthdrs.c
> +++ b/net/ipv6/exthdrs.c
> @@ -548,6 +548,17 @@ static int ipv6_rpl_srh_rcv(struct sk_buff *skb, struct inet6_dev *idev)
>  		return -1;
>  	}
>  
> +	if (skb_cloned(skb)) {
> +		if (pskb_expand_head(skb, 0, 0, GFP_ATOMIC)) {

ipv6_rpl_srh_rcv() already has pskb_expand_head() call later
and I think this can be merged there.


> +			__IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)),
> +					IPSTATS_MIB_OUTDISCARDS);
> +			kfree_skb_reason(skb, SKB_DROP_REASON_NOMEM);
> +			return -1;
> +		}
> +
> +		hdr = (struct ipv6_rpl_sr_hdr *)skb_transport_header(skb);
> +	}
> +
>  	hdr->segments_left--;
>  	i = n - hdr->segments_left;

This and the later swap(ipv6_hdr(skb)->daddr, ohdr->rpl_segaddr[i])
only modify the header before the existing pskb_expand_head().

Also this seems wrong because skb_postpull_rcsum() is applied to
the modified header, which should corrupt checksum.

I think we can merge the clone check and pskb_expand_head() with
skb_cow() and tmp IPv6 buffer like this.

compiled only:

---8<---
diff --git a/net/ipv6/exthdrs.c b/net/ipv6/exthdrs.c
index 09a4552f7f08..af285edd4d16 100644
--- a/net/ipv6/exthdrs.c
+++ b/net/ipv6/exthdrs.c
@@ -485,6 +485,7 @@ static int ipv6_rpl_srh_rcv(struct sk_buff *skb, struct inet6_dev *idev)
 	struct net *net = dev_net(skb->dev);
 	struct ipv6hdr *oldhdr;
 	unsigned int chdr_len;
+	struct in6_addr addr;
 	unsigned char *buf;
 	int accept_rpl_seg;
 	int i, err;
@@ -548,9 +549,6 @@ static int ipv6_rpl_srh_rcv(struct sk_buff *skb, struct inet6_dev *idev)
 		return -1;
 	}
 
-	hdr->segments_left--;
-	i = n - hdr->segments_left;
-
 	buf = kcalloc(struct_size(hdr, segments.addr, n + 2), 2, GFP_ATOMIC);
 	if (unlikely(!buf)) {
 		kfree_skb(skb);
@@ -559,6 +557,8 @@ static int ipv6_rpl_srh_rcv(struct sk_buff *skb, struct inet6_dev *idev)
 
 	ohdr = (struct ipv6_rpl_sr_hdr *)buf;
 	ipv6_rpl_srh_decompress(ohdr, hdr, &ipv6_hdr(skb)->daddr, n);
+	ohdr->segments_left--;
+	i = n - ohdr->segments_left;
 	chdr = (struct ipv6_rpl_sr_hdr *)(buf + ((ohdr->hdrlen + 1) << 3));
 
 	if (ipv6_addr_is_multicast(&ohdr->rpl_segaddr[i])) {
@@ -575,28 +575,24 @@ static int ipv6_rpl_srh_rcv(struct sk_buff *skb, struct inet6_dev *idev)
 		return -1;
 	}
 
-	swap(ipv6_hdr(skb)->daddr, ohdr->rpl_segaddr[i]);
+	addr = ohdr->rpl_segaddr[i];
+	ohdr->rpl_segaddr[i] = ipv6_hdr(skb)->daddr;
 
-	ipv6_rpl_srh_compress(chdr, ohdr, &ipv6_hdr(skb)->daddr, n);
-
-	oldhdr = ipv6_hdr(skb);
+	ipv6_rpl_srh_compress(chdr, ohdr, &addr, n);
 
 	skb_pull(skb, ((hdr->hdrlen + 1) << 3));
-	skb_postpull_rcsum(skb, oldhdr,
+	skb_postpull_rcsum(skb, ipv6_hdr(skb),
 			   sizeof(struct ipv6hdr) + ((hdr->hdrlen + 1) << 3));
 	chdr_len = sizeof(struct ipv6hdr) + ((chdr->hdrlen + 1) << 3);
-	if (unlikely(!hdr->segments_left ||
-		     skb_headroom(skb) < chdr_len + skb->mac_len)) {
-		if (pskb_expand_head(skb, chdr_len + skb->mac_len, 0,
-				     GFP_ATOMIC)) {
-			__IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)), IPSTATS_MIB_OUTDISCARDS);
-			kfree_skb(skb);
-			kfree(buf);
-			return -1;
-		}
-
-		oldhdr = ipv6_hdr(skb);
+	if (unlikely(skb_cow(skb, chdr_len + skb->mac_len))) {
+		__IP6_INC_STATS(net, ip6_dst_idev(skb_dst(skb)), IPSTATS_MIB_OUTDISCARDS);
+		kfree_skb_reason(skb, SKB_DROP_REASON_NOMEM);
+		kfree(buf);
+		return -1;
 	}
+
+	oldhdr = ipv6_hdr(skb);
+
 	skb_push(skb, chdr_len);
 	skb_reset_network_header(skb);
 	skb_mac_header_rebuild(skb);
@@ -605,6 +601,7 @@ static int ipv6_rpl_srh_rcv(struct sk_buff *skb, struct inet6_dev *idev)
 	memmove(ipv6_hdr(skb), oldhdr, sizeof(struct ipv6hdr));
 	memcpy(skb_transport_header(skb), chdr, (chdr->hdrlen + 1) << 3);
 
+	ipv6_hdr(skb)->daddr = addr;
 	ipv6_hdr(skb)->payload_len = htons(skb->len - sizeof(struct ipv6hdr));
 	skb_postpush_rcsum(skb, ipv6_hdr(skb),
 			   sizeof(struct ipv6hdr) + ((chdr->hdrlen + 1) << 3));
---8<---