netdev - Re: [PATCH net-next] net: Move net:netns_ids destruction out of rtnl

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] [day] [month] [year] [list]

Message-Id: <20180125.111634.2266308253284284530.davem@davemloft.net>
Date:   Thu, 25 Jan 2018 11:16:34 -0500 (EST)
From:   David Miller <davem@...emloft.net>
To:     ktkhai@...tuozzo.com
Cc:     netdev@...r.kernel.org, ebiederm@...ssion.com, fw@...len.de,
        nicolas.dichtel@...nd.com, dhowells@...hat.com,
        roman.kapl@...go.com, dsa@...ulusnetworks.com, avagin@...tuozzo.com
Subject: Re: [PATCH net-next] net: Move net:netns_ids destruction out of
 rtnl_lock() and document locking scheme

From: Kirill Tkhai <ktkhai@...tuozzo.com>
Date: Fri, 19 Jan 2018 19:14:53 +0300

> Currently, we unhash a dying net from netns_ids lists
> under rtnl_lock(). It's a leftover from the time when
> net::netns_ids was introduced. There was no net::nsid_lock,
> and rtnl_lock() was mostly need to order modification
> of alive nets nsid idr, i.e. for:
> 	for_each_net(tmp) {
> 		...
> 		id = __peernet2id(tmp, net);
> 		idr_remove(&tmp->netns_ids, id);
> 		...
> 	}
> 
> Since we have net::nsid_lock, the modifications are
> protected by this local lock, and now we may introduce
> better scheme of netns_ids destruction.
> 
> Let's look at the functions peernet2id_alloc() and
> get_net_ns_by_id(). Previous commits taught these
> functions to work well with dying net acquired from
> rtnl unlocked lists. And they are the only functions
> which can hash a net to netns_ids or obtain from there.
> And as easy to check, other netns_ids operating functions
> works with id, not with net pointers. So, we do not
> need rtnl_lock to synchronize cleanup_net() with all them.
> 
> The another property, which is used in the patch,
> is that net is unhashed from net_namespace_list
> in the only place and by the only process. So,
> we avoid excess rcu_read_lock() or rtnl_lock(),
> when we'are iterating over the list in unhash_nsid().
> 
> All the above makes possible to keep rtnl_lock() locked
> only for net->list deletion, and completely avoid it
> for netns_ids unhashing and destruction. As these two
> doings may take long time (e.g., memory allocation
> to send skb), the patch should positively act on
> the scalability and signify decrease the time, which
> rtnl_lock() is held in cleanup_net().
> 
> Signed-off-by: Kirill Tkhai <ktkhai@...tuozzo.com>

Ok, this looks ok.  Once we get it off the global list, new
references can't appear.

Then we do an RCU barrier, so all RCU walkers are gone.

And therefore at that point new peer IDs can't get created
for that ns as well.

Applied to net-next, thanks.