public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] ipv6: icmp: clear skb2->cb[] in ip6_err_gen_icmpv6_unreach()
@ 2026-03-26 20:26 Eric Dumazet
  2026-03-27 17:54 ` Ido Schimmel
  0 siblings, 1 reply; 2+ messages in thread
From: Eric Dumazet @ 2026-03-26 20:26 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Simon Horman, David Ahern, netdev, eric.dumazet, Eric Dumazet,
	Ido Schimmel, Oskar Kjos

Sashiko AI-review observed:

  In ip6_err_gen_icmpv6_unreach(), the skb is an outer IPv4 ICMP error packet
  where its cb contains an IPv4 inet_skb_parm. When skb is cloned into skb2
  and passed to icmp6_send(), it uses IP6CB(skb2).

  IP6CB interprets the IPv4 inet_skb_parm as an inet6_skb_parm. The cipso
  offset in inet_skb_parm.opt directly overlaps with dsthao in inet6_skb_parm
  at offset 18.

  If an attacker sends a forged ICMPv4 error with a CIPSO IP option, dsthao
  would be a non-zero offset. Inside icmp6_send(), mip6_addr_swap() is called
  and uses ipv6_find_tlv(skb, opt->dsthao, IPV6_TLV_HAO).

  This would scan the inner, attacker-controlled IPv6 packet starting at that
  offset, potentially returning a fake TLV without checking if the remaining
  packet length can hold the full 18-byte struct ipv6_destopt_hao.

  Could mip6_addr_swap() then perform a 16-byte swap that extends past the end
  of the packet data into skb_shared_info?

  Should the cb array also be cleared in ip6_err_gen_icmpv6_unreach() and
  ip6ip6_err() to prevent this?

This patch implements the first suggestion.

I am not sure if ip6ip6_err() needs to be changed.
A separate patch would be better anyway.

Fixes: ca15a078bd90 ("sit: generate icmpv6 error when receiving icmpv4 error")
Reported-by: Ido Schimmel <idosch@nvidia.com>
Closes: https://sashiko.dev/#/patchset/20260326155138.2429480-1-edumazet%40google.com
Signed-off-by: Eric Dumazet <edumazet@google.com>
Cc: Ido Schimmel <idosch@nvidia.com>
Cc: Oskar Kjos <oskar.kjos@hotmail.com>
---
 net/ipv6/icmp.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index 813d2e9edb8bed7c1649e279cea9229806af4132..d5d23a9296eac86a4a7ae8ea0240314653170035 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -875,6 +875,9 @@ int ip6_err_gen_icmpv6_unreach(struct sk_buff *skb, int nhs, int type,
 	if (!skb2)
 		return 1;
 
+	/* Remove debris left by IPv4 stack. */
+	memset(IP6CB(skb2), 0, sizeof(*IP6CB(skb2)));
+
 	skb_dst_drop(skb2);
 	skb_pull(skb2, nhs);
 	skb_reset_network_header(skb2);
-- 
2.53.0.1018.g2bb0e51243-goog


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH net] ipv6: icmp: clear skb2->cb[] in ip6_err_gen_icmpv6_unreach()
  2026-03-26 20:26 [PATCH net] ipv6: icmp: clear skb2->cb[] in ip6_err_gen_icmpv6_unreach() Eric Dumazet
@ 2026-03-27 17:54 ` Ido Schimmel
  0 siblings, 0 replies; 2+ messages in thread
From: Ido Schimmel @ 2026-03-27 17:54 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Jakub Kicinski, Paolo Abeni, Simon Horman,
	David Ahern, netdev, eric.dumazet, Oskar Kjos

On Thu, Mar 26, 2026 at 08:26:08PM +0000, Eric Dumazet wrote:
> Sashiko AI-review observed:
> 
>   In ip6_err_gen_icmpv6_unreach(), the skb is an outer IPv4 ICMP error packet
>   where its cb contains an IPv4 inet_skb_parm. When skb is cloned into skb2
>   and passed to icmp6_send(), it uses IP6CB(skb2).
> 
>   IP6CB interprets the IPv4 inet_skb_parm as an inet6_skb_parm. The cipso
>   offset in inet_skb_parm.opt directly overlaps with dsthao in inet6_skb_parm
>   at offset 18.
> 
>   If an attacker sends a forged ICMPv4 error with a CIPSO IP option, dsthao
>   would be a non-zero offset. Inside icmp6_send(), mip6_addr_swap() is called
>   and uses ipv6_find_tlv(skb, opt->dsthao, IPV6_TLV_HAO).
> 
>   This would scan the inner, attacker-controlled IPv6 packet starting at that
>   offset, potentially returning a fake TLV without checking if the remaining
>   packet length can hold the full 18-byte struct ipv6_destopt_hao.
> 
>   Could mip6_addr_swap() then perform a 16-byte swap that extends past the end
>   of the packet data into skb_shared_info?
> 
>   Should the cb array also be cleared in ip6_err_gen_icmpv6_unreach() and
>   ip6ip6_err() to prevent this?
> 
> This patch implements the first suggestion.
> 
> I am not sure if ip6ip6_err() needs to be changed.
> A separate patch would be better anyway.
> 
> Fixes: ca15a078bd90 ("sit: generate icmpv6 error when receiving icmpv4 error")
> Reported-by: Ido Schimmel <idosch@nvidia.com>
> Closes: https://sashiko.dev/#/patchset/20260326155138.2429480-1-edumazet%40google.com
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Cc: Ido Schimmel <idosch@nvidia.com>
> Cc: Oskar Kjos <oskar.kjos@hotmail.com>

Reviewed-by: Ido Schimmel <idosch@nvidia.com>

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-03-27 17:54 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-03-26 20:26 [PATCH net] ipv6: icmp: clear skb2->cb[] in ip6_err_gen_icmpv6_unreach() Eric Dumazet
2026-03-27 17:54 ` Ido Schimmel

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox