MPTCP Linux Development
 help / color / mirror / Atom feed
From: Shigeru Yoshida <syoshida@redhat.com>
To: mathew.j.martineau@linux.intel.com, matthieu.baerts@tessares.net
Cc: davem@davemloft.net, edumazet@google.com, kuba@kernel.org,
	pabeni@redhat.com, dmytro@shytyi.net, netdev@vger.kernel.org,
	mptcp@lists.linux.dev, linux-kernel@vger.kernel.org,
	Shigeru Yoshida <syoshida@redhat.com>
Subject: [PATCH] mptcp: Fix deadlock in mptcp_sendmsg()
Date: Fri,  6 Jan 2023 05:12:05 +0900	[thread overview]
Message-ID: <20230105201205.1087439-1-syoshida@redhat.com> (raw)

__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@redhat.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


             reply	other threads:[~2023-01-05 20:12 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-01-05 20:12 Shigeru Yoshida [this message]
2023-01-05 20:26 ` [PATCH] mptcp: Fix deadlock in mptcp_sendmsg() Mat Martineau
2023-01-05 21:00   ` Shigeru Yoshida

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20230105201205.1087439-1-syoshida@redhat.com \
    --to=syoshida@redhat.com \
    --cc=davem@davemloft.net \
    --cc=dmytro@shytyi.net \
    --cc=edumazet@google.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mathew.j.martineau@linux.intel.com \
    --cc=matthieu.baerts@tessares.net \
    --cc=mptcp@lists.linux.dev \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox