netdev - Re: [RFC PATCH net-next] netlink: avoid namespace change while creating socket

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]

Date:	Tue, 5 May 2015 10:25:09 +0800
From:	Ying Xue <ying.xue@...driver.com>
To:	Herbert Xu <herbert@...dor.apana.org.au>
CC:	<xemul@...nvz.org>, <den@...nvz.org>, <davem@...emloft.net>,
	<avagin@...il.com>, <netdev@...r.kernel.org>
Subject: Re: [RFC PATCH net-next] netlink: avoid namespace change while creating
 socket

On 05/05/2015 09:52 AM, Herbert Xu wrote:
> On Mon, May 04, 2015 at 05:22:19PM +0800, Ying Xue wrote:
>> Commit 23fe18669e7f ("[NETNS]: Fix race between put_net() and
>> netlink_kernel_create().") attempts to fix the following race
>> scenario:
>>
>> put_net()
>>   if (atomic_dec_and_test(&net->refcnt))
>>     /* true */
>>       __put_net(net);
>>         queue_work(...);
>>
>> /*
>>  * note: the net now has refcnt 0, but still in
>>  * the global list of net namespaces
>>  */
>>
>> == re-schedule ==
>>
>> register_pernet_subsys(&some_ops);
>>   register_pernet_operations(&some_ops);
>>     (*some_ops)->init(net);
>>       /*
>>        * we call netlink_kernel_create() here
>>        * in some places
>>        */
>>       netlink_kernel_create();
>>          sk_alloc();
>>             get_net(net); /* refcnt = 1 */
>>          /*
>>           * now we drop the net refcount not to
>>           * block the net namespace exit in the
>>           * future (or this can be done on the
>>           * error path)
>>           */
>>          put_net(sk->sk_net);
>>              if (atomic_dec_and_test(&...))
>>                    /*
>>                     * true. BOOOM! The net is
>>                     * scheduled for release twice
>>                     */
> 
> Surely the problem here is that the caller of netlink_kernel_create
> should hold a ref count on net, so why doesn't it?
> 

I guess the main reason is because calling netlink_kernel_create() just happens
on the path of registering a network namespace subsystem. When
__register_pernet_operations() iterates the global list of net namespace(ie,
net_namespace_list) with for_each_net() to call each net's ops_init(ops, net),
it's supposed that it's safe to touch net instance without holding its refcount
as the net_namespace_list is being protected by net_mutex lock. More
importantly, even if the net refcount is decremented to 0 in putnet(), the net
is still in the global list of net namesapces(ie, net_namespace_list). This is
why the race happens.

Regards,
Ying


> Cheers,
> 

--
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html