[PATCH net] net: avoid NULL deref in inet_ctl_sock_destroy()

[PATCH net] net: avoid NULL deref in inet_ctl_sock_destroy()

[Eric Dumazet]

From: Eric Dumazet <edumazet@google.com>

Under low memory conditions, tcp_sk_init() and icmp_sk_init()
can both iterate on all possible cpus and call inet_ctl_sock_destroy(),
with eventual NULL pointer.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: Dmitry Vyukov <dvyukov@google.com>
---
 include/net/inet_common.h |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/include/net/inet_common.h b/include/net/inet_common.h
index 279f83591971..109e3ee9108c 100644
--- a/include/net/inet_common.h
+++ b/include/net/inet_common.h
@@ -41,7 +41,8 @@ int inet_recv_error(struct sock *sk, struct msghdr *msg, int len,
 
 static inline void inet_ctl_sock_destroy(struct sock *sk)
 {
-	sock_release(sk->sk_socket);
+	if (sk)
+		sock_release(sk->sk_socket);
 }
 
 #endif

Re: [PATCH net] net: avoid NULL deref in inet_ctl_sock_destroy()

[Hannes Frederic Sowa]

On Mon, Nov 2, 2015, at 16:50, Eric Dumazet wrote:
> From: Eric Dumazet <edumazet@google.com>
> 
> Under low memory conditions, tcp_sk_init() and icmp_sk_init()
> can both iterate on all possible cpus and call inet_ctl_sock_destroy(),
> with eventual NULL pointer.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Dmitry Vyukov <dvyukov@google.com>

Eric, was this a private report or some of those floating around
publicly?

Thanks,
Hannes

Re: [PATCH net] net: avoid NULL deref in inet_ctl_sock_destroy()

[Eric Dumazet]

On Mon, 2015-11-02 at 17:53 +0100, Hannes Frederic Sowa wrote:
> On Mon, Nov 2, 2015, at 16:50, Eric Dumazet wrote:
> > From: Eric Dumazet <edumazet@google.com>
> > 
> > Under low memory conditions, tcp_sk_init() and icmp_sk_init()
> > can both iterate on all possible cpus and call inet_ctl_sock_destroy(),
> > with eventual NULL pointer.
> > 
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Reported-by: Dmitry Vyukov <dvyukov@google.com>
> 
> Eric, was this a private report or some of those floating around
> publicly?

Dmitry Vyukov filled two internal bug reports at Google,
not sure if he mentioned the issue elsewhere.

Google-Bug-Id: 25415196
Google-Bug-Id: 25416355

(But you do not have access to them)

Re: [PATCH net] net: avoid NULL deref in inet_ctl_sock_destroy()

[Dmitry Vyukov]

On Mon, Nov 2, 2015 at 6:00 PM, Eric Dumazet <eric.dumazet@gmail.com> wrote:
> On Mon, 2015-11-02 at 17:53 +0100, Hannes Frederic Sowa wrote:
>> On Mon, Nov 2, 2015, at 16:50, Eric Dumazet wrote:
>> > From: Eric Dumazet <edumazet@google.com>
>> >
>> > Under low memory conditions, tcp_sk_init() and icmp_sk_init()
>> > can both iterate on all possible cpus and call inet_ctl_sock_destroy(),
>> > with eventual NULL pointer.
>> >
>> > Signed-off-by: Eric Dumazet <edumazet@google.com>
>> > Reported-by: Dmitry Vyukov <dvyukov@google.com>
>>
>> Eric, was this a private report or some of those floating around
>> publicly?
>
> Dmitry Vyukov filled two internal bug reports at Google,
> not sure if he mentioned the issue elsewhere.

No, I did not.
Can I now?

> Google-Bug-Id: 25415196
> Google-Bug-Id: 25416355
>
> (But you do not have access to them)
>
>
>

Re: [PATCH net] net: avoid NULL deref in inet_ctl_sock_destroy()

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 02 Nov 2015 07:50:07 -0800

> From: Eric Dumazet <edumazet@google.com>
> 
> Under low memory conditions, tcp_sk_init() and icmp_sk_init()
> can both iterate on all possible cpus and call inet_ctl_sock_destroy(),
> with eventual NULL pointer.
> 
> Signed-off-by: Eric Dumazet <edumazet@google.com>
> Reported-by: Dmitry Vyukov <dvyukov@google.com>

Applied.

Re: [PATCH net] net: avoid NULL deref in inet_ctl_sock_destroy()

[Eric Dumazet]

On Mon, 2015-11-02 at 22:46 -0500, David Miller wrote:
> From: Eric Dumazet <eric.dumazet@gmail.com>
> Date: Mon, 02 Nov 2015 07:50:07 -0800
> 
> > From: Eric Dumazet <edumazet@google.com>
> > 
> > Under low memory conditions, tcp_sk_init() and icmp_sk_init()
> > can both iterate on all possible cpus and call inet_ctl_sock_destroy(),
> > with eventual NULL pointer.
> > 
> > Signed-off-by: Eric Dumazet <edumazet@google.com>
> > Reported-by: Dmitry Vyukov <dvyukov@google.com>
> 
> Applied.

Thanks David.

Bug origin was in linux-4.2 :

commit 26abe14379f8e2fa3fd1bcf97c9a7ad9364886fe
Author: Eric W. Biederman <ebiederm@xmission.com>
Date:   Fri May 8 21:10:31 2015 -0500

    net: Modify sk_alloc to not reference count the netns of kernel sockets.
    
    Now that sk_alloc knows when a kernel socket is being allocated modify
    it to not reference count the network namespace of kernel sockets.
    
    Keep track of if a socket needs reference counting by adding a flag to
    struct sock called sk_net_refcnt.
    
    Update all of the callers of sock_create_kern to stop using
    sk_change_net and sk_release_kernel as those hacks are no longer
    needed, to avoid reference counting a kernel socket.
    
    Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
    Signed-off-by: David S. Miller <davem@davemloft.net>

Re: [PATCH net] net: avoid NULL deref in inet_ctl_sock_destroy()

[David Miller]

From: Eric Dumazet <eric.dumazet@gmail.com>
Date: Mon, 02 Nov 2015 21:04:01 -0800

> On Mon, 2015-11-02 at 22:46 -0500, David Miller wrote:
>> From: Eric Dumazet <eric.dumazet@gmail.com>
>> Date: Mon, 02 Nov 2015 07:50:07 -0800
>> 
>> > From: Eric Dumazet <edumazet@google.com>
>> > 
>> > Under low memory conditions, tcp_sk_init() and icmp_sk_init()
>> > can both iterate on all possible cpus and call inet_ctl_sock_destroy(),
>> > with eventual NULL pointer.
>> > 
>> > Signed-off-by: Eric Dumazet <edumazet@google.com>
>> > Reported-by: Dmitry Vyukov <dvyukov@google.com>
>> 
>> Applied.
> 
> Thanks David.
> 
> Bug origin was in linux-4.2 :

Ok, queued up for -stable then...