Commit 8873c064 authored by Eric Dumazet's avatar Eric Dumazet Committed by David S. Miller

tcp: do not release socket ownership in tcp_close()

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: 67db3e4b ("tcp: no longer hold ehash lock while calling tcp_get_info()")
Signed-off-by: default avatarEric Dumazet <edumazet@google.com>
Reported-by: default avatarsyzbot <syzkaller@googlegroups.com>
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parent 744486d4
...@@ -1492,6 +1492,7 @@ static inline void lock_sock(struct sock *sk) ...@@ -1492,6 +1492,7 @@ static inline void lock_sock(struct sock *sk)
lock_sock_nested(sk, 0); lock_sock_nested(sk, 0);
} }
void __release_sock(struct sock *sk);
void release_sock(struct sock *sk); void release_sock(struct sock *sk);
/* BH context may only use the following locking interface. */ /* BH context may only use the following locking interface. */
......
...@@ -2317,7 +2317,7 @@ static void __lock_sock(struct sock *sk) ...@@ -2317,7 +2317,7 @@ static void __lock_sock(struct sock *sk)
finish_wait(&sk->sk_lock.wq, &wait); 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) __releases(&sk->sk_lock.slock)
__acquires(&sk->sk_lock.slock) __acquires(&sk->sk_lock.slock)
{ {
......
...@@ -2416,16 +2416,10 @@ void tcp_close(struct sock *sk, long timeout) ...@@ -2416,16 +2416,10 @@ void tcp_close(struct sock *sk, long timeout)
sock_hold(sk); sock_hold(sk);
sock_orphan(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(); local_bh_disable();
bh_lock_sock(sk); 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); percpu_counter_inc(sk->sk_prot->orphan_count);
...@@ -2494,6 +2488,7 @@ void tcp_close(struct sock *sk, long timeout) ...@@ -2494,6 +2488,7 @@ void tcp_close(struct sock *sk, long timeout)
out: out:
bh_unlock_sock(sk); bh_unlock_sock(sk);
local_bh_enable(); local_bh_enable();
release_sock(sk);
sock_put(sk); sock_put(sk);
} }
EXPORT_SYMBOL(tcp_close); EXPORT_SYMBOL(tcp_close);
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment