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 for Android: free password hash cracker in your pocket

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

Message-ID: <20230724102741.469c0e42@kernel.org>
Date: Mon, 24 Jul 2023 10:27:41 -0700
From: Jakub Kicinski <kuba@...nel.org>
To: Paolo Abeni <pabeni@...hat.com>
Cc: davem@...emloft.net, netdev@...r.kernel.org, edumazet@...gle.com,
 mkubecek@...e.cz, lorenzo@...nel.org, Herbert Xu
 <herbert@...dor.apana.org.au>, Neil Brown <neilb@...e.de>
Subject: Re: [PATCH net-next 1/2] net: store netdevs in an xarray

On Mon, 24 Jul 2023 18:23:03 +0200 Paolo Abeni wrote:
> On Mon, 2023-07-24 at 08:41 -0700, Jakub Kicinski wrote:
> > On Mon, 24 Jul 2023 10:18:04 +0200 Paolo Abeni wrote:  
> > > A possibly dumb question: why using an xarray over a plain list?  
> > 
> > We need to drop the lock during the walk.   
> 
> I should have looked more closely to patch 2/2.
> 
> > So for a list we'd need 
> > to either 
> >  - add explicit iteration "cursor" or   
> 
> Would a cursor require acquiring a netdev reference? If so it looks
> problematic (an evil/buggy userspace could keep such reference held for
> an unbounded amount of time).

I was thinking we can add a cursor which looks like:

struct netdev_list_node {
	struct list head_list;
	bool is_a_device;
};

then iteration can put this struct into its dump state and insert
*itself* into the device list. All iterations will then have to:

	struct netdev_list_node *lnode;

	list_for_each_entry... lnode) {
		/* skip cursors inserted into the list */
		if (!lnode->is_a_device)
			continue;
		netdev = containerof(lnode);

		...
	}

But then we need to add some code to .start and .stop callbacks
and make sure the "cursor" is removed if someone closes the socket
half way thru the dump.

> I agree xarray looks a better solution.

I do think xarray is the simplest solution.

> 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.

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).