netdev - Re: [PATCH net-next 1/2] net: store netdevs in an xarray

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: Tue, 25 Jul 2023 13:11:50 +0200
From: Paolo Abeni <pabeni@...hat.com>
To: Jakub Kicinski <kuba@...nel.org>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com, 
	mkubecek@...e.cz, lorenzo@...nel.org
Subject: Re: [PATCH net-next 1/2] net: store netdevs in an xarray

On Mon, 2023-07-24 at 12:07 -0700, Jakub Kicinski wrote:
> On Mon, 24 Jul 2023 10:27:41 -0700 Jakub Kicinski wrote:
> > > I still have some minor doubts WRT the 'missed device' scenario you
> > > described in the commit message. What if the user-space is doing
> > > 'create the new one before deleting the old one' with the assumption
> > > that at least one of old/new is always reported in dumps? Is that a too
> > > bold assumption?  
> > 
> > The problem is kinda theoretical in the first place because it assumes
> > ifindexes got wrapped so that the new netdev comes "before" the old in
> > the xarray. Which would require adding and removing 2B netdevs, assuming
> > one add+remove takes 10 usec (impossibly fast), wrapping ifindex would
> > take 68 years.
> 
> I guess the user space can shoot itself in the foot by selecting 
> the lower index for the new device explicitly.
> 
> > And if that's not enough we can make the iteration index ulong 
> > (i.e. something separate from ifindex as ifindex is hardwired to 31b
> > by uAPI).
> 
> We can get the create, delete ordering with this or the list, but the
> inverse theoretical case of delete, create ordering can't be covered.
> A case where user wants to make sure at most one device is visible.
> 
> I'm not sure how much we should care about this. The basic hash table
> had the very real problem of hiding devices which were there *before
> and after* the dump.
> 
> Inconsistent info on devices which were created / deleted *during* the
> dump seems to me like something that's best handled with notifications.
> 
> I'm not sure whether we should set the inconsistency mark on the dump
> when del/add operation happened in the meantime either, as 
> the probability that the user space will care is minuscule.

You convinced me the 'missed device' scenario is not very relevant. 

The cursor with the dummy placeholder looks error-prone and/or too
invasive to me.

I'm fine with this approach.

Thanks!

Paolo