[PATCH net] net: ipv4: really enforce backoff for redirects

[PATCH net] net: ipv4: really enforce backoff for redirects

[Paolo Abeni]

In commit b406472b5ad7 ("net: ipv4: avoid mixed n_redirects and
rate_tokens usage") I missed the fact that a 0 'rate_tokens' will
bypass the backoff algorithm.

Since rate_tokens is cleared after a redirect silence, and never
incremented on redirects, if the host keeps receiving packets
requiring redirect it will reply ignoring the backoff.

Additionally, the 'rate_last' field will be updated with the
cadence of the ingress packet requiring redirect. If that rate is
high enough, that will prevent the host from generating any
other kind of ICMP messages

The check for a zero 'rate_tokens' value was likely a shortcut
to avoid the more complex backoff algorithm after a redirect
silence period. Address the issue checking for 'n_redirects'
instead, which is incremented on successful redirect, and
does not interfere with other ICMP replies.

Fixes: b406472b5ad7 ("net: ipv4: avoid mixed n_redirects and rate_tokens usage")
Reported-and-tested-by: Colin Walters <walters@redhat.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/ipv4/route.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/ipv4/route.c b/net/ipv4/route.c
index 788c69d9bfe0..fa829f31a3f5 100644
--- a/net/ipv4/route.c
+++ b/net/ipv4/route.c
@@ -915,7 +915,7 @@ void ip_rt_send_redirect(struct sk_buff *skb)
 	/* Check for load limit; set rate_last to the latest sent
 	 * redirect.
 	 */
-	if (peer->rate_tokens == 0 ||
+	if (peer->n_redirects == 0 ||
 	    time_after(jiffies,
 		       (peer->rate_last +
 			(ip_rt_redirect_load << peer->n_redirects)))) {
-- 
2.21.1

Re: [PATCH net] net: ipv4: really enforce backoff for redirects

[Jakub Kicinski]

On Fri,  8 May 2020 19:28:34 +0200 Paolo Abeni wrote:
> In commit b406472b5ad7 ("net: ipv4: avoid mixed n_redirects and
> rate_tokens usage") I missed the fact that a 0 'rate_tokens' will
> bypass the backoff algorithm.
> 
> Since rate_tokens is cleared after a redirect silence, and never
> incremented on redirects, if the host keeps receiving packets
> requiring redirect it will reply ignoring the backoff.
> 
> Additionally, the 'rate_last' field will be updated with the
> cadence of the ingress packet requiring redirect. If that rate is
> high enough, that will prevent the host from generating any
> other kind of ICMP messages
> 
> The check for a zero 'rate_tokens' value was likely a shortcut
> to avoid the more complex backoff algorithm after a redirect
> silence period. Address the issue checking for 'n_redirects'
> instead, which is incremented on successful redirect, and
> does not interfere with other ICMP replies.
> 
> Fixes: b406472b5ad7 ("net: ipv4: avoid mixed n_redirects and rate_tokens usage")

Looks like this one got backported all the way back to 3.16..

> Reported-and-tested-by: Colin Walters <walters@redhat.com>
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>

Applied, thanks!