[PATCH net] ipv4: route: don't blackhole in-use routes when retiring fnhe routes

[Chase Hiltz]

When a route cached in a nexthop exception (fnhe) is replaced or
flushed, rt_bind_exception() and fnhe_flush_routes() call
dst_dev_put() on it. That sets dst->dev to blackhole_netdev and
dst->input/output to dst_discard*(), but other CPUs can still be
using the route, e.g. as a noref dst taken from the fnhe under RCU.
Packets that reach dst_input() or dst_output() after that are dropped.

If dst_dev_put() runs after a forwarded packet has entered ip_output(),
ip_skb_dst_mtu() in __ip_finish_output() can return the MTU of
blackhole_netdev (68), and ip_fragment() can then drop a DF packet and
send ICMP_FRAG_NEEDED with mtu 68 to its source.

This was hit on a 6.12 router that sent ICMP_FRAG_NEEDED with mtu 68
for packets it forwarded. A kprobe on __icmp_send() showed that the
skb's dst had dev == blackhole_netdev and still had the PMTU from its
fnhe.

Commit 5018c59607a5 ("ipv4: fix race condition between route lookup and
invalidation") fixed the same race in rt_cache_route() by putting the
old route on the uncached list instead of calling dst_dev_put(). Do the
same for routes retired from an fnhe. rt_flush_dev() still moves them
to blackhole_netdev when their device is unregistered.

fnhe routes are also retired on fnhe expiry (ip_del_fnhe()) and
eviction (fnhe_remove_oldest()). Those paths don't bump rt_genid, so
without the obsolete marking ipv4_dst_check() could still accept the
route. Keep marking retired routes DST_OBSOLETE_DEAD, as dst_dev_put()
did, so that users that cached them, such as sockets, look up again.

Tested on v7.4-rc1 with network namespaces: a router forwards IPv4 TCP
(iperf3 -P 8) over an ip6tnl device whose PMTU update creates an fnhe
for the destination, while a loop adds and deletes an IPv4 route. The
sender drops incoming ICMP_FRAG_NEEDED, so it keeps setting DF.

Over 60s without this patch, the sender received 505,291
ICMP_FRAG_NEEDED with mtu 68, and the router dropped 67,024 packets in
dst_discard_out() and 599 in dst_discard(). With this patch there were
none. Neither kernel sent any without the route churn, or without an
fnhe (ip6tnl with encaplimit none).

When the sender accepted the ICMPs, its route to the destination ended
up with "mtu lock 552" without this patch. With it, the route had no
PMTU.

Deleting the tunnel, its underlay or the peer netns under the same load
(12 runs) completed without unregister hangs or warnings, with and
without this patch.

The same tests on a kernel with PROVE_LOCKING, KASAN, DEBUG_LIST,
DEBUG_ATOMIC_SLEEP and the netdev/netns refcount trackers produced no
reports. Lockdep recorded the new fnhe_lock -> uncached list lock
dependency, and in a separate 20s run 109,305 routes were retired
from an fnhe through the new path.

Fixes: 95c47f9cf5e0 ("ipv4: call dst_dev_put() properly")
Signed-off-by: Chase Hiltz <me@xxxxxxxxxxx>
Co-authored-by: Claude Opus 5.5 <noreply@xxxxxxxxxxxxx>
---
 net/ipv4/route.c | 23 +++++++++++++++--------
 1 file changed, 15 insertions(+), 8 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 3a90c8d..cd69242 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -573,6 +573,17 @@ static void ip_rt_build_flow_key(struct flowi4 *fl4, const struct sock *sk,
 
 static DEFINE_SPINLOCK(fnhe_lock);
 
+/* Other CPUs may still be using @rt, so don't dst_dev_put() it.
+ * rt_flush_dev() handles it if its device is unregistered.
+ * Marking it dead makes ipv4_dst_check() fail for users that cached it.
+ */
+static void fnhe_retire_route(struct rtable *rt)
+{
+	WRITE_ONCE(rt->dst.obsolete, DST_OBSOLETE_DEAD);
+	rt_add_uncached_list(rt);
+	dst_release(&rt->dst);
+}
+
 static void fnhe_flush_routes(struct fib_nh_exception *fnhe)
 {
 	struct rtable *rt;
@@ -580,14 +591,12 @@ static void fnhe_flush_routes(struct fib_nh_exception *fnhe)
 	rt = rcu_dereference(fnhe->fnhe_rth_input);
 	if (rt) {
 		RCU_INIT_POINTER(fnhe->fnhe_rth_input, NULL);
-		dst_dev_put(&rt->dst);
-		dst_release(&rt->dst);
+		fnhe_retire_route(rt);
 	}
 	rt = rcu_dereference(fnhe->fnhe_rth_output);
 	if (rt) {
 		RCU_INIT_POINTER(fnhe->fnhe_rth_output, NULL);
-		dst_dev_put(&rt->dst);
-		dst_release(&rt->dst);
+		fnhe_retire_route(rt);
 	}
 }
 
@@ -1505,10 +1514,8 @@ static bool rt_bind_exception(struct rtable *rt, struct fib_nh_exception *fnhe,
 		if (do_cache) {
 			dst_hold(&rt->dst);
 			rcu_assign_pointer(*porig, rt);
-			if (orig) {
-				dst_dev_put(&orig->dst);
-				dst_release(&orig->dst);
-			}
+			if (orig)
+				fnhe_retire_route(orig);
 			ret = true;
 		}
 
-- 
2.56.0