diff options
| -rw-r--r-- | patches/cve/CVE-2017-15129-net-Fix-double-free-and-memory-corruption-in-get_net.patch | 107 |
1 files changed, 107 insertions, 0 deletions
diff --git a/patches/cve/CVE-2017-15129-net-Fix-double-free-and-memory-corruption-in-get_net.patch b/patches/cve/CVE-2017-15129-net-Fix-double-free-and-memory-corruption-in-get_net.patch new file mode 100644 index 0000000..18153db --- /dev/null +++ b/patches/cve/CVE-2017-15129-net-Fix-double-free-and-memory-corruption-in-get_net.patch | |||
| @@ -0,0 +1,107 @@ | |||
| 1 | From dd9a2648b3e35c2369f580215d916baf7e23253a Mon Sep 17 00:00:00 2001 | ||
| 2 | From: "Eric W. Biederman" <ebiederm@xmission.com> | ||
| 3 | Date: Tue, 19 Dec 2017 11:27:56 -0600 | ||
| 4 | Subject: [PATCH] net: Fix double free and memory corruption in | ||
| 5 | get_net_ns_by_id() | ||
| 6 | |||
| 7 | [ Upstream commit 21b5944350052d2583e82dd59b19a9ba94a007f0 ] | ||
| 8 | |||
| 9 | (I can trivially verify that that idr_remove in cleanup_net happens | ||
| 10 | after the network namespace count has dropped to zero --EWB) | ||
| 11 | |||
| 12 | Function get_net_ns_by_id() does not check for net::count | ||
| 13 | after it has found a peer in netns_ids idr. | ||
| 14 | |||
| 15 | It may dereference a peer, after its count has already been | ||
| 16 | finaly decremented. This leads to double free and memory | ||
| 17 | corruption: | ||
| 18 | |||
| 19 | put_net(peer) rtnl_lock() | ||
| 20 | atomic_dec_and_test(&peer->count) [count=0] ... | ||
| 21 | __put_net(peer) get_net_ns_by_id(net, id) | ||
| 22 | spin_lock(&cleanup_list_lock) | ||
| 23 | list_add(&net->cleanup_list, &cleanup_list) | ||
| 24 | spin_unlock(&cleanup_list_lock) | ||
| 25 | queue_work() peer = idr_find(&net->netns_ids, id) | ||
| 26 | | get_net(peer) [count=1] | ||
| 27 | | ... | ||
| 28 | | (use after final put) | ||
| 29 | v ... | ||
| 30 | cleanup_net() ... | ||
| 31 | spin_lock(&cleanup_list_lock) ... | ||
| 32 | list_replace_init(&cleanup_list, ..) ... | ||
| 33 | spin_unlock(&cleanup_list_lock) ... | ||
| 34 | ... ... | ||
| 35 | ... put_net(peer) | ||
| 36 | ... atomic_dec_and_test(&peer->count) [count=0] | ||
| 37 | ... spin_lock(&cleanup_list_lock) | ||
| 38 | ... list_add(&net->cleanup_list, &cleanup_list) | ||
| 39 | ... spin_unlock(&cleanup_list_lock) | ||
| 40 | ... queue_work() | ||
| 41 | ... rtnl_unlock() | ||
| 42 | rtnl_lock() ... | ||
| 43 | for_each_net(tmp) { ... | ||
| 44 | id = __peernet2id(tmp, peer) ... | ||
| 45 | spin_lock_irq(&tmp->nsid_lock) ... | ||
| 46 | idr_remove(&tmp->netns_ids, id) ... | ||
| 47 | ... ... | ||
| 48 | net_drop_ns() ... | ||
| 49 | net_free(peer) ... | ||
| 50 | } ... | ||
| 51 | | | ||
| 52 | v | ||
| 53 | cleanup_net() | ||
| 54 | ... | ||
| 55 | (Second free of peer) | ||
| 56 | |||
| 57 | Also, put_net() on the right cpu may reorder with left's cpu | ||
| 58 | list_replace_init(&cleanup_list, ..), and then cleanup_list | ||
| 59 | will be corrupted. | ||
| 60 | |||
| 61 | Since cleanup_net() is executed in worker thread, while | ||
| 62 | put_net(peer) can happen everywhere, there should be | ||
| 63 | enough time for concurrent get_net_ns_by_id() to pick | ||
| 64 | the peer up, and the race does not seem to be unlikely. | ||
| 65 | The patch fixes the problem in standard way. | ||
| 66 | |||
| 67 | (Also, there is possible problem in peernet2id_alloc(), which requires | ||
| 68 | check for net::count under nsid_lock and maybe_get_net(peer), but | ||
| 69 | in current stable kernel it's used under rtnl_lock() and it has to be | ||
| 70 | safe. Openswitch begun to use peernet2id_alloc(), and possibly it should | ||
| 71 | be fixed too. While this is not in stable kernel yet, so I'll send | ||
| 72 | a separate message to netdev@ later). | ||
| 73 | |||
| 74 | CVE: CVE-2017-15129 | ||
| 75 | Upstream-Status: Backport [https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/commit/?h=linux-4.14.y&id=dd9a2648b3e35c2369f580215d916baf7e23253a] | ||
| 76 | |||
| 77 | Cc: Nicolas Dichtel <nicolas.dichtel@6wind.com> | ||
| 78 | Signed-off-by: Kirill Tkhai <ktkhai@virtuozzo.com> | ||
| 79 | Fixes: 0c7aecd4bde4 "netns: add rtnl cmd to add and get peer netns ids" | ||
| 80 | Reviewed-by: Andrey Ryabinin <aryabinin@virtuozzo.com> | ||
| 81 | Reviewed-by: "Eric W. Biederman" <ebiederm@xmission.com> | ||
| 82 | Signed-off-by: Eric W. Biederman <ebiederm@xmission.com> | ||
| 83 | Reviewed-by: Eric Dumazet <edumazet@google.com> | ||
| 84 | Acked-by: Nicolas Dichtel <nicolas.dichtel@6wind.com> | ||
| 85 | Signed-off-by: David S. Miller <davem@davemloft.net> | ||
| 86 | Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> | ||
| 87 | Signed-off-by: Andreas Wellving <andreas.wellving@enea.com> | ||
| 88 | --- | ||
| 89 | net/core/net_namespace.c | 2 +- | ||
| 90 | 1 file changed, 1 insertion(+), 1 deletion(-) | ||
| 91 | |||
| 92 | diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c | ||
| 93 | index 6cfdc7c84c48..0dd6359e5924 100644 | ||
| 94 | --- a/net/core/net_namespace.c | ||
| 95 | +++ b/net/core/net_namespace.c | ||
| 96 | @@ -266,7 +266,7 @@ struct net *get_net_ns_by_id(struct net *net, int id) | ||
| 97 | spin_lock_bh(&net->nsid_lock); | ||
| 98 | peer = idr_find(&net->netns_ids, id); | ||
| 99 | if (peer) | ||
| 100 | - get_net(peer); | ||
| 101 | + peer = maybe_get_net(peer); | ||
| 102 | spin_unlock_bh(&net->nsid_lock); | ||
| 103 | rcu_read_unlock(); | ||
| 104 | |||
| 105 | -- | ||
| 106 | 2.20.1 | ||
| 107 | |||
