[PATCH] ipv6: icmp: use skb_dst_dev_net_rcu() for echo reply and unreach

[PATCH] ipv6: icmp: use skb_dst_dev_net_rcu() for echo reply and unreach

[Haoran Wang]

ip6_err_gen_icmpv6_unreach() and icmpv6_echo_reply() derive the network
namespace from the ingress device with dev_net_rcu(skb->dev).

    /* ip6_err_gen_icmpv6_unreach, net/ipv6/icmp.c:885 */
    rt = rt6_lookup(dev_net_rcu(skb->dev), &ipv6_hdr(skb2)->saddr, ...);

    /* icmpv6_echo_reply, net/ipv6/icmp.c:923 */
    struct net *net = dev_net_rcu(skb->dev);

Under a VRF, skb->dev is the VRF master device, so dev_net_rcu(skb->dev)
returns the netns of the master rather than the netns the packet is
destined to.  The route lookup for the unreach and the send policy for
the echo reply (ratelimit, IPsec, route) then run against the wrong
namespace.  The rt6_lookup may miss the route to the source, leaving
skb2->dev unset, and the echo reply is emitted/routed per the ingress
netns instead of the destination netns.

The IPv4 ICMP echo path already uses the destination netns via
skb_dst_dev_net_rcu(skb) (net/ipv4/icmp.c:1123 and :1267), and the IPv6
echo path still uses dev_net_rcu(skb->dev).

Use skb_dst_dev_net_rcu(skb) at both IPv6 sites so the namespace is
taken from the skb destination, matching the IPv4 path.

Signed-off-by: Haoran Wang <haoranwangsec@gmail.com>
---
 net/ipv6/icmp.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
index efb23807a..de4f39ca0 100644
--- a/net/ipv6/icmp.c
+++ b/net/ipv6/icmp.c
@@ -882,7 +882,7 @@ int ip6_err_gen_icmpv6_unreach(struct sk_buff *skb, int nhs, int type,
 	skb_pull(skb2, nhs);
 	skb_reset_network_header(skb2);
 
-	rt = rt6_lookup(dev_net_rcu(skb->dev), &ipv6_hdr(skb2)->saddr,
+	rt = rt6_lookup(skb_dst_dev_net_rcu(skb), &ipv6_hdr(skb2)->saddr,
 			NULL, 0, skb, 0);
 
 	if (rt && rt->dst.dev)
@@ -920,7 +920,7 @@ EXPORT_SYMBOL(ip6_err_gen_icmpv6_unreach);
 
 static enum skb_drop_reason icmpv6_echo_reply(struct sk_buff *skb)
 {
-	struct net *net = dev_net_rcu(skb->dev);
+	struct net *net = skb_dst_dev_net_rcu(skb);
 	struct sock *sk;
 	struct inet6_dev *idev;
 	struct ipv6_pinfo *np;

Re: [PATCH] ipv6: icmp: use skb_dst_dev_net_rcu() for echo reply and unreach

[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:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - 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] ipv6: icmp: use skb_dst_dev_net_rcu() for echo reply and unreach

[Ido Schimmel]

On Mon, Oct 19, 2026 at 09:22:13PM +0800, Haoran Wang wrote:
> ip6_err_gen_icmpv6_unreach() and icmpv6_echo_reply() derive the network
> namespace from the ingress device with dev_net_rcu(skb->dev).
> 
>     /* ip6_err_gen_icmpv6_unreach, net/ipv6/icmp.c:885 */
>     rt = rt6_lookup(dev_net_rcu(skb->dev), &ipv6_hdr(skb2)->saddr, ...);
> 
>     /* icmpv6_echo_reply, net/ipv6/icmp.c:923 */
>     struct net *net = dev_net_rcu(skb->dev);
> 
> Under a VRF, skb->dev is the VRF master device, so dev_net_rcu(skb->dev)
> returns the netns of the master rather than the netns the packet is
> destined to.  The route lookup for the unreach and the send policy for
> the echo reply (ratelimit, IPsec, route) then run against the wrong
> namespace.  The rt6_lookup may miss the route to the source, leaving
> skb2->dev unset, and the echo reply is emitted/routed per the ingress
> netns instead of the destination netns.
> 
> The IPv4 ICMP echo path already uses the destination netns via
> skb_dst_dev_net_rcu(skb) (net/ipv4/icmp.c:1123 and :1267), and the IPv6
> echo path still uses dev_net_rcu(skb->dev).
> 
> Use skb_dst_dev_net_rcu(skb) at both IPv6 sites so the namespace is
> taken from the skb destination, matching the IPv4 path.

I don't understand any of this. VRF master and slave are always in the
same netns. Same comment about the IPv4 patch.

Re: [PATCH] ipv6: icmp: use skb_dst_dev_net_rcu() for echo reply and unreach

[netdev-bot+sashiko]

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

Critical: 0 · High: 1 · Medium: 2 · Low: 0 (2)

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] The patch now takes the netns in icmpv6_echo_reply() and
  ip6_err_gen_icmpv6_unreach() from skb_dst(skb)->dev_rcu (via…
- [Medium] The commit message's premise is wrong.
- [Medium] The netns conversion is partial, so the changes are internally
  inconsistent.

Pre-existing issues:
- [Low] This problem wasn't introduced by this patch, but it is on the
  line the patch rewrites.
- [Low] This problem wasn't introduced by this patch, but the comment
  above ip6_err_gen_icmpv6_unreach() (net/ipv6/icmp.c:852-857) says the…

--

ipv6: icmp: use skb_dst_dev_net_rcu() for echo reply and unreach

This patch changes how ip6_err_gen_icmpv6_unreach() and
icmpv6_echo_reply() pick the network namespace. They now use
skb_dst_dev_net_rcu(skb) instead of dev_net_rcu(skb->dev). The commit
message says that under a VRF the ingress device netns differs from the
destination netns, and that the IPv4 echo path already uses the dst
netns.

> Under a VRF, skb->dev is the VRF master device, so dev_net_rcu(skb->dev)
> returns the netns of the master rather than the netns the packet is
> destined to.  The route lookup for the unreach and the send policy for
> the echo reply (ratelimit, IPsec, route) then run against the wrong
> namespace.  The rt6_lookup may miss the route to the source, leaving
> skb2->dev unset, and the echo reply is emitted/routed per the ingress
> netns instead of the destination netns.

[Severity: Medium]
Can a VRF master actually be in a different netns from the packet's
destination?

do_set_master() looks the master up in the slave's own netns:

net/core/rtnetlink.c:do_set_master() {
    ...
	upper_dev = __dev_get_by_index(dev_net(dev), ifindex);
    ...
}

VRF devices can't change netns:

drivers/net/vrf.c:vrf_setup() {
    ...
	dev->netns_immutable = true;
    ...
}

A slave that moves to another netns gets NETDEV_UNREGISTER, and
vrf_device_event() removes it from the VRF. vrf_ip6_input_dst() also
does its lookup in dev_net(vrf_dev). Packets that cross netns go through
skb_scrub_packet(), which drops the dst.

So on live receive paths, skb->dev and skb_dst(skb)->dev seem to be in
the same netns, and both expressions return the same struct net.

If rt6_lookup() really misses the VRF route, wouldn't that be because it
is called with oif=0 and no l3mdev? Then the l3mdev FIB rule can't pick
the VRF table, and this patch doesn't change those arguments.

Is there a reproducer for the VRF case described here? There is no
Fixes: tag either. Could the description be reworked so the patch isn't
picked for stable based on this scenario?

> diff --git a/net/ipv6/icmp.c b/net/ipv6/icmp.c
> index a95b0351824f..5bdfbb656301 100644
> --- a/net/ipv6/icmp.c
> +++ b/net/ipv6/icmp.c
> @@ -882,7 +882,7 @@ int ip6_err_gen_icmpv6_unreach(struct sk_buff *skb, int nhs, int type,
>  	skb_pull(skb2, nhs);
>  	skb_reset_network_header(skb2);
>  
> -	rt = rt6_lookup(dev_net_rcu(skb->dev), &ipv6_hdr(skb2)->saddr,
> +	rt = rt6_lookup(skb_dst_dev_net_rcu(skb), &ipv6_hdr(skb2)->saddr,
>  			NULL, 0, skb, 0);

[Severity: Low]
This isn't a bug introduced by this patch, but this line is being
rewritten anyway. The skb passed to rt6_lookup() here is the original
IPv4 ICMP skb. On multipath routes the lookup goes:

rt6_lookup()
  fib6_rule_lookup()
    ip6_pol_route_lookup()
      fib6_select_path()
        rt6_multipath_hash()
          ip6_multipath_l3_keys(skb, ...)
            outer_iph = ipv6_hdr(skb);

This reads the outer IPv4 header as an IPv6 header to build the ECMP
hash keys. The pskb_may_pull() at the top keeps the read inside the
linear area. So the only effect is on which nexthop device ends up in
skb2->dev.

Would it be better to pass skb2 or NULL here?

[Severity: Low]
This is also a pre-existing issue, not one introduced by this patch. The
comment above ip6_err_gen_icmpv6_unreach() only mentions type/code
ICMPV6_DEST_UNREACH/ICMPV6_ADDR_UNREACH. The function also sends
ICMPV6_TIME_EXCEED/ICMPV6_EXC_HOPLIMIT, and the SIT and GRE callers do
pass ICMP_TIME_EXCEEDED:

	if (type == ICMP_TIME_EXCEEDED)
		icmp6_send(skb2, ICMPV6_TIME_EXCEED, ICMPV6_EXC_HOPLIMIT,
			   info, &temp_saddr, IP6CB(skb2));

Should that comment be updated?

>  
>  	if (rt && rt->dst.dev)
> @@ -920,7 +920,7 @@ EXPORT_SYMBOL(ip6_err_gen_icmpv6_unreach);
>  
>  static enum skb_drop_reason icmpv6_echo_reply(struct sk_buff *skb)
>  {
> -	struct net *net = dev_net_rcu(skb->dev);
> +	struct net *net = skb_dst_dev_net_rcu(skb);

[Severity: High]
Can net end up as &init_net here if the dst has been invalidated?

skb->dev is pinned while the packet is in flight, but dst->dev_rcu can
change. When a route is torn down, dst_dev_put() repoints it:

net/core/dst.c:dst_dev_put() {
    ...
	rcu_assign_pointer(dst->dev_rcu, blackhole_netdev);
    ...
}

blackhole_netdev is never registered, so its netns stays &init_net.

Here is one path in netns X:

  An echo request to a local address is queued by NFQUEUE at
  NF_INET_LOCAL_IN. __nf_queue() calls skb_dst_force() and dev_hold()s
  skb->dev.

  The local address or route is deleted. After the grace period,
  fib6_info_destroy_rcu()->fib6_nh_release()->fib6_nh_release_dsts()
  calls dst_dev_put() on the pcpu rt6_info that the queued skb still
  holds. nh_rt_cache_flush() also reaches fib6_nh_release_dsts() when an
  IPv6 nexthop group is replaced.

  On NF_ACCEPT, nf_reinject() calls ip6_input_finish() directly.
  nf_ip6_reroute() only does anything for NF_INET_LOCAL_OUT, so the dead
  dst is not checked again.

  icmpv6_rcv() still uses dev_net_rcu(skb->dev), which is X, so the
  echo_ignore_all check passes. icmpv6_echo_reply() then sets
  net = &init_net.

After that, icmpv6_xmit_lock() sets the shared per-cpu ICMPv6 socket's
netns to init_net. The sysctls, IP6_REPLY_MARK, ratelimits,
ip6_dst_lookup() and xfrm_lookup() all run in init_net.

fl6.saddr is X's address, and flowi6_oif is an ifindex from X that gets
resolved in init_net. The reply carries the requester's payload and is
routed and sent from the host namespace.

The unreach hunk above looks affected the same way. The IPv4 input dst
of a queued ICMP error can go through dst_dev_put() via
rt_fibinfo_free()/rt_fibinfo_free_cpus(). ipip6_err() and ipgre_err()
still find the tunnel in dev_net(skb->dev), but rt6_lookup() would then
run in init_net. skb2->dev can then become an init_net device, and
icmp6_send() also runs in init_net.

All of this needs only CAP_NET_ADMIN in X, which an unprivileged user can
get through a user namespace. A route teardown running at the same time
(nh_rt_cache_flush(), rt6_remove_exception()) can also open the window
without nfqueue.

Before this patch, the netns came from skb->dev. That device is pinned
and can't change netns while the packet is queued. Isn't that the more
reliable source for the receiving netns?

The IPv4 sites cited in the commit message seem to have the same
weakness, so matching them doesn't make this change safe.

[Severity: Medium]
The commit message says this now matches the IPv4 path. Is the
conversion complete, though? IPv4 icmp_rcv() takes net from the dst for
the whole receive decision:

net/ipv4/icmp.c:icmp_rcv() {
    ...
	struct net *net = dev_net_rcu(rt->dst.dev);
    ...
}

IPv6 icmpv6_rcv() still uses the ingress device:

net/ipv6/icmp.c:icmpv6_rcv() {
	...
	struct net *net = dev_net_rcu(skb->dev);
	...
	case ICMPV6_ECHO_REQUEST:
		if (!net->ipv6.sysctl.icmpv6_echo_ignore_all)
			reason = icmpv6_echo_reply(skb);
	...
}

That net is used for xfrm6_policy_check() and for the echo_ignore_all
and sysctl_icmp_echo_enable_probe checks. Those checks decide whether
icmpv6_echo_reply() runs at all.

Inside icmpv6_echo_reply(), idev and flowi6_oif still come from
skb->dev. The shared icmp_build_probe() also still uses
dev_net_rcu(skb->dev).

On the unreach path, ipip6_err() and ipgre_err() find the tunnel in
dev_net(skb->dev), rt6_lookup() now uses the dst netns, and icmp6_send()
uses dev_net_rcu(skb2->dev). skb2->dev is set to the lookup's device only
when rt6_lookup() succeeds. So the netns used for sending depends on
whether the lookup succeeded.

The two sources can differ, for example in the invalidated dst case
above. Then a single packet is gated by X's echo_ignore_all, probe and
xfrm settings but answered under init_net's policy.

Shouldn't all of these sites use the same netns source?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/179241613335.5916.18191480036849517944%40whr