netdev - Re: [PATCH v2 2/3] net: Add BUG_ON() to get

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

Message-ID: <38b8d4da-664e-9f91-6c99-c5d290c6fe3f@virtuozzo.com>
Date:   Wed, 10 Jan 2018 10:37:52 +0300
From:   Kirill Tkhai <ktkhai@...tuozzo.com>
To:     Eric Dumazet <eric.dumazet@...il.com>, netdev@...r.kernel.org,
        davem@...emloft.net
Cc:     ebiederm@...ssion.com
Subject: Re: [PATCH v2 2/3] net: Add BUG_ON() to get_net()

On 09.01.2018 21:52, Eric Dumazet wrote:
> On Tue, 2018-01-09 at 18:00 +0300, Kirill Tkhai wrote:
>> Since people may mistakenly obtain destroying net
>> from net_namespace_list and from net::netns_ids
>> without checking for its net::counter, let's protect
>> against such situations and insert BUG_ON() to stop
>> move on after this.
>>
>> Panic is better, than memory corruption and undefined
>> behavior.
>>
>> Signed-off-by: Kirill Tkhai <ktkhai@...tuozzo.com>
>> ---
>>  include/net/net_namespace.h |    2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/include/net/net_namespace.h b/include/net/net_namespace.h
>> index 10f99dafd5ac..ff0e47471d5b 100644
>> --- a/include/net/net_namespace.h
>> +++ b/include/net/net_namespace.h
>> @@ -195,7 +195,7 @@ void __put_net(struct net *net);
>>  
>>  static inline struct net *get_net(struct net *net)
>>  {
>> -	atomic_inc(&net->count);
>> +	BUG_ON(atomic_inc_return(&net->count) <= 1);
>>  	return net;
>>  }
> 
> 
> Why not simply use refcount_t instead of duplicating its logic?

The main goal of the change is to catch rare races happening on production nodes
with real load and to prevent memory corruption. You can't simply use refcount_t
primitives, as there is no appropriate primitive with BUG_ON() among them. WARN_ON()
from the primitives doesn't protect from memory corruption.

Also, keep in mind, that CONFIG_REFCOUNT_FULL is usually disabled on no-debug kernel.
I've checked both Fedora and Debian. So, the only possibility to catch such the races,
if someone really happy meets them on test kernel and test workload, which is usually
is very unlikely.

Kirill