mptcp: Fix unlocking-check on wrong socket

syzkaller found its way into triggering inet_csk_reqsk_queue_add() with a listener that is no more listening. In that case we unlock the master-socket to avoid having to do the unlock-magic in mptcp_disconnect(), that we removed with commit b39aafe ("mptcp: fix master unlock race in mptcp_disconnect"). A typo found its way into the code though. We need to check the child-socket, not the sk-socket. Also, we need to make sure this actually is a TCP-socket. Because inet_csk_reqsk_queue_add can be called from DCCP. That caused us to never unlock the master-socket. Adding a BUG_ON() as well, as it helped tremenduosly to debug this case as otherwise any bugs happen much later. Cc: Tim Froidcoeur <[email protected]> Fixes: b39aafe ("mptcp: fix master unlock race in mptcp_disconnect") Signed-off-by: Christoph Paasch <[email protected]> Signed-off-by: Matthieu Baerts <[email protected]> (cherry picked from commit cb954b5) Signed-off-by: Matthieu Baerts <[email protected]> (cherry picked from commit 3fdb0ed) Signed-off-by: Matthieu Baerts <[email protected]> (cherry picked from commit 8cb79b5) Signed-off-by: Matthieu Baerts <[email protected]>
multipath-tcp · Nov 4, 2022 · 49eb5c1 · 49eb5c1
1 parent 1c6edd5
commit 49eb5c1
Show file tree

Hide file tree

Showing 2 changed files with 10 additions and 6 deletions.
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
@@ -965,13 +965,15 @@ struct sock *inet_csk_reqsk_queue_add(struct sock *sk,
 
 	spin_lock(&queue->rskq_lock);
 	if (unlikely(sk->sk_state != TCP_LISTEN)) {
-		struct tcp_sock *tp = tcp_sk(sk);
+		if (sk->sk_protocol == IPPROTO_TCP) {
+			struct tcp_sock *child_tp = tcp_sk(child);
 
-		/* in case of mptcp, two locks may been taken, one
-		 * on the meta, the other on master_sk
-		 */
-		if (mptcp(tp) && tp->mpcb && tp->mpcb->master_sk)
-			bh_unlock_sock(tp->mpcb->master_sk);
+			/* in case of mptcp, two locks may been taken, one
+			 * on the meta, the other on master_sk
+			 */
+			if (mptcp(child_tp) && child_tp->mpcb && child_tp->mpcb->master_sk)
+				bh_unlock_sock(child_tp->mpcb->master_sk);
+		}
 
 		inet_child_forget(sk, req, child);
 		child = NULL;

diff --git a/net/mptcp/mptcp_ctrl.c b/net/mptcp/mptcp_ctrl.c
@@ -1915,6 +1915,8 @@ void mptcp_disconnect(struct sock *meta_sk)
 
 	local_bh_disable();
 	mptcp_for_each_sk_safe(meta_tp->mpcb, subsk, tmpsk) {
+		BUG_ON(spin_is_locked(&subsk->sk_lock.slock));
+
 		tcp_sk(subsk)->tcp_disconnect = 1;
 
 		meta_sk->sk_prot->disconnect(subsk, O_NONBLOCK);