[PATCH v1 net 0/3] ip: Fix route leak and unregister_netdevice() stuck.

[PATCH v1 net 0/3] ip: Fix route leak and unregister_netdevice() stuck.

[Kuniyuki Iwashima]

Jason Zhang fed commit 0346ec2f080b ("ipv6: Prevent
rt6_insert_exception() for dying fib6_info.") to LLM to
find tricks to cause the same symptom:

  unregister_netdevice: waiting for gre6 to become free. Usage count = 4
  ref_tracker: netdev@000000009652f190 has 1/3 users at
       dst_alloc+0x14b/0x490
       ip6_rt_cache_alloc+0x27b/0x670
       __ip6_rt_update_pmtu+0x572/0x850

This series treats them.


Kuniyuki Iwashima (3):
  ipv6: Flush exception route while replacing nexthop.
  ipv6: Flush rt->from in rt6_uncached_list_flush_dev().
  ipv4: Call rt_add_uncached_list() if !do_cache in
    ip_route_input_slow().

 net/ipv4/route.c |  2 ++
 net/ipv6/route.c | 20 ++++++++++++++++++--
 2 files changed, 20 insertions(+), 2 deletions(-)

-- 
2.56.0.317.g454f8302e1-goog

[PATCH v1 net 1/3] ipv6: Flush exception route while replacing nexthop.

[Kuniyuki Iwashima]

The following creates a cyclic reference between IPv6 route
and nexthop:

  1. create a nexthop A
  2. create a nexthop group w/ A
  3. create an IPv6 route with the nexthop group
  4. create an exception route w/ A
  5. replace A with a new nexthop B in the nexthop group
  6. remove the IPv6 route
  7. replace B with A in the nexthop group
  8. remove the nexthop group

Then, removing a device tied to the route gets stuck and
triggers the splat below. [0]

At 5., fib6_nh_release_dsts() is called, but it does not
flush exception routes, so the nexthop A continues to grab
the IPv6 route reference.

  nh_A->rt6i_exception_bucket[] -> rt6_ex->rt6i->from

At 6., fib6_purge_rt() calls rt6_flush_exceptions(), but it
only purges exceptions tied to nexthop B.  Also, the route is
unlinked from the nexthop group by list_del_init(&rt->nh_list).

  -> f6i->nh -> nhg->nh_entries[]

At 8., __remove_nexthop_fib() cannot find the IPv6 route, and
remove_nexthop_group() calls list_del_init(&nhge->nh_list) to
unlink the group from the nexthop A's nhe->grp_list.

  nhg->nh_entries[] -> nh_A -> ... -> f6i

Here, the nexthop A is reachable from net->nexthop.devhash[],
but remove_nexthop_from_groups() cannot find the nexthop group,
leaking the whole chain.

Let's call fib6_nh_flush_exceptions() in fib6_nh_release_dsts().

[0]:
unregister_netdevice: waiting for gre6 to become free. Usage count = 4
ref_tracker: netdev@000000009652f190 has 1/3 users at
     dst_alloc+0x14b/0x490
     ip6_rt_cache_alloc+0x27b/0x670
     __ip6_rt_update_pmtu+0x572/0x850
     ip6_tnl_xmit+0x8c5/0x1b70
     __gre6_xmit+0x7af/0xaa0
     ip6gre_tunnel_xmit+0x685/0xdc0
     dev_hard_start_xmit+0x204/0x5a0
     __dev_queue_xmit+0xe8a/0x2550
     ip6_finish_output+0x232/0x570
     ip6_output+0x226/0x3b0
     ip6_send_skb+0x133/0x250
     rawv6_push_pending_frames+0x503/0x6f0
     rawv6_sendmsg+0xdf0/0x1310
     sock_sendmsg_nosec+0xde/0x130
     __sys_sendto+0x27f/0x320
     __x64_sys_sendto+0xd9/0xf0

Fixes: 7bf4796dd099 ("nexthops: add support for replace")
Reported-by: Jason Zhang <jdz@google.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv6/route.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index 616b86f89f63..fae698c5522b 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -3701,9 +3701,10 @@ void fib6_nh_release(struct fib6_nh *fib6_nh)
 {
 	struct rt6_exception_bucket *bucket;
 
+	fib6_nh_release_dsts(fib6_nh);
+
 	rcu_read_lock();
 
-	fib6_nh_flush_exceptions(fib6_nh, NULL);
 	bucket = fib6_nh_get_excptn_bucket(fib6_nh, NULL);
 	if (bucket) {
 		rcu_assign_pointer(fib6_nh->rt6i_exception_bucket, NULL);
@@ -3712,7 +3713,6 @@ void fib6_nh_release(struct fib6_nh *fib6_nh)
 
 	rcu_read_unlock();
 
-	fib6_nh_release_dsts(fib6_nh);
 	free_percpu(fib6_nh->rt6i_pcpu);
 
 	fib_nh_common_release(&fib6_nh->nh_common);
@@ -3722,6 +3722,8 @@ void fib6_nh_release_dsts(struct fib6_nh *fib6_nh)
 {
 	int cpu;
 
+	fib6_nh_flush_exceptions(fib6_nh, NULL);
+
 	if (!fib6_nh->rt6i_pcpu)
 		return;
 
-- 
2.56.0.317.g454f8302e1-goog

[PATCH v1 net 2/3] ipv6: Flush rt->from in rt6_uncached_list_flush_dev().

[Kuniyuki Iwashima]

When a device is unregistered, rt6_uncached_list_flush_dev()
iterates over uncached routes and switches the dying dev
with blackhole_netdev.

However, rt->from could be cached via sk->sk_rx_dst and
hold the device reference.

Then, unregister_netdevice() gets stuck with the splat
below. [0]

Let's release rt->from in rt6_uncached_list_flush_dev().

Note that ip6_rt_copy_init() could set rt->from even if
in6_dev_get() returns NULL (e.g. mtu < IPV6_MIN_MTU is
set concurrently).

Also, rt->from->nh could be replaced with another device
than rt_dev and rt_idev->dev, and in such a case,
nexthop_flush_dev() sets fib6_destroying before
rt6_uncached_list_flush_dev().

[0]:
unregister_netdevice: waiting for dummy2 to become free. Usage count = 2
ref_tracker: netdev@000000006fb9da8c has 1/1 users at
     netdev_get_by_index+0x14c/0x170
     fib6_nh_init+0x1ae/0x1950
     ip6_route_info_create_nh+0x15d/0x780
     ip6_route_add+0x55/0x150
     inet6_rtm_newroute+0x15d/0x1360
     rtnetlink_rcv_msg+0x606/0x710
     netlink_rcv_skb+0x14f/0x2f0
     netlink_unicast+0x71b/0x940
     netlink_sendmsg+0x595/0x840
     __sock_sendmsg+0x119/0x150
     ____sys_sendmsg+0x3e9/0x5c0
     ___sys_sendmsg+0x157/0x1a0
     __x64_sys_sendmsg+0x119/0x170
     do_syscall_64+0x109/0x330
     entry_SYSCALL_64_after_hwframe+0x77/0x7f

Fixes: 93531c674315 ("net/ipv6: separate handling of FIB entries from dst based routes")
Reported-by: Jason Zhang <jdz@google.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv6/route.c | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/net/ipv6/route.c b/net/ipv6/route.c
index fae698c5522b..2eb884558c24 100644
--- a/net/ipv6/route.c
+++ b/net/ipv6/route.c
@@ -179,12 +179,20 @@ void rt6_uncached_list_flush_dev(struct net_device *dev)
 		list_for_each_entry_safe(rt, safe, &ul->head, dst.rt_uncached) {
 			struct inet6_dev *rt_idev = rt->rt6i_idev;
 			struct net_device *rt_dev = rt->dst.dev;
+			struct fib6_info *from;
+			bool release_fib;
+
+			rcu_read_lock();
+			from = rcu_dereference(rt->from);
+			release_fib = from && from->fib6_destroying;
+			rcu_read_unlock();
 
 			if (rt_idev &&
 			    (dev ? rt_idev->dev == dev :
 			     READ_ONCE(rt_idev->dev->reg_state) == NETREG_UNREGISTERED)) {
 				rt->rt6i_idev = in6_dev_get(blackhole_netdev);
 				in6_dev_put(rt_idev);
+				release_fib = true;
 			}
 
 			if (dev ? rt_dev == dev :
@@ -193,6 +201,12 @@ void rt6_uncached_list_flush_dev(struct net_device *dev)
 				netdev_ref_replace(rt_dev, blackhole_netdev,
 						   &rt->dst.dev_tracker,
 						   GFP_ATOMIC);
+				release_fib = true;
+			}
+
+			if (release_fib) {
+				from = unrcu_pointer(xchg(&rt->from, NULL));
+				fib6_info_release(from);
 			}
 
 			if (rt->dst.dev == blackhole_netdev &&
-- 
2.56.0.317.g454f8302e1-goog

[PATCH v1 net 3/3] ipv4: Call rt_add_uncached_list() if !do_cache in ip_route_input_slow().

[Kuniyuki Iwashima]

Since the cited commit, ip_route_input_slow() could call
rt_dst_alloc() with vrf in the local_input: path.

If res->fi != NULL and one of the following is true,

  - a non-zero itag from fib_validate_source_reason()
    (realms/classid)
  - RTN_BROADCAST with BC_FORWARDING enabled

do_cache is set to false.

Then, the route is not added to rt_uncached_list, so the
route cannot be removed from rt_flush_dev().

If the route is cached in sk->sk_rx_dst, removing the
vrf device gets stuck. [0]

Let's call rt_add_uncached_list() if !do_cache in
ip_route_input_slow().

[0]:
unregister_netdevice: waiting for vrf0 to become free. Usage count = 2
ref_tracker: netdev@000000008b304ac4 has 1/1 users at
     dst_alloc+0x14b/0x490
     ip_route_input_rcu+0x1540/0x2530
     ip_route_input_noref+0x5a/0x90
     ip_rcv_finish_core+0x4b0/0x1140
     ip_rcv_finish+0xe6/0x190
     ip_rcv+0x6c/0x130
     __netif_receive_skb+0x121/0x300
     process_backlog+0x2df/0x5e0
     __napi_poll+0x8c/0x380
     net_rx_action+0x43c/0xab0
     handle_softirqs+0x19f/0x520
     do_softirq+0x46/0x60
     __local_bh_enable_ip+0x79/0x80
     __dev_queue_xmit+0x1b0e/0x24c0
     ip_finish_output2+0x8b2/0xcd0
     ip_output+0xf2/0x1e0

Fixes: f5a0aab84b74 ("net: ipv4: dst for local input routes should use l3mdev if relevant")
Reported-by: Jason Zhang <jdz@google.com>
Signed-off-by: Kuniyuki Iwashima <kuniyu@google.com>
---
 net/ipv4/route.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 93b9f4189525..3a90c8d28b94 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -2494,6 +2494,8 @@ ip_route_input_slow(struct sk_buff *skb, __be32 daddr, __be32 saddr,
 
 		if (unlikely(!rt_cache_route(nhc, rth)))
 			rt_add_uncached_list(rth);
+	} else {
+		rt_add_uncached_list(rth);
 	}
 	skb_dst_set(skb, &rth->dst);
 	reason = SKB_NOT_DROPPED_YET;
-- 
2.56.0.317.g454f8302e1-goog