NACK+Cmnt: [PATCH] icmp: randomize the global rate limiter

[Thadeu Lima de Souza Cascardo]

On Mon, Feb 22, 2021 at 07:35:50AM -0700, Tim Gardner wrote:
> From: Eric Dumazet <edumazet at google.com>
> 
> Keyu Man reported that the ICMP rate limiter could be used
> by attackers to get useful signal. Details will be provided
> in an upcoming academic publication.
> 
> Our solution is to add some noise, so that the attackers
> no longer can get help from the predictable token bucket limiter.
> 
> Fixes: 4cdf507d5452 ("icmp: add a global rate limitation")
> Signed-off-by: Eric Dumazet <edumazet at google.com>
> Reported-by: Keyu Man <kman001 at ucr.edu>
> Signed-off-by: Jakub Kicinski <kuba at kernel.org>
> (backported from commit b38e7819cae946e2edf869e604af1e65a5d241c5)
> CVE-2020-25705
> Signed-off-by: Tim Gardner <tim.gardner at canonical.com>
> 
> Back port notes: dropped edits to Documentation/networking/ip-sysctl.rst
> as that file does not yet exist.

Well, it is a txt instead of rst file, and 5.4 backport modifies the old
version just fine. Can you resend picking up the same change for that file as
the 5.4 one?

Cascardo.

> ---
>  net/ipv4/icmp.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> index f369e7ce685b..dcffda472585 100644
> --- a/net/ipv4/icmp.c
> +++ b/net/ipv4/icmp.c
> @@ -239,7 +239,7 @@ static struct {
>  /**
>   * icmp_global_allow - Are we allowed to send one more ICMP message ?
>   *
> - * Uses a token bucket to limit our ICMP messages to sysctl_icmp_msgs_per_sec.
> + * Uses a token bucket to limit our ICMP messages to ~sysctl_icmp_msgs_per_sec.
>   * Returns false if we reached the limit and can not send another packet.
>   * Note: called with BH disabled
>   */
> @@ -267,7 +267,10 @@ bool icmp_global_allow(void)
>  	}
>  	credit = min_t(u32, icmp_global.credit + incr, sysctl_icmp_msgs_burst);
>  	if (credit) {
> -		credit--;
> +		/* We want to use a credit of one in average, but need to randomize
> +		 * it for security reasons.
> +		 */
> +		credit = max_t(int, credit - prandom_u32_max(3), 0);
>  		rc = true;
>  	}
>  	WRITE_ONCE(icmp_global.credit, credit);
> -- 
> 2.17.1
> 
> 
> -- 
> kernel-team mailing list
> kernel-team at lists.ubuntu.com
> https://lists.ubuntu.com/mailman/listinfo/kernel-team