* [PATCH net-next] tcp: do not release socket ownership in tcp_close()
@ 2018-10-02 6:24 Eric Dumazet
2018-10-03 5:18 ` David Miller
0 siblings, 1 reply; 2+ messages in thread
From: Eric Dumazet @ 2018-10-02 6:24 UTC (permalink / raw)
To: David S . Miller; +Cc: netdev, Eric Dumazet, Eric Dumazet
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>
---
include/net/sock.h | 1 +
net/core/sock.c | 2 +-
net/ipv4/tcp.c | 11 +++--------
3 files changed, 5 insertions(+), 9 deletions(-)
diff --git a/include/net/sock.h b/include/net/sock.h
index 38cae35f6e16056c862b29dee8ceb46fe16fff7b..751549ac0a849144ab0382203ee5c877374523e2 100644
--- a/include/net/sock.h
+++ b/include/net/sock.h
@@ -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. */
diff --git a/net/core/sock.c b/net/core/sock.c
index 8537b6ca72c5013a75c70978c079646f6278aabf..7e8796a6a0892efbb7dfce67d12b8062b2d5daa9 100644
--- a/net/core/sock.c
+++ b/net/core/sock.c
@@ -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)
{
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 2827fa5643bde84633b174914bbb13a8273e9d32..43ef83b2330e6238a55c9843580a585d87708e0c 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2416,16 +2416,10 @@ void tcp_close(struct sock *sk, long timeout)
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 @@ void tcp_close(struct sock *sk, long timeout)
out:
bh_unlock_sock(sk);
local_bh_enable();
+ release_sock(sk);
sock_put(sk);
}
EXPORT_SYMBOL(tcp_close);
--
2.19.0.605.g01d371f741-goog
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH net-next] tcp: do not release socket ownership in tcp_close()
2018-10-02 6:24 [PATCH net-next] tcp: do not release socket ownership in tcp_close() Eric Dumazet
@ 2018-10-03 5:18 ` David Miller
0 siblings, 0 replies; 2+ messages in thread
From: David Miller @ 2018-10-03 5:18 UTC (permalink / raw)
To: edumazet; +Cc: netdev, eric.dumazet
From: Eric Dumazet <edumazet@google.com>
Date: Mon, 1 Oct 2018 23:24:26 -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>
Applied.
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2018-10-03 12:05 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-10-02 6:24 [PATCH net-next] tcp: do not release socket ownership in tcp_close() Eric Dumazet
2018-10-03 5:18 ` David Miller
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.