netdev - [PATCH] mptcp: Fix deadlock in mptcp

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-Id: <20230105201205.1087439-1-syoshida@redhat.com>
Date:   Fri,  6 Jan 2023 05:12:05 +0900
From:   Shigeru Yoshida <syoshida@...hat.com>
To:     mathew.j.martineau@...ux.intel.com, matthieu.baerts@...sares.net
Cc:     davem@...emloft.net, edumazet@...gle.com, kuba@...nel.org,
        pabeni@...hat.com, dmytro@...tyi.net, netdev@...r.kernel.org,
        mptcp@...ts.linux.dev, linux-kernel@...r.kernel.org,
        Shigeru Yoshida <syoshida@...hat.com>
Subject: [PATCH] mptcp: Fix deadlock in mptcp_sendmsg()

__mptcp_close_ssk() can be called from mptcp_sendmsg() with subflow
socket locked.  This can cause a deadlock as below:

mptcp_sendmsg()
  mptcp_sendmsg_fastopen() --> lock ssk
    tcp_sendmsg_fastopen()
       __inet_stream_connect()
          mptcp_disconnect()
             mptcp_destroy_common()
                __mptcp_close_ssk() --> lock ssk again

This patch fixes the issue by skipping locking for subflow socket
which is already locked.

Fixes: d98a82a6afc7 ("mptcp: handle defer connect in mptcp_sendmsg")
Signed-off-by: Shigeru Yoshida <syoshida@...hat.com>
---
 net/mptcp/protocol.c | 15 +++++++++------
 net/mptcp/protocol.h |  4 ++--
 2 files changed, 11 insertions(+), 8 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index f6f93957275b..979265f66082 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1672,9 +1672,9 @@ static int mptcp_sendmsg_fastopen(struct sock *sk, struct sock *ssk, struct msgh
 	lock_sock(ssk);
 	msg->msg_flags |= MSG_DONTWAIT;
 	msk->connect_flags = O_NONBLOCK;
-	msk->is_sendmsg = 1;
+	msk->sendmsg_locked_sk = ssk;
 	ret = tcp_sendmsg_fastopen(ssk, msg, copied_syn, len, NULL);
-	msk->is_sendmsg = 0;
+	msk->sendmsg_locked_sk = NULL;
 	msg->msg_flags = saved_flags;
 	release_sock(ssk);
 
@@ -2319,7 +2319,8 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
 	if (dispose_it)
 		list_del(&subflow->node);
 
-	lock_sock_nested(ssk, SINGLE_DEPTH_NESTING);
+	if (msk->sendmsg_locked_sk != ssk)
+		lock_sock_nested(ssk, SINGLE_DEPTH_NESTING);
 
 	if (flags & MPTCP_CF_FASTCLOSE) {
 		/* be sure to force the tcp_disconnect() path,
@@ -2335,7 +2336,8 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
 		tcp_disconnect(ssk, 0);
 		msk->subflow->state = SS_UNCONNECTED;
 		mptcp_subflow_ctx_reset(subflow);
-		release_sock(ssk);
+		if (msk->sendmsg_locked_sk != ssk)
+			release_sock(ssk);
 
 		goto out;
 	}
@@ -2362,7 +2364,8 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
 		/* close acquired an extra ref */
 		__sock_put(ssk);
 	}
-	release_sock(ssk);
+	if (msk->sendmsg_locked_sk != ssk)
+		release_sock(ssk);
 
 	sock_put(ssk);
 
@@ -3532,7 +3535,7 @@ static int mptcp_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
 	/* if reaching here via the fastopen/sendmsg path, the caller already
 	 * acquired the subflow socket lock, too.
 	 */
-	if (msk->is_sendmsg)
+	if (msk->sendmsg_locked_sk)
 		err = __inet_stream_connect(ssock, uaddr, addr_len, msk->connect_flags, 1);
 	else
 		err = inet_stream_connect(ssock, uaddr, addr_len, msk->connect_flags);
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 955fb3d88eb3..43afc399e16b 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -294,8 +294,7 @@ struct mptcp_sock {
 	u8		mpc_endpoint_id;
 	u8		recvmsg_inq:1,
 			cork:1,
-			nodelay:1,
-			is_sendmsg:1;
+			nodelay:1;
 	int		connect_flags;
 	struct work_struct work;
 	struct sk_buff  *ooo_last_skb;
@@ -318,6 +317,7 @@ struct mptcp_sock {
 	u32 setsockopt_seq;
 	char		ca_name[TCP_CA_NAME_MAX];
 	struct mptcp_sock	*dl_next;
+	struct sock	*sendmsg_locked_sk;
 };
 
 #define mptcp_data_lock(sk) spin_lock_bh(&(sk)->sk_lock.slock)
-- 
2.39.0