netdev - Re: [PATCH net] sock_diag: invert socket destroy broadcast check

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, 24 Jun 2016 18:22:50 -0400
From:	Willem de Bruijn <willemdebruijn.kernel@...il.com>
To:	"Eric W. Biederman" <ebiederm@...ssion.com>
Cc:	Network Development <netdev@...r.kernel.org>,
	David Miller <davem@...emloft.net>,
	Craig Gallek <kraig@...gle.com>,
	Eric Dumazet <eric.dumazet@...il.com>,
	Willem de Bruijn <willemb@...gle.com>
Subject: Re: [PATCH net] sock_diag: invert socket destroy broadcast check

On Fri, Jun 24, 2016 at 4:41 PM, Eric W. Biederman
<ebiederm@...ssion.com> wrote:
> Willem de Bruijn <willemdebruijn.kernel@...il.com> writes:
>
>> From: Willem de Bruijn <willemb@...gle.com>
>>
>> Socket destruction is only broadcast for a socket sk if a diag
>> listener is registered and sk is not a kernel socket.
>>
>> Invert the test to not even check for listeners for kernel sockets.
>>
>> The sock_diag_has_destroy_listeners invocation dereferences
>> sock_net(sk), which for kernel sockets can be invalid as they do not
>> take a reference on the network namespace.
>
> No.  That isn't so.  A kernel socket for a network namespace must be
> destroyed in the network namespace teardown.

The issue would be an skbuff with a reference to that sk and
sock_wfree as destructor escaping the network namespace with that
reference intact.

Both macvlan and veth scrub packets before passing them between
namespaces. That alone should prevent this.

I'll try to reproduce with a short dedicated test on the current net branch.

>> I observed skbs queued on a device queue in another namespace from
>> a kernel socket in SOCK_DEAD state with dangling sock_net(sk). Socket
>> refcnt is zero, but sk_wmem_alloc is not. (This was on an older
>> kernel, have not yet tried to reproduce on net).