[PATCH net v2 0/2] ipv4: do not write shared IP options when sending ICMP errors

[PATCH net v2 0/2] ipv4: do not write shared IP options when sending ICMP errors

[Eric Dumazet]

__ip_options_compile() updates Record Route and Timestamp options in
place. Two callers recompile options on an skb that can be a clone
sharing its data with other skbs, and thus write into shared packet
data:

 - ipv4_send_dest_unreach(), called from ipv4_link_failure()
 - cipso_v4_error(), called from the LSM socket receive hooks

If clones are processed concurrently, an option pointer can change
after it has been checked, corrupting the next option's length, which
__ip_options_echo() then trusts when building the ICMP reply.

Patch 1 recompiles the options on a private pskb_copy(), because
callers of dst_link_failure() keep pointers into skb->head.
Patch 2 unclones the skb, as ip_rcv_options() already does.
The two patches are independent.

The issues were reported by Kyle Zeng and found by an AI code review
agent. They were not reproduced, and the fixes were only build-tested.

v2:
 - patch 1: use pskb_copy() instead of skb_unclone(), since callers of
   dst_link_failure() such as ip_tunnel_xmit() cache pointers into
   skb->head across the call (Sashiko).
 - patch 2: no change.

v1: https://lore.kernel.org/netdev/20261106234313.2537882-1-edumazet@kernel.org/

Eric Dumazet (2):
  ipv4: recompile IP options on a private copy in
    ipv4_send_dest_unreach()
  cipso: unshare headers before recompiling IP options in
    cipso_v4_error()

 net/ipv4/cipso_ipv4.c |  3 +++
 net/ipv4/route.c      | 15 ++++++++++++++-
 2 files changed, 17 insertions(+), 1 deletion(-)


base-commit: 1d5e2c57e9ffc134e14b3f837c8d7c53b508bd22
-- 
2.53.0

[PATCH net v2 1/2] ipv4: recompile IP options on a private copy in ipv4_send_dest_unreach()

[Eric Dumazet]

__ip_options_compile() updates Record Route and Timestamp options in
place. ipv4_send_dest_unreach() can receive skbs whose data is shared
with clones (for example TC-mirred copies or TCP transmit clones), and
pskb_network_may_pull() does not unshare an already linear IPv4 header.

Concurrent link-failure processing of such clones can therefore change
an option pointer after it has been checked. For example, a Record Route
pointer can advance during fib_compute_spec_dst() (called from
spec_dst_fill()), causing the following write to overwrite the next
option's length. __ip_options_echo() then trusts that length when
building the ICMP reply, which can overflow both the stack reply-option
buffer and the cork-option allocation.

Uncloning the skb is not an option: ipv4_send_dest_unreach() runs as
the dst_link_failure() callback, and callers such as ip_tunnel_xmit()
keep pointers into skb->head (inner_iph, and tnl_params for GRE devices
with header_ops) across the call.

Instead, recompile and echo the options on a private pskb_copy(),
leaving the caller's skb untouched. This also stops rewriting the
options of a packet the caller may still transmit. If the copy fails,
do not send the ICMP error.

The issue was found by an AI code review agent. It was not reproduced,
and the fix was only build-tested.

Fixes: ed0de45a1008 ("ipv4: recompile ip options in ipv4_link_failure")
Reported-by: Kyle Zeng <kylebot@openai.com>
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
v2: use a private pskb_copy() instead of skb_unclone(), callers of
    dst_link_failure() cache pointers into skb->head (Sashiko).
v1: https://lore.kernel.org/netdev/20261106234313.2537882-1-edumazet@kernel.org/

 net/ipv4/route.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 3a90c8d28b940bf615c165d9b30ac634943586e1..9af72cb892034deff2bb3169911c02c025d73cf2 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -1256,6 +1256,7 @@ EXPORT_INDIRECT_CALLABLE(ipv4_dst_check);
 
 static void ipv4_send_dest_unreach(struct sk_buff *skb)
 {
+	struct sk_buff *nskb = NULL;
 	struct inet_skb_parm parm;
 	struct net_device *dev;
 	int res;
@@ -1271,6 +1272,16 @@ static void ipv4_send_dest_unreach(struct sk_buff *skb)
 	if (ip_hdr(skb)->ihl > 5) {
 		if (!pskb_network_may_pull(skb, ip_hdr(skb)->ihl * 4))
 			return;
+
+		/* __ip_options_compile() writes into the options, and
+		 * callers of dst_link_failure() may cache pointers into
+		 * skb->head: work on a private copy.
+		 */
+		nskb = pskb_copy(skb, GFP_ATOMIC);
+		if (!nskb)
+			return;
+		skb = nskb;
+
 		parm.opt.optlen = ip_hdr(skb)->ihl * 4 - sizeof(struct iphdr);
 
 		rcu_read_lock();
@@ -1279,9 +1290,11 @@ static void ipv4_send_dest_unreach(struct sk_buff *skb)
 		rcu_read_unlock();
 
 		if (res)
-			return;
+			goto out;
 	}
 	__icmp_send(skb, ICMP_DEST_UNREACH, ICMP_HOST_UNREACH, 0, &parm);
+out:
+	consume_skb(nskb);
 }
 
 static void ipv4_link_failure(struct sk_buff *skb)
-- 
2.53.0

Re: [PATCH net v2 1/2] ipv4: recompile IP options on a private copy in ipv4_send_dest_unreach()

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [High] ipv4: skb->head reallocation and ihl TOCTOU in
  ipv4_send_dest_unreach()
- [High] ipv4: uninitialized headroom access after pskb_copy() when
  network_offset < 0
--

commit 5a814100bdfbbe07fb37410d0b393cb7deb2e38a
Author: Eric Dumazet <edumazet@kernel.org>

ipv4: recompile IP options on a private copy in ipv4_send_dest_unreach()

This commit uses pskb_copy() in ipv4_send_dest_unreach() so IP options are
recompiled and echoed on a private skb copy rather than modifying the
caller's skb.

> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 3a90c8d28b940..9af72cb892034 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -1256,6 +1256,7 @@ EXPORT_INDIRECT_CALLABLE(ipv4_dst_check);
>  
>  static void ipv4_send_dest_unreach(struct sk_buff *skb)
>  {
> +	struct sk_buff *nskb = NULL;
>  	struct inet_skb_parm parm;
>  	struct net_device *dev;
>  	int res;
> @@ -1271,6 +1272,16 @@ static void ipv4_send_dest_unreach(struct sk_buff *skb)
>  	if (ip_hdr(skb)->ihl > 5) {
>  		if (!pskb_network_may_pull(skb, ip_hdr(skb)->ihl * 4))
>  			return;
> +
> +		/* __ip_options_compile() writes into the options, and
> +		 * callers of dst_link_failure() may cache pointers into
> +		 * skb->head: work on a private copy.
> +		 */
> +		nskb = pskb_copy(skb, GFP_ATOMIC);
> +		if (!nskb)
> +			return;
> +		skb = nskb;

[Severity: High]
Can calling pskb_network_may_pull() on the caller's skb before pskb_copy()
still reallocate skb->head and cause a use-after-free in callers of
dst_link_failure()?

Tunnel transmit paths such as ipip_tunnel_xmit() and ipgre_xmit() call
pskb_inet_may_pull(), which only pulls sizeof(struct iphdr) and leaves IPv4
options in nonlinear fragments.  When ip_tunnel_xmit() calls
dst_link_failure(skb) before skb_cow_head(), pskb_network_may_pull(skb,
ip_hdr(skb)->ihl * 4) pulls the option bytes via __pskb_pull_tail() ->
pskb_expand_head() on the caller's skb (and pskb_copy() itself can call
skb_orphan_frags() -> skb_unclone() on a cloned skb with zerocopy frags),
freeing the old skb->head that ip_tunnel_xmit()'s cached inner_iph and
tnl_params pointers still reference.

> +
>  		parm.opt.optlen = ip_hdr(skb)->ihl * 4 - sizeof(struct iphdr);

[Severity: High]
Does re-reading ip_hdr(skb)->ihl after pskb_copy() also allow a TOCTOU race
when skb->head is shared, or an uninitialized read when
skb_network_offset(skb) is negative?

If a concurrent clone modifies ip_hdr(skb)->ihl between the check on the
original skb and pskb_copy(), re-reading ip_hdr(nskb)->ihl without
re-validating it can underflow parm.opt.optlen or exceed the pulled length
in nskb, leading to out-of-bounds accesses in __ip_options_compile() and
__ip_options_echo().

Separately, when a packet is redirected (for example via act_mirred or
bpf_redirect) to an NBMA GRE device with header_ops set, ipgre_xmit() pulls
pull_len bytes before gre_build_header() pushes tun_hlen, entering
ip_tunnel_xmit() -> dst_link_failure() with a negative
skb_network_offset(skb).  pskb_network_may_pull() accepts negative network
offsets, but pskb_copy() only copies data starting at skb->data and leaves
the headroom [nskb->head .. nskb->data] uninitialized while preserving
nskb->network_header in that headroom.  ip_hdr(nskb) then points into
uninitialized heap memory, corrupting parm.opt.optlen and causing
out-of-bounds reads and writes in __ip_options_compile().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109131736.3060756-1-edumazet@kernel.org?part=1

Re: [PATCH net v2 1/2] ipv4: recompile IP options on a private copy in ipv4_send_dest_unreach()

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 0 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [High] ipv4_send_dest_unreach() (net/ipv4/route.c) now re-reads the
  IPv4 header from the private copy (nskb = pskb_copy(skb, GFP_ATOMIC))…

--

ipv4: recompile IP options on a private copy in ipv4_send_dest_unreach()

This patch changes ipv4_send_dest_unreach() so that it recompiles and
echoes the IP options on a private pskb_copy() instead of the caller's
skb. __ip_options_compile() rewrites RR/TS options in place, and the skb
may share data with clones. If the copy fails, no ICMP error is sent.

> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 3a90c8d28b940..9af72cb892034 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c

[ ... ]

> @@ -1271,6 +1272,16 @@ static void ipv4_send_dest_unreach(struct sk_buff *skb)
>  	if (ip_hdr(skb)->ihl > 5) {
>  		if (!pskb_network_may_pull(skb, ip_hdr(skb)->ihl * 4))
>  			return;
> +
> +		/* __ip_options_compile() writes into the options, and
> +		 * callers of dst_link_failure() may cache pointers into
> +		 * skb->head: work on a private copy.
> +		 */
> +		nskb = pskb_copy(skb, GFP_ATOMIC);
> +		if (!nskb)
> +			return;
> +		skb = nskb;
> +
>  		parm.opt.optlen = ip_hdr(skb)->ihl * 4 - sizeof(struct iphdr);

[Severity: High]
What happens here if skb_network_offset(skb) is negative when we get
here?

__pskb_copy_fclone() reserves the headroom but copies only the linear
data that starts at skb->data:

net/core/skbuff.c:__pskb_copy_fclone() {
    ...
	skb_reserve(n, headroom);
	skb_put(n, skb_headlen(skb));
	skb_copy_from_linear_data(skb, n->data, n->len);
    ...
	skb_copy_header(n, skb);
}

skb_copy_header() keeps network_header at the same offset from head. If
the network header sits before skb->data, ip_hdr(nskb) points into
headroom that was never initialized.

The earlier checks don't catch this. pskb_network_may_pull() is
pskb_may_pull(skb, skb_network_offset(skb) + len), and that passes with
an offset of -20. The version and ihl checks ran on the original skb and
are not repeated on the copy.

One way to get a negative offset looks like this. A tc mirred (or
bpf_redirect) egress redirect of a routed IPv4 packet goes to an
NBMA/multicast ipgre device that has header_ops. That device is
ARPHRD_IPGRE, so tcf_mirred_to_dev() hands it over with data at the
network header and keeps the IPv4 dst:

tcf_mirred_to_dev()
  ipgre_xmit()             /* header_ops branch: skb_pull(hlen + 20) */
    __gre_xmit()           /* gre_build_header() pushes tun_hlen */
      ip_tunnel_xmit()     /* err_count > 0 */
        dst_link_failure()
          ipv4_link_failure()
            ipv4_send_dest_unreach()   /* skb_network_offset() == -20 */

Before this patch, the function read real header bytes from the original
skb's headroom. Now parm.opt.optlen is computed from uninitialized
memory. optlen is an unsigned char, so ihl < 5 wraps to 236..252.

Can __ip_options_compile(..., nskb, NULL) then parse and write past
nskb's tail? The writes include the IPOPT_END zero fill and the RR/TS
pointer and address writes. They could reach skb_shared_info (nr_frags,
frags[], frag_list), which pskb_copy() filled with references to the
original skb's frags.

__icmp_send() would also use the garbage saddr/daddr/tos/frag_off/
protocol. icmp_glue_bits() copies from skb_network_offset(nskb), so
uninitialized headroom bytes could go out in the ICMP payload.

Would it make sense to bail out when skb_network_offset(skb) < 0, use a
copy that includes the headroom, or copy only the IP header and options
into a private buffer?

>  
>  		rcu_read_lock();

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261109131736.3060756-1-edumazet%40kernel.org

[PATCH net v2 2/2] cipso: unshare headers before recompiling IP options in cipso_v4_error()

[Eric Dumazet]

cipso_v4_error() recompiles the IP options of the offending packet with
__ip_options_compile() before echoing them in an ICMP error.
__ip_options_compile() updates Record Route and Timestamp options in
place.

cipso_v4_error() is called from the LSM socket receive hooks through
netlbl_skbuff_err(). ip_rcv_options() unshared the IPv4 header earlier,
but at socket delivery time the skb can again be a clone sharing its
data with other skbs, such as the per-socket clones made for raw sockets
or multicast/broadcast delivery. Writing to the options then modifies
packet data seen by the other users of the buffer, and a concurrent
recompilation would hit the same check-then-write race as the one fixed
in ipv4_send_dest_unreach().

Make the header private before recompiling options, as ip_rcv_options()
already does. If the copy fails, do not send the ICMP error.

The issue was found by an AI code review agent. It was not reproduced,
and the fix was only build-tested.

Fixes: 3da1ed7ac398 ("net: avoid use IPCB in cipso_v4_error")
Reported-by: Kyle Zeng <kylebot@openai.com>
Cc: Paul Moore <paul@paul-moore.com>
Cc: linux-security-module@vger.kernel.org
Signed-off-by: Eric Dumazet <edumazet@kernel.org>
---
v2: no change.
v1: https://lore.kernel.org/netdev/20261106234313.2537882-1-edumazet@kernel.org/

 net/ipv4/cipso_ipv4.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ipv4/cipso_ipv4.c b/net/ipv4/cipso_ipv4.c
index e65a3f29d2caf31c34cc5aaaf862257d7d374275..f5f6a812ab9ef05117e002779baf136e615c043d 100644
--- a/net/ipv4/cipso_ipv4.c
+++ b/net/ipv4/cipso_ipv4.c
@@ -1725,6 +1725,9 @@ void cipso_v4_error(struct sk_buff *skb, int error, u32 gateway)
 	 * so we can not use icmp_send and IPCB here.
 	 */
 
+	if (skb_unclone(skb, GFP_ATOMIC))
+		return;
+
 	memset(&parm, 0, sizeof(parm));
 	parm.opt.optlen = ip_hdr(skb)->ihl * 4 - sizeof(struct iphdr);
 	rcu_read_lock();
-- 
2.53.0

Re: [PATCH net v2 2/2] cipso: unshare headers before recompiling IP options in cipso_v4_error()

[Paul Moore]

On Mon, Nov 9, 2026 at 8:17 AM Eric Dumazet <edumazet@kernel.org> wrote:
>
> cipso_v4_error() recompiles the IP options of the offending packet with
> __ip_options_compile() before echoing them in an ICMP error.
> __ip_options_compile() updates Record Route and Timestamp options in
> place.
>
> cipso_v4_error() is called from the LSM socket receive hooks through
> netlbl_skbuff_err(). ip_rcv_options() unshared the IPv4 header earlier,
> but at socket delivery time the skb can again be a clone sharing its
> data with other skbs, such as the per-socket clones made for raw sockets
> or multicast/broadcast delivery. Writing to the options then modifies
> packet data seen by the other users of the buffer, and a concurrent
> recompilation would hit the same check-then-write race as the one fixed
> in ipv4_send_dest_unreach().
>
> Make the header private before recompiling options, as ip_rcv_options()
> already does. If the copy fails, do not send the ICMP error.
>
> The issue was found by an AI code review agent. It was not reproduced,
> and the fix was only build-tested.
>
> Fixes: 3da1ed7ac398 ("net: avoid use IPCB in cipso_v4_error")
> Reported-by: Kyle Zeng <kylebot@openai.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: linux-security-module@vger.kernel.org
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>
> ---
> v2: no change.
> v1: https://lore.kernel.org/netdev/20261106234313.2537882-1-edumazet@kernel.org/
>
>  net/ipv4/cipso_ipv4.c | 3 +++
>  1 file changed, 3 insertions(+)

My apologies, I missed that the v2 had been posted while ACK'ing the
v1.  Anyway, just so it's explicit ...

Acked-by: Paul Moore <paul@paul-moore.com>

-- 
paul-moore.com

Re: [PATCH net v2 2/2] cipso: unshare headers before recompiling IP options in cipso_v4_error()

[sashiko-bot]

> cipso_v4_error() recompiles the IP options of the offending packet with
> __ip_options_compile() before echoing them in an ICMP error.
> __ip_options_compile() updates Record Route and Timestamp options in
> place.
> 
> cipso_v4_error() is called from the LSM socket receive hooks through
> netlbl_skbuff_err(). ip_rcv_options() unshared the IPv4 header earlier,
> but at socket delivery time the skb can again be a clone sharing its
> data with other skbs, such as the per-socket clones made for raw sockets
> or multicast/broadcast delivery. Writing to the options then modifies
> packet data seen by the other users of the buffer, and a concurrent
> recompilation would hit the same check-then-write race as the one fixed
> in ipv4_send_dest_unreach().
> 
> Make the header private before recompiling options, as ip_rcv_options()
> already does. If the copy fails, do not send the ICMP error.
> 
> The issue was found by an AI code review agent. It was not reproduced,
> and the fix was only build-tested.
> 
> Fixes: 3da1ed7ac398 ("net: avoid use IPCB in cipso_v4_error")
> Reported-by: Kyle Zeng <kylebot@openai.com>
> Cc: Paul Moore <paul@paul-moore.com>
> Cc: linux-security-module@vger.kernel.org
> Signed-off-by: Eric Dumazet <edumazet@kernel.org>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109131736.3060756-1-edumazet@kernel.org?part=2