netdev - Re: [PATCH 1/4] net: arp: introduce arp_evict

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:   Thu, 14 Oct 2021 09:20:39 -0700
From:   James Prestwood <prestwoj@...il.com>
To:     Daniel Borkmann <daniel@...earbox.net>
Cc:     netdev@...r.kernel.org, roopa@...dia.com, dsahern@...nel.org,
        idosch@...sch.org
Subject: Re: [PATCH 1/4] net: arp: introduce arp_evict_nocarrier sysctl
 parameter

Hi Daniel,

On Thu, 2021-10-14 at 10:30 +0200, Daniel Borkmann wrote:
> [ Adding few more to Cc ]
> 
> On 10/14/21 12:27 AM, James Prestwood wrote:
> > This change introduces a new sysctl parameter, arp_evict_nocarrier.
> > When set (default) the ARP cache will be cleared on a NOCARRIER
> > event.
> > This new option has been defaulted to '1' which maintains existing
> > behavior.
> > 
> > Clearing the ARP cache on NOCARRIER is relatively new, introduced
> > by:
> > 
> > commit 859bd2ef1fc1110a8031b967ee656c53a6260a76
> > Author: David Ahern <dsahern@...il.com>
> > Date:   Thu Oct 11 20:33:49 2018 -0700
> > 
> >      net: Evict neighbor entries on carrier down
> > 
> > The reason for this changes is to prevent the ARP cache from being
> > cleared when a wireless device roams. Specifically for wireless
> > roams
> > the ARP cache should not be cleared because the underlying network
> > has not
> > changed. Clearing the ARP cache in this case can introduce
> > significant
> > delays sending out packets after a roam.
> > 
> > A user reported such a situation here:
> > 
> > https://lore.kernel.org/linux-wireless/CACsRnHWa47zpx3D1oDq9JYnZWniS8yBwW1h0WAVZ6vrbwL_S0w@mail.gmail.com/
> > 
> > After some investigation it was found that the kernel was holding
> > onto
> > packets until ARP finished which resulted in this 1 second delay.
> > It
> > was also found that the first ARP who-has was never responded to,
> > which is actually what caues the delay. This change is more or less
> > working around this behavior, but again, there is no reason to
> > clear
> > the cache on a roam anyways.
> > 
> > As for the unanswered who-has, we know the packet made it OTA since
> > it was seen while monitoring. Why it never received a response is
> > unknown.
> 
> Wouldn't it make more sense to extend neigh_flush_dev() where we skip
> eviction
> of NUD_PERMANENT (see the skip_perm condition)? Either as a per table
> setting
> (tbl->parms) or as a NTF_EXT_* flag for specific neighbors?
> 

This is all new code to me, but from what I can tell NUD_PERMANENT is a
per-neighbor thing, which a wireless supplicant shouldn't be expected
to manage. It also seems like a pain to mark all neighbors as permanent
prior to a roam, then undo it all after a roam.

As for table settings I'll need to look into those, and if/how you
expose them to userspace? I modeled both these (arp/ndisc) options
after the other arp_*/ndisc_* sysctl parameters, as they seemed to fit
in there.

Thanks,
James

>