All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.