net: tcp: check for SOCK_DEAD again in tcp_nuke_addr

Liping Zhang spotted a race between tcp_nuke_addr and tcp_close
that can cause a crash. If a userspace process calls tcp_close
on a socket at the same time that tcp_nuke_addr is closing it,
and tcp_close wins the race to call lock_sock, it will call
sock_orphan before releasing the lock. sock_orphan sets the
SOCK_DEAD flag on the socket and proceeds to close it, eventually
calling inet_csk_destroy_sock. When tcp_nuke_addr gets the socket
lock, it calls tcp_done. But if tcp_done sees the SOCK_DEAD flag,
it calls inet_csk_destroy_sock as well, resulting in a double
free.

Fix this by checking for SOCK_DEAD again after lock_sock
succeeds. Eric had already pointed out that this could be a
problem in b/23663111, so there was already a TODO in the code
for this.

Change-Id: I0c87c3fd0598384d957b69734366bd4e2fd7e8d7
Git-commit: 61469ddc534f255c709349a1a611216ecd07e13d
Git-repo: https://android.googlesource.com/kernel/common/
Signed-off-by: Subash Abhinov Kasiviswanathan <subashab@codeaurora.org>

1 files changed, 9 insertions, 6 deletions

diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 9456c5822ed7..fea4961647ee 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -3349,14 +3349,17 @@ restart:
 			spin_unlock_bh(lock);
 
 			lock_sock(sk);
-			// TODO:
-			// Check for SOCK_DEAD again, it could have changed.
-			// Add a write barrier, see tcp_reset().
 			local_bh_disable();
-			sk->sk_err = ETIMEDOUT;
-			sk->sk_error_report(sk);
+			bh_lock_sock(sk);
 
-			tcp_done(sk);
+			if (!sock_flag(sk, SOCK_DEAD)) {
+				smp_wmb();  /* be consistent with tcp_reset */
+				sk->sk_err = ETIMEDOUT;
+				sk->sk_error_report(sk);
+				tcp_done(sk);
+			}
+
+			bh_unlock_sock(sk);
 			local_bh_enable();
 			release_sock(sk);
 			sock_put(sk);