netdev - Re: [PATCH net] neighbour: allow NUD_NOARP entries to be forced GCed

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: <c704333a-e326-57ba-78e7-1e7111f1e79c@nvidia.com>
Date:   Mon, 7 Jun 2021 11:53:09 -0700
From:   Roopa Prabhu <roopa@...dia.com>
To:     David Ahern <dsahern@...nel.org>, <netdev@...r.kernel.org>,
        <kuba@...nel.org>, <davem@...emloft.net>
CC:     Kasper Dupont <kasperd@...wv.06.feb.2021.kasperd.net>,
        Thadeu Lima de Souza Cascardo <cascardo@...onical.com>
Subject: Re: [PATCH net] neighbour: allow NUD_NOARP entries to be forced GCed


On 6/7/21 10:35 AM, David Ahern wrote:
> IFF_POINTOPOINT interfaces use NUD_NOARP entries for IPv6. It's possible to
> fill up the neighbour table with enough entries that it will overflow for
> valid connections after that.
>
> This behaviour is more prevalent after commit 58956317c8de ("neighbor:
> Improve garbage collection") is applied, as it prevents removal from
> entries that are not NUD_FAILED, unless they are more than 5s old.
>
> Fixes: 58956317c8de (neighbor: Improve garbage collection)
> Reported-by: Kasper Dupont <kasperd@...wv.06.feb.2021.kasperd.net>
> Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo@...onical.com>
> Signed-off-by: David Ahern <dsahern@...nel.org>
> ---
> rebased to net tree


There are other use-cases  that use NUD_NOARP as static neighbour 
entries which should be exempt from forced gc.

for example when qualified by NTF_EXT_LEARNED for the E-VPN use-case.

The check in your patch below should exclude NTF_EXT_LEARNED entries.


(unrelated to the neighbour code ,  but bridge driver also uses 
NUD_NOARP for static entries)


>
>   net/core/neighbour.c | 1 +
>   1 file changed, 1 insertion(+)
>
> diff --git a/net/core/neighbour.c b/net/core/neighbour.c
> index 98f20efbfadf..bf774575ad71 100644
> --- a/net/core/neighbour.c
> +++ b/net/core/neighbour.c
> @@ -238,6 +238,7 @@ static int neigh_forced_gc(struct neigh_table *tbl)
>   
>   			write_lock(&n->lock);
>   			if ((n->nud_state == NUD_FAILED) ||
> +			    (n->nud_state == NUD_NOARP) ||
>   			    (tbl->is_multicast &&
>   			     tbl->is_multicast(n->primary_key)) ||
>   			    time_after(tref, n->updated))