All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH net] net: do not send ICMP/NDISC Redirects when peer allocation fails
@ 2026-07-24  7:29 Eric Dumazet
  2026-07-24 13:42 ` Jakub Kicinski
  0 siblings, 1 reply; 4+ messages in thread
From: Eric Dumazet @ 2026-07-24  7:29 UTC (permalink / raw)
  To: David S . Miller, Jakub Kicinski, Paolo Abeni
  Cc: Simon Horman, Ido Schimmel, David Ahern, netdev, eric.dumazet,
	Eric Dumazet

When inet_getpeer_v4() or inet_getpeer_v6() fails to allocate a peer entry
under memory pressure or tree size caps, redirect handlers previously fell
back to sending un-rate-limited ICMP/NDISC Redirect messages.

In IPv4, ip_rt_send_redirect() called icmp_send() directly when peer == NULL.
In IPv6, ip6_forward() and ndisc_send_redirect() passed a NULL peer into
inet_peer_xrlim_allow(), which returned true when peer == NULL.

Because ICMP/NDISC Redirects are not part of the default global rate limit
mask (sysctl_icmp_ratemask), sending redirects when peer == NULL creates
an un-rate-limited ICMP packet storm.

Fix this by failing closed in ip_rt_send_redirect(), ip6_forward(), and
ndisc_send_redirect() when peer is NULL.

Fixes: 92d868292634 ("inetpeer: Move ICMP rate limiting state into inet_peer entries.")
Signed-off-by: Eric Dumazet <edumazet@google.com>
---
 net/ipv4/route.c      | 2 --
 net/ipv6/ip6_output.c | 2 +-
 net/ipv6/ndisc.c      | 2 ++
 3 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 3f3de5164d6e5854cae3ebe6fcecbac10fb63418..152d8cb28f65aacee378521e16885c9cf1a4870e 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -892,8 +892,6 @@ void ip_rt_send_redirect(struct sk_buff *skb)
 	peer = inet_getpeer_v4(net->ipv4.peers, ip_hdr(skb)->saddr, vif);
 	if (!peer) {
 		rcu_read_unlock();
-		icmp_send(skb, ICMP_REDIRECT, ICMP_REDIR_HOST,
-			  rt_nexthop(rt, ip_hdr(skb)->daddr));
 		return;
 	}
 
diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 368e4fa3b43ca2a96f53fa4f3bc14fd6832c346f..2c44e5ed617167ce060c265052cd40449fdab69f 100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -641,7 +641,7 @@ int ip6_forward(struct sk_buff *skb)
 		/* Limit redirects both by destination (here)
 		   and by source (inside ndisc_send_redirect)
 		 */
-		if (inet_peer_xrlim_allow(peer, 1*HZ))
+		if (peer && inet_peer_xrlim_allow(peer, 1*HZ))
 			ndisc_send_redirect(skb, target);
 		rcu_read_unlock();
 	} else {
diff --git a/net/ipv6/ndisc.c b/net/ipv6/ndisc.c
index f867ec8d3d90510c1ed92012c3b34ab8b49c9ac7..fe36b3f512850369e138b6a99ae7a9391494c2f1 100644
--- a/net/ipv6/ndisc.c
+++ b/net/ipv6/ndisc.c
@@ -1707,6 +1707,8 @@ void ndisc_send_redirect(struct sk_buff *skb, const struct in6_addr *target)
 	}
 
 	peer = inet_getpeer_v6(net->ipv6.peers, &ipv6_hdr(skb)->saddr);
+	if (!peer)
+		goto release;
 	ret = inet_peer_xrlim_allow(peer, 1*HZ);
 
 	if (!ret)
-- 
2.55.0.229.g6434b31f56-goog


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

* Re: [PATCH net] net: do not send ICMP/NDISC Redirects when peer allocation fails
  2026-07-24  7:29 [PATCH net] net: do not send ICMP/NDISC Redirects when peer allocation fails Eric Dumazet
@ 2026-07-24 13:42 ` Jakub Kicinski
  2026-07-24 14:15   ` Eric Dumazet
  0 siblings, 1 reply; 4+ messages in thread
From: Jakub Kicinski @ 2026-07-24 13:42 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Paolo Abeni, Simon Horman, Ido Schimmel,
	David Ahern, netdev, eric.dumazet

On Fri, 24 Jul 2026 07:29:01 +0000 Eric Dumazet wrote:
> When inet_getpeer_v4() or inet_getpeer_v6() fails to allocate a peer entry
> under memory pressure or tree size caps, redirect handlers previously fell
> back to sending un-rate-limited ICMP/NDISC Redirect messages.
> 
> In IPv4, ip_rt_send_redirect() called icmp_send() directly when peer == NULL.
> In IPv6, ip6_forward() and ndisc_send_redirect() passed a NULL peer into
> inet_peer_xrlim_allow(), which returned true when peer == NULL.
> 
> Because ICMP/NDISC Redirects are not part of the default global rate limit
> mask (sysctl_icmp_ratemask), sending redirects when peer == NULL creates
> an un-rate-limited ICMP packet storm.
> 
> Fix this by failing closed in ip_rt_send_redirect(), ip6_forward(), and
> ndisc_send_redirect() when peer is NULL.

Bot says:

The following selftest regression was observed after applying this patch:

  Test:   tools/testing/selftests/net/fib_tests.sh
  Result: FAIL (reproduced on retry)
  Runner: vmksft-net (x86-64, kernel selftests VM)

The test passes all sections up through the IPv6 route garbage-collection
tests and then fails with:

  Error while performing Neighbor Discovery for the Destination Address
  Error while learning Source Address and Next Hop
  not ok 1 selftests: net: fib_tests.sh # exit=1

This happens during the multipath balance/list tests that follow fib6_gc_test.
Those tests probe IPv6 multipath routes using `ip route get`, which requires
NDP to resolve nexthop addresses through a forwarding path.

The new early-return in ip6_forward() when inet_getpeer_v6() returns NULL
(peer allocation failure, added by this patch) can drop IPv6 packets that
would previously have been forwarded (and a redirect sent).  In the test
environment, freshly-created network namespaces may encounter peer-tree
allocation pressure after the many namespace allocations in earlier test
sections, causing this path to be exercised unexpectedly and silently
dropping NDP probes, which causes `ip route get` to return EHOSTUNREACH.

Could you investigate whether the early-return in ip6_forward() for the
NULL-peer case is too aggressive?  In particular, if the goal is only to
suppress the redirect (which is correct), dropping the packet entirely on
a NULL peer may be undesirable — the right approach might be to continue
forwarding the packet but skip the redirect logic when peer is NULL.

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

* Re: [PATCH net] net: do not send ICMP/NDISC Redirects when peer allocation fails
  2026-07-24 13:42 ` Jakub Kicinski
@ 2026-07-24 14:15   ` Eric Dumazet
  2026-07-24 14:20     ` Jakub Kicinski
  0 siblings, 1 reply; 4+ messages in thread
From: Eric Dumazet @ 2026-07-24 14:15 UTC (permalink / raw)
  To: Jakub Kicinski
  Cc: David S . Miller, Paolo Abeni, Simon Horman, Ido Schimmel,
	David Ahern, netdev, eric.dumazet

On Fri, Jul 24, 2026 at 3:42 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Fri, 24 Jul 2026 07:29:01 +0000 Eric Dumazet wrote:
> > When inet_getpeer_v4() or inet_getpeer_v6() fails to allocate a peer entry
> > under memory pressure or tree size caps, redirect handlers previously fell
> > back to sending un-rate-limited ICMP/NDISC Redirect messages.
> >
> > In IPv4, ip_rt_send_redirect() called icmp_send() directly when peer == NULL.
> > In IPv6, ip6_forward() and ndisc_send_redirect() passed a NULL peer into
> > inet_peer_xrlim_allow(), which returned true when peer == NULL.
> >
> > Because ICMP/NDISC Redirects are not part of the default global rate limit
> > mask (sysctl_icmp_ratemask), sending redirects when peer == NULL creates
> > an un-rate-limited ICMP packet storm.
> >
> > Fix this by failing closed in ip_rt_send_redirect(), ip6_forward(), and
> > ndisc_send_redirect() when peer is NULL.
>
> Bot says:
>
> The following selftest regression was observed after applying this patch:
>
>   Test:   tools/testing/selftests/net/fib_tests.sh
>   Result: FAIL (reproduced on retry)
>   Runner: vmksft-net (x86-64, kernel selftests VM)
>
> The test passes all sections up through the IPv6 route garbage-collection
> tests and then fails with:
>
>   Error while performing Neighbor Discovery for the Destination Address
>   Error while learning Source Address and Next Hop
>   not ok 1 selftests: net: fib_tests.sh # exit=1
>
> This happens during the multipath balance/list tests that follow fib6_gc_test.
> Those tests probe IPv6 multipath routes using `ip route get`, which requires
> NDP to resolve nexthop addresses through a forwarding path.
>
> The new early-return in ip6_forward() when inet_getpeer_v6() returns NULL
> (peer allocation failure, added by this patch) can drop IPv6 packets that
> would previously have been forwarded (and a redirect sent).  In the test
> environment, freshly-created network namespaces may encounter peer-tree
> allocation pressure after the many namespace allocations in earlier test
> sections, causing this path to be exercised unexpectedly and silently
> dropping NDP probes, which causes `ip route get` to return EHOSTUNREACH.
>
> Could you investigate whether the early-return in ip6_forward() for the
> NULL-peer case is too aggressive?  In particular, if the goal is only to
> suppress the redirect (which is correct), dropping the packet entirely on
> a NULL peer may be undesirable — the right approach might be to continue
> forwarding the packet but skip the redirect logic when peer is NULL.

I have no idea really :/

vng --cwd tools/testing/selftests/net -p 4 -r ./arch/x86/boot/bzImage
-- ./fib_tests.sh

 All tests are [ OK ] for me.

Tests passed: 257
Tests failed:   0

Note that my change in ip6_forward() does not early-return, unless I
am mistaken.

diff --git a/net/ipv6/ip6_output.c b/net/ipv6/ip6_output.c
index 368e4fa3b43ca2a96f53fa4f3bc14fd6832c346f..2c44e5ed617167ce060c265052cd40449fdab69f
100644
--- a/net/ipv6/ip6_output.c
+++ b/net/ipv6/ip6_output.c
@@ -641,7 +641,7 @@ int ip6_forward(struct sk_buff *skb)
                /* Limit redirects both by destination (here)
                   and by source (inside ndisc_send_redirect)
                 */
-               if (inet_peer_xrlim_allow(peer, 1*HZ))
+               if (peer && inet_peer_xrlim_allow(peer, 1*HZ))
                        ndisc_send_redirect(skb, target);
                rcu_read_unlock();
        } else {

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

* Re: [PATCH net] net: do not send ICMP/NDISC Redirects when peer allocation fails
  2026-07-24 14:15   ` Eric Dumazet
@ 2026-07-24 14:20     ` Jakub Kicinski
  0 siblings, 0 replies; 4+ messages in thread
From: Jakub Kicinski @ 2026-07-24 14:20 UTC (permalink / raw)
  To: Eric Dumazet
  Cc: David S . Miller, Paolo Abeni, Simon Horman, Ido Schimmel,
	David Ahern, netdev, eric.dumazet

On Fri, 24 Jul 2026 16:15:54 +0200 Eric Dumazet wrote:
> I have no idea really :/
> 
> vng --cwd tools/testing/selftests/net -p 4 -r ./arch/x86/boot/bzImage
> -- ./fib_tests.sh
> 
>  All tests are [ OK ] for me.
> 
> Tests passed: 257
> Tests failed:   0
> 
> Note that my change in ip6_forward() does not early-return, unless I
> am mistaken.

Ack, weird. The only two other patches that came in recently seem to be:

  net: ip6_tunnel: use tunnel parameters for fill_forward_path route lookup
  net: udp_tunnel: fix memory leak in udp_tunnel_nic_unregister()

neither of which seem like they should cause this :/

I'll keep this patch in and keep an eye..

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

end of thread, other threads:[~2026-07-24 14:20 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-24  7:29 [PATCH net] net: do not send ICMP/NDISC Redirects when peer allocation fails Eric Dumazet
2026-07-24 13:42 ` Jakub Kicinski
2026-07-24 14:15   ` Eric Dumazet
2026-07-24 14:20     ` Jakub Kicinski

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.