From patchwork Tue Jan 30 17:26:42 2018 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Kleber Sacilotto de Souza X-Patchwork-Id: 867567 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=lists.ubuntu.com (client-ip=91.189.94.19; helo=huckleberry.canonical.com; envelope-from=kernel-team-bounces@lists.ubuntu.com; receiver=) Received: from huckleberry.canonical.com (huckleberry.canonical.com [91.189.94.19]) by ozlabs.org (Postfix) with ESMTP id 3zWCwt0npVz9sDB; Wed, 31 Jan 2018 04:26:54 +1100 (AEDT) Received: from localhost ([127.0.0.1] helo=huckleberry.canonical.com) by huckleberry.canonical.com with esmtp (Exim 4.86_2) (envelope-from ) id 1egZgI-00051o-A2; Tue, 30 Jan 2018 17:26:50 +0000 Received: from youngberry.canonical.com ([91.189.89.112]) by huckleberry.canonical.com with esmtps (TLS1.0:DHE_RSA_AES_128_CBC_SHA1:128) (Exim 4.86_2) (envelope-from ) id 1egZgG-000516-32 for kernel-team@lists.ubuntu.com; Tue, 30 Jan 2018 17:26:48 +0000 Received: from mail-wm0-f70.google.com ([74.125.82.70]) by youngberry.canonical.com with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.76) (envelope-from ) id 1egZgF-0002Tk-SG for kernel-team@lists.ubuntu.com; Tue, 30 Jan 2018 17:26:47 +0000 Received: by mail-wm0-f70.google.com with SMTP id f15so683651wmd.1 for ; Tue, 30 Jan 2018 09:26:47 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:subject:date:message-id:in-reply-to :references; bh=g8lgmmyqQ4J1LOVRSZuqz2UNJ8oLpeTWoTzQOLwR300=; b=FTVeFTG09ShmX1+WhOY7oeMChme6FLG0ltsxPVIgJMl+vX2OJQEx/L5gs+SkzwzVmt DdjGmS6G3fRxjxQQrXYa+hAfrBMAF4NWCQ4PKhOozFh9brVFI5qiDcj9tZn1z6ZfIWPc NM+5tZk55yGFDmizO7SuqYW/P5U/INp1Vux2mASNHC0OUmQVXDpeiIyY5CumZz2+c4Zw dLc0ngqfIf+h9yGZXfJ8dZP2kAeV7yRpRCv1gOo5TBk+uT78lR1cOd0CBZVEZmVxBp/X bEYfXOM6aLgz7x63Roo2ML4idN0HBbKGn/EA1G/VT4//lObDR28JCLiAY9uptkHlA6ol 7OVg== X-Gm-Message-State: AKwxytfgsfdRplw36ZuMGEveUHzpZhPY7KY9KQ3hDtmc1EHONcIACqVe cjnAwYIpz1xUz75HumpXxOJlTCrRwj3uXPFJPkHkir/38he4v0nd/K0sAuTVXMtxip8prQlT4Tl 1jKuqDoq61BzTrqskSlA2oEHBgG5x3dPsfIL7ROFBdA== X-Received: by 10.28.148.150 with SMTP id w144mr20818643wmd.0.1517333207238; Tue, 30 Jan 2018 09:26:47 -0800 (PST) X-Google-Smtp-Source: AH8x225c0UdMJ4zF8R7LUP/5JoOxDcWV+r72s1vpxSaDihh04rfQusyAmGabyGFJAU+ChtacJ+FKEQ== X-Received: by 10.28.148.150 with SMTP id w144mr20818636wmd.0.1517333207000; Tue, 30 Jan 2018 09:26:47 -0800 (PST) Received: from localhost ([2a02:8109:98c0:1604:e0bc:dea5:ede9:cfef]) by smtp.gmail.com with ESMTPSA id m191sm11215826wma.42.2018.01.30.09.26.45 for (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Tue, 30 Jan 2018 09:26:46 -0800 (PST) From: Kleber Sacilotto de Souza To: kernel-team@lists.ubuntu.com Subject: [SRU][Xenial][PATCH 1/1] net: Fix double free and memory corruption in get_net_ns_by_id() Date: Tue, 30 Jan 2018 18:26:42 +0100 Message-Id: <20180130172643.1632-2-kleber.souza@canonical.com> X-Mailer: git-send-email 2.14.1 In-Reply-To: <20180130172643.1632-1-kleber.souza@canonical.com> References: <20180130172643.1632-1-kleber.souza@canonical.com> X-BeenThere: kernel-team@lists.ubuntu.com X-Mailman-Version: 2.1.20 Precedence: list List-Id: Kernel team discussions List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , MIME-Version: 1.0 Errors-To: kernel-team-bounces@lists.ubuntu.com Sender: "kernel-team" From: "Eric W. Biederman" (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). Cc: Nicolas Dichtel Signed-off-by: Kirill Tkhai Fixes: 0c7aecd4bde4 "netns: add rtnl cmd to add and get peer netns ids" Reviewed-by: Andrey Ryabinin Reviewed-by: "Eric W. Biederman" Signed-off-by: Eric W. Biederman Reviewed-by: Eric Dumazet Acked-by: Nicolas Dichtel Signed-off-by: David S. Miller CVE-2017-15129 (backported from commit 21b5944350052d2583e82dd59b19a9ba94a007f0) Signed-off-by: Kleber Sacilotto de Souza --- net/core/net_namespace.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/net/core/net_namespace.c b/net/core/net_namespace.c index 2e9a1c2818c7..b5c351d2830b 100644 --- a/net/core/net_namespace.c +++ b/net/core/net_namespace.c @@ -261,7 +261,7 @@ struct net *get_net_ns_by_id(struct net *net, int id) spin_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();