netdev - Re: [PATCH net-next 1/3] icmp: change the order of rate limits

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <ed3ca336-7fbb-4247-bdd1-4ee5f3697d11@kernel.org>
Date: Wed, 28 Aug 2024 21:29:07 -0700
From: David Ahern <dsahern@...nel.org>
To: Eric Dumazet <edumazet@...gle.com>, "David S . Miller"
 <davem@...emloft.net>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>
Cc: Willy Tarreau <w@....eu>, Keyu Man <keyu.man@...il.ucr.edu>,
 Jesper Dangaard Brouer <hawk@...nel.org>, netdev@...r.kernel.org,
 eric.dumazet@...il.com, stable@...r.kernel.org
Subject: Re: [PATCH net-next 1/3] icmp: change the order of rate limits

On 8/28/24 1:39 PM, Eric Dumazet wrote:
> ICMP messages are ratelimited :
> 
> After the blamed commits, the two rate limiters are applied in this order:
> 
> 1) host wide ratelimit (icmp_global_allow())
> 
> 2) Per destination ratelimit (inetpeer based)
> 
> In order to avoid side-channels attacks, we need to apply
> the per destination check first.
> 
> This patch makes the following change :
> 
> 1) icmp_global_allow() checks if the host wide limit is reached.
>    But credits are not yet consumed. This is deferred to 3)
> 
> 2) The per destination limit is checked/updated.
>    This might add a new node in inetpeer tree.
> 
> 3) icmp_global_consume() consumes tokens if prior operations succeeded.
> 
> This means that host wide ratelimit is still effective
> in keeping inetpeer tree small even under DDOS.
> 
> As a bonus, I removed icmp_global.lock as the fast path
> can use a lock-free operation.
> 
> Fixes: c0303efeab73 ("net: reduce cycles spend on ICMP replies that gets rate limited")
> Fixes: 4cdf507d5452 ("icmp: add a global rate limitation")
> Reported-by: Keyu Man <keyu.man@...il.ucr.edu>
> Signed-off-by: Eric Dumazet <edumazet@...gle.com>
> Cc: Jesper Dangaard Brouer <hawk@...nel.org>
> Cc: stable@...r.kernel.org
> ---
>  include/net/ip.h |   2 +
>  net/ipv4/icmp.c  | 103 ++++++++++++++++++++++++++---------------------
>  net/ipv6/icmp.c  |  28 ++++++++-----
>  3 files changed, 76 insertions(+), 57 deletions(-)
> 

Reviewed-by: David Ahern <dsahern@...nel.org>