netdev - Re: [PATCH] net: Fix double free and memory corruption in get_net_ns_by

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: <20171220.124323.1486261503649399607.davem@davemloft.net>
Date:   Wed, 20 Dec 2017 12:43:23 -0500 (EST)
From:   David Miller <davem@...emloft.net>
To:     ebiederm@...ssion.com
Cc:     netdev@...r.kernel.org, ktkhai@...tuozzo.com, security@...nel.org,
        secalert@...hat.com, eric.dumazet@...il.com,
        stephen@...workplumber.org, nicolas.dichtel@...nd.com
Subject: Re: [PATCH] net: Fix double free and memory corruption in
 get_net_ns_by_id()

From: ebiederm@...ssion.com (Eric W. Biederman)
Date: Tue, 19 Dec 2017 11:27:56 -0600

> (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:
 ...
> 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 <nicolas.dichtel@...nd.com>
> Signed-off-by: Kirill Tkhai <ktkhai@...tuozzo.com>
> Fixes: 0c7aecd4bde4 "netns: add rtnl cmd to add and get peer netns ids"
> Reviewed-by: Andrey Ryabinin <aryabinin@...tuozzo.com>
> Reviewed-by: "Eric W. Biederman" <ebiederm@...ssion.com>
> Signed-off-by: Eric W. Biederman <ebiederm@...ssion.com>

Applied and queued up for -stable, thanks.