Patch "net: Fix double free and memory corruption in get_net_ns_by_id()" has been added to the 4.4-stable tree
Subsystems (2)
This is a note to let you know that I've just added the patch titled
net: Fix double free and memory corruption in get_net_ns_by_id()
to the 4.4-stable tree which can be found at:
http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary
The filename of the patch is:
net-fix-double-free-and-memory-corruption-in-get_net_ns_by_id.patch
and it can be found in the queue-4.4 subdirectory.
If you, or anyone else, feels it should not be added to the stable tree, please let <[email protected]> know about it.
Quoted 1 line
From foo@baz Sun Dec 31 11:20:35 CET 2017
From: "Eric W. Biederman" <[email protected]>
Date: Tue, 19 Dec 2017 11:27:56 -0600
Subject: net: Fix double free and memory corruption in get_net_ns_by_id()
From: "Eric W. Biederman" <[email protected]>
[ Upstream commit 21b5944350052d2583e82dd59b19a9ba94a007f0 ]
(I can trivially verify that that idr_remove in cleanup_net happens
after the network namespace count has dropped to zero --EWB)
Function get_net_ns_by_id() does not check for net::count after it has found a peer in netns_ids idr.
It may dereference a peer, after its count has already been finaly decremented. This leads to double free and memory corruption:
put_net(peer) rtnl_lock()
atomic_dec_and_test(&peer->count) [count=0] ...
__put_net(peer) get_net_ns_by_id(net, id)
spin_lock(&cleanup_list_lock)
list_add(&net->cleanup_list, &cleanup_list)
spin_unlock(&cleanup_list_lock)
queue_work() peer = idr_find(&net->netns_ids, id)
| get_net(peer) [count=1]
| ...
| (use after final put)
v ...
cleanup_net() ...
spin_lock(&cleanup_list_lock) ...
list_replace_init(&cleanup_list, ..) ...
spin_unlock(&cleanup_list_lock) ...
... ...
... put_net(peer)
... atomic_dec_and_test(&peer->count) [count=0]
... spin_lock(&cleanup_list_lock)
... list_add(&net->cleanup_list, &cleanup_list)
... spin_unlock(&cleanup_list_lock)
... queue_work()
... rtnl_unlock()
rtnl_lock() ...
for_each_net(tmp) { ...
id = __peernet2id(tmp, peer) ...
spin_lock_irq(&tmp->nsid_lock) ...
idr_remove(&tmp->netns_ids, id) ...
... ...
net_drop_ns() ...
net_free(peer) ...
} ... |
v
cleanup_net()
...
(Second free of peer)
Also, put_net() on the right cpu may reorder with left's cpu list_replace_init(&cleanup_list, ..), and then cleanup_list will be corrupted.
Since cleanup_net() is executed in worker thread, while put_net(peer) can happen everywhere, there should be enough time for concurrent get_net_ns_by_id() to pick the peer up, and the race does not seem to be unlikely. The patch fixes the problem in standard way.
(Also, there is possible problem in peernet2id_alloc(), which requires check for net::count under nsid_lock and maybe_get_net(peer), but in current stable kernel it's used under rtnl_lock() and it has to be safe. Openswitch begun to use peernet2id_alloc(), and possibly it should be fixed too. While this is not in stable kernel yet, so I'll send a separate message to netdev@ later).
--- net/core/net_namespace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
net/core/net_namespace.c +1 -1
@@ -261,7 +261,7 @@ struct net *get_net_ns_by_id(struct netspin_lock_irqsave (&net->nsid_lock, flags); peer =idr_find (&net->netns_ids, id);if (peer) -get_net (peer); + peer =maybe_get_net (peer);spin_unlock_irqrestore (&net->nsid_lock, flags);rcu_read_unlock ();
Patches currently in stable-queue which might be from [email protected] are
queue-4.4/net-fix-double-free-and-memory-corruption-in-get_net_ns_by_id.patch