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>
This commit is contained in:
parent
30cac4c462
commit
c5ee2b07c1
|
@ -3585,14 +3585,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);
|
||||
|
|
Loading…
Reference in New Issue