netdev - Re: [PATCH net] neigh: Force garbage collection if an entry is deleted administratively

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]

Date:	Mon, 18 Nov 2013 16:21:15 -0500 (EST)
From:	David Miller <davem@...emloft.net>
To:	steffen.klassert@...unet.com
Cc:	yoshfuji@...ux-ipv6.org, netdev@...r.kernel.org
Subject: Re: [PATCH net] neigh: Force garbage collection if an entry is
 deleted administratively

From: Steffen Klassert <steffen.klassert@...unet.com>
Date: Mon, 18 Nov 2013 11:08:43 +0100

> Subject: [PATCH RFC] neigh: Fix garbage collection if the cached entries are
>  below the threshold
> 
> Since git commit 2724680 ("neigh: Keep neighbour cache entries if number
> of them is small enough."), we keep all neighbour cache entries if the
> number is below a threshold. But if we now delete an entry administratively
> and then try to replace this by a permanent one, we get -EEXIST because the
> old entry ist still in the table (in NUD_FAILED state).
> 
> So remove the threshold check in neigh_periodic_work() and schedule the
> gc_work only when needed, i.e. if gc_thresh1 is reached or if there is
> an administrative change. We reschedule gc_work either if the number of
> cache entries is still above gc_thresh1 or if there are invalid entries
> with "refcnt != 1" cached.
> 
> Signed-off-by: Steffen Klassert <steffen.klassert@...unet.com>

I think the main issue is that after this patch, the problem is really
still there.

Let's say some device holds onto the neigh for a long time, then during
this time an administrative replacement will still get that -EEXIST
failure.

My conclusion is that the management of the state is the problem.
Specifically, if we invalidate an entry then we should remove it's
visisbility.  This means the table should operate by unhashing the
entry unconditionally during such operations.

If some stray references exist, that's fine, the entity holding the
reference will perform the final neigh cleanup at release time.

Does this make sense to you?
--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html