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

[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@chasehil.tz>
Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
---
 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

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

[Eric Dumazet]

Le mer. 4 nov. 2026 à 19:21, Chase Hiltz <me@chasehil.tz> a écrit :
>
> 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@chasehil.tz>
> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
> ---

Co-authored-by: is not a tag we use (checkpatch complains about it),
please use Assisted-by: instead.

The changelog is also quite long, you could move some of the testing
details below the --- line.

Otherwise this looks good to me, thanks.

fnhe routes are never on the uncached list while they are cached,
and the fnhe slot is cleared under fnhe_lock before
fnhe_retire_route() is called, so there is no risk of a double
list_add(). rt_flush_dev() takes care of the device reference.

Note that IPv6 has a similar issue in rt6_remove_exception().

Reviewed-by: Eric Dumazet <edumazet@kernel.org>

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

[Chase Hiltz]

On Wed, Nov 04, 2026 at 07:54:23PM +0100, Eric Dumazet wrote:
> Co-authored-by: is not a tag we use (checkpatch complains about it),
> please use Assisted-by: instead.
>
> The changelog is also quite long, you could move some of the testing
> details below the --- line.

Thanks for the review, Eric. I've addressed your comments in v2.

> Note that IPv6 has a similar issue in rt6_remove_exception().

I haven't looked at the IPv6 side, but I'd be happy to check it out
and submit a separate patch for it if nobody beats me to it.

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

[netdev-bot+sashiko]

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

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

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).

- [Medium] One fnhe retirement path still calls dst_dev_put() on routes
  that may be in use, and it comes from the same Fixes target.
- [Low] The commit ends with a "Co-authored-by: Claude Opus 5.5
  <noreply@anthropic.com>" trailer.

--

ipv4: route: don't blackhole in-use routes when retiring fnhe routes

The patch stops calling dst_dev_put() on routes retired from a nexthop
exception in rt_bind_exception() and fnhe_flush_routes(). The new
fnhe_retire_route() helper instead marks the route DST_OBSOLETE_DEAD,
puts it on the uncached list and drops the reference, so CPUs still
using the route are not switched to blackhole_netdev.

> Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>

[Severity: Low]
This isn't a bug, but checkpatch.pl doesn't recognise the Co-authored-by
tag. It isn't in $signature_tags, so checkpatch warns about it with
BAD_SIGN_OFF "Non-standard signature".

Documentation/process/coding-assistants.rst says an AI-assisted fix "must
pass the checkpatch.pl checks". Documentation/process/generated-content.rst
asks for tool use to be disclosed in the changelog or cover letter, not in
a custom trailer.

Switching to Co-developed-by would not fix this. checkpatch requires a
matching Signed-off-by right after Co-developed-by, and
coding-assistants.rst does not allow AI agents to add a Signed-off-by.

Could the trailer be removed and the tool use disclosed in the changelog
text instead?

> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index 3a90c8d28b940..cd69242afab07 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);
> +}

[Severity: Medium]
Is there one more fnhe retirement path that still blackholes in-use
routes? Commit 95c47f9cf5e0 ("ipv4: call dst_dev_put() properly"), the
Fixes target, also added dst_dev_put() to rt_fibinfo_free() in
net/ipv4/fib_semantics.c. That function is used on fnhe routes when the
owning fib_info or nexthop is freed:

free_fib_info()
  call_rcu_hurry(free_fib_info_rcu)
    fib_nh_release()
      fib_nh_common_release()
        free_nh_exceptions()
          rt_fibinfo_free(&fnhe->fnhe_rth_input)
          rt_fibinfo_free(&fnhe->fnhe_rth_output)

net/ipv4/fib_semantics.c:rt_fibinfo_free() {
    ...
	dst_dev_put(&rt->dst);
	dst_release_immediate(&rt->dst);
}

The RCU grace period before free_fib_info_rcu() covers noref users.
However, __mkroute_output() hands out fnhe_rth_output with a reference:

	rth = rcu_dereference(*prth);
	if (rt_cache_valid(rth) && dst_hold_safe(&rth->dst))
		return rth;

Also, __nf_queue() calls skb_dst_force() on queued skbs. Both of those
holders can outlive the grace period.

For example, a UDP sender could get the dst and then sleep copying user
data in ip_make_skb() while the route is deleted. Its later
ip_send_skb()->dst_output() would then go to dst_discard_out().

Another case is a forwarded skb queued to NFQUEUE at POSTROUTING and
reinjected after rt_fibinfo_free(). It goes through
ip_finish_output()->__ip_finish_output()->ip_skb_dst_mtu(), sees the
blackhole_netdev MTU of 68, and ip_fragment() can send ICMP_FRAG_NEEDED
with mtu 68 to the remote source. That is the symptom described in the
commit message.

The commit message says "Do the same for routes retired from an fnhe",
and the comment above says not to dst_dev_put() routes other CPUs may
still be using. Could rt_fibinfo_free() handle fnhe routes the same way:
mark them DST_OBSOLETE_DEAD, add them with rt_add_uncached_list() and
drop the reference?

Commit 5018c59607a5 also left the matching dst_dev_put() calls in
rt_fibinfo_free_cpus() and for nhc_rth_input, which affect non-fnhe
cached routes.

> +
>  static void fnhe_flush_routes(struct fib_nh_exception *fnhe)
>  {
>  	struct rtable *rt;

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261104182058.2673882-1-me%40chasehil.tz