tcp: do not release socket ownership in tcp_close()
authorEric Dumazet <edumazet@google.com>
Tue, 2 Oct 2018 06:24:26 +0000 (23:24 -0700)
committerDavid S. Miller <davem@davemloft.net>
Wed, 3 Oct 2018 05:17:35 +0000 (22:17 -0700)
syzkaller was able to hit the WARN_ON(sock_owned_by_user(sk));
in tcp_close()

While a socket is being closed, it is very possible other
threads find it in rtnetlink dump.

tcp_get_info() will acquire the socket lock for a short amount
of time (slow = lock_sock_fast(sk)/unlock_sock_fast(sk, slow);),
enough to trigger the warning.

Fixes: 67db3e4bfbc9 ("tcp: no longer hold ehash lock while calling tcp_get_info()")
Signed-off-by: Eric Dumazet <edumazet@google.com>
Reported-by: syzbot <syzkaller@googlegroups.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
include/net/sock.h
net/core/sock.c
net/ipv4/tcp.c

index 38cae35f6e16056c862b29dee8ceb46fe16fff7b..751549ac0a849144ab0382203ee5c877374523e2 100644 (file)
@@ -1492,6 +1492,7 @@ static inline void lock_sock(struct sock *sk)
        lock_sock_nested(sk, 0);
 }
 
+void __release_sock(struct sock *sk);
 void release_sock(struct sock *sk);
 
 /* BH context may only use the following locking interface. */
index 8537b6ca72c5013a75c70978c079646f6278aabf..7e8796a6a0892efbb7dfce67d12b8062b2d5daa9 100644 (file)
@@ -2317,7 +2317,7 @@ static void __lock_sock(struct sock *sk)
        finish_wait(&sk->sk_lock.wq, &wait);
 }
 
-static void __release_sock(struct sock *sk)
+void __release_sock(struct sock *sk)
        __releases(&sk->sk_lock.slock)
        __acquires(&sk->sk_lock.slock)
 {
index 2827fa5643bde84633b174914bbb13a8273e9d32..43ef83b2330e6238a55c9843580a585d87708e0c 100644 (file)
@@ -2416,16 +2416,10 @@ adjudge_to_death:
        sock_hold(sk);
        sock_orphan(sk);
 
-       /* It is the last release_sock in its life. It will remove backlog. */
-       release_sock(sk);
-
-
-       /* Now socket is owned by kernel and we acquire BH lock
-        *  to finish close. No need to check for user refs.
-        */
        local_bh_disable();
        bh_lock_sock(sk);
-       WARN_ON(sock_owned_by_user(sk));
+       /* remove backlog if any, without releasing ownership. */
+       __release_sock(sk);
 
        percpu_counter_inc(sk->sk_prot->orphan_count);
 
@@ -2494,6 +2488,7 @@ adjudge_to_death:
 out:
        bh_unlock_sock(sk);
        local_bh_enable();
+       release_sock(sk);
        sock_put(sk);
 }
 EXPORT_SYMBOL(tcp_close);