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

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

[Andrea Mayer]

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@kernel.org>
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925133807.32-1-andrea.mayer%40uniroma2.it
Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
---
 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)) {
+			__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;
 
-- 
2.43.0

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

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

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

[Andrea Mayer]

On Thu, 01 Oct 2026 18:29:07 +0000
netdev-bot+sinfo@kernel.org wrote:

> Hi!
> 
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
> 
>  - Whether the issue was actually triggered, or is only theoretical
>    (e.g. found by code inspection). If it was triggered please include
>    the symptoms, like the stack trace or error messages.
> 
> [snip]

Hi,

Yes, I triggered it. I reproduced it in a VM.

One namespace sends pings to fc00:2::1 through the RPL segment
fc00::2, which belongs to a second namespace with rpl_seg_enabled=1.
Each ping arrives with destination address fc00::2 and a routing header
with Segments Left 1 that carries fc00:2::1. In the second namespace, a
Python program opens an AF_PACKET socket and reads the queued frames
only after the pings were processed. For each frame with a routing
header, it prints the destination address and Segments Left.

Without the fix, all these frames showed:

  daddr fc00:2::1 segments_left 0

These are the values written by ipv6_rpl_srh_rcv(), not the received
ones. With the fix, all the frames with a routing header showed the
received values:

  daddr fc00::2 segments_left 1

To reproduce it, I did not modify the kernel or use error injection.

This is how the packet reaches ipv6_rpl_srh_rcv(), with some calls
left out:

__netif_receive_skb_one_core
  __netif_receive_skb_core
    deliver_skb               [orig: users=2]
      packet_rcv
        skb_clone             clone queued to the AF_PACKET socket
        consume_skb(orig)     [orig: users=1, cloned=1]
  ipv6_rcv
    ip6_rcv_core              skb_share_check: no-op [orig: users=1]
    [...]
      ip6_protocol_deliver_rcu
        ipv6_rthdr_rcv
          ipv6_rpl_srh_rcv    writes into the data shared with the clone

The reproducer is a shell script and a Python program.
I can post it if it helps.

Thanks,
Andrea

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

[Kuniyuki Iwashima]

From: Andrea Mayer <andrea.mayer@uniroma2.it>
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@kernel.org>
> Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260925133807.32-1-andrea.mayer%40uniroma2.it
> Signed-off-by: Andrea Mayer <andrea.mayer@uniroma2.it>
> ---
>  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<---