netdev - Re: [PATCH v3] net: #ifdef inet_bind_bucket::ib

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:	Fri, 14 Nov 2008 09:14:53 +0300
From:	Alexey Dobriyan <adobriyan@...il.com>
To:	Eric Dumazet <dada1@...mosbay.com>
Cc:	David Miller <davem@...emloft.net>, netdev@...r.kernel.org
Subject: Re: [PATCH v3] net: #ifdef inet_bind_bucket::ib_net

On Fri, Nov 14, 2008 at 05:54:26AM +0100, Eric Dumazet wrote:
> David Miller a écrit :
>> From: Eric Dumazet <dada1@...mosbay.com>
>> Date: Fri, 14 Nov 2008 05:36:15 +0100
>>
>>> This is better because :
>>>
>>> 1) No #ifdef CONFIG_NET_NS
>>>
>>> 2) The magic about &init_net is not duplicated in ten different include files, but
>>>    centralized in the right file : include/net/net_namespace.h
>>
>> I %100 agree.
>
> Speaking of those functions, what do you think of this one ?
>
> static inline
> void dev_net_set(struct net_device *dev, struct net *net)
> {
> #ifdef CONFIG_NET_NS
>        release_net(dev->nd_net);
>        dev->nd_net = hold_net(net);
> #endif
> }
>
> I believe that its safer to hold a reference on "new" *before*
> releasing reference on "old" object.
>
> Also, release_net() and hold_net() can be defined to do
> the use_count refcounting regardless of CONFIG_NET_NS
> (Its a different NETNS_REFCNT_DEBUG #ifdef)

NETNS_REFCNT_DEBUG makes sense only with NET_NS=y because init_net
is never freed.

> Yet another example where read_pnet() and write_pnet()
> are the right answer : Its cleaner and fixes *bugs*.

pnet stuff by definition can't fix bugs :-)

Ask from where new net comes?

If from current, nsproxy refcount is at least 1, so netns refcount is at least 1,
so shutdown sequence can't start.

If from userspace socket, there is task in netns -- see #1

If from netdevice on which ioctl is done, some task did ioctl -- see #1.

And so on.

But this is peanuts, because your race matters only when netns is almost freed
(in kmem_cache_free sense), so you're stashing dangling pointer somewhere else
which is a bug by itself.

> static inline
> void dev_net_set(struct net_device *dev, struct net *net)
> {
> 	hold_net(net);
> 	release_net(read_pnet(&dev->nd_net);
>        write_pnet(&dev->nd_net, net);
> }
--
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