All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthieu Baerts <matthieu.baerts@tessares.net>
To: mptcp@lists.linux.dev, "David S. Miller" <davem@davemloft.net>,
	 Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>,
	 Paolo Abeni <pabeni@redhat.com>,
	Mat Martineau <martineau@kernel.org>,
	 Jiang Biao <benbjiang@tencent.com>,
	Menglong Dong <imagedong@tencent.com>,
	 Mengen Sun <mengensun@tencent.com>,
	Shuah Khan <shuah@kernel.org>,  Florian Westphal <fw@strlen.de>
Cc: netdev@vger.kernel.org, linux-kernel@vger.kernel.org,
	 linux-kselftest@vger.kernel.org,
	 Matthieu Baerts <matthieu.baerts@tessares.net>,
	 Christoph Paasch <cpaasch@apple.com>,
	stable@vger.kernel.org
Subject: [PATCH net v2 1/8] mptcp: fix possible deadlock in subflow_error_report
Date: Thu, 09 Mar 2023 15:49:57 +0100	[thread overview]
Message-ID: <20230227-upstream-net-20230227-mptcp-fixes-v2-1-47c2e95eada9@tessares.net> (raw)
In-Reply-To: <20230227-upstream-net-20230227-mptcp-fixes-v2-0-47c2e95eada9@tessares.net>

From: Paolo Abeni <pabeni@redhat.com>

Christoph reported a possible deadlock while the TCP stack
destroys an unaccepted subflow due to an incoming reset: the
MPTCP socket error path tries to acquire the msk-level socket
lock while TCP still owns the listener socket accept queue
spinlock, and the reverse dependency already exists in the
TCP stack.

Note that the above is actually a lockdep false positive, as
the chain involves two separate sockets. A different per-socket
lockdep key will address the issue, but such a change will be
quite invasive.

Instead, we can simply stop earlier the socket error handling
for orphaned or unaccepted subflows, breaking the critical
lockdep chain. Error handling in such a scenario is a no-op.

Reported-and-tested-by: Christoph Paasch <cpaasch@apple.com>
Fixes: 15cc10453398 ("mptcp: deliver ssk errors to msk")
Cc: stable@vger.kernel.org
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/355
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts <matthieu.baerts@tessares.net>
Signed-off-by: Matthieu Baerts <matthieu.baerts@tessares.net>
---
 net/mptcp/subflow.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 4ae1a7304cf0..5070dc33675d 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1432,6 +1432,13 @@ static void subflow_error_report(struct sock *ssk)
 {
 	struct sock *sk = mptcp_subflow_ctx(ssk)->conn;
 
+	/* bail early if this is a no-op, so that we avoid introducing a
+	 * problematic lockdep dependency between TCP accept queue lock
+	 * and msk socket spinlock
+	 */
+	if (!sk->sk_socket)
+		return;
+
 	mptcp_data_lock(sk);
 	if (!sock_owned_by_user(sk))
 		__mptcp_error_report(sk);

-- 
2.39.2


  reply	other threads:[~2023-03-09 14:50 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-03-09 14:49 [PATCH net v2 0/8] mptcp: fixes for 6.3 Matthieu Baerts
2023-03-09 14:49 ` Matthieu Baerts [this message]
2023-03-09 14:49 ` [PATCH net v2 2/8] mptcp: refactor passive socket initialization Matthieu Baerts
2023-03-09 14:49 ` [PATCH net v2 3/8] mptcp: use the workqueue to destroy unaccepted sockets Matthieu Baerts
2023-03-09 14:50 ` [PATCH net v2 4/8] mptcp: fix UaF in listener shutdown Matthieu Baerts
2023-03-09 14:50 ` [PATCH net v2 5/8] selftests: mptcp: userspace pm: fix printed values Matthieu Baerts
2023-03-09 14:50 ` [PATCH net v2 6/8] mptcp: add ro_after_init for tcp{,v6}_prot_override Matthieu Baerts
2023-03-09 14:50 ` [PATCH net v2 7/8] mptcp: avoid setting TCP_CLOSE state twice Matthieu Baerts
2023-03-09 14:50 ` [PATCH net v2 8/8] mptcp: fix lockdep false positive in mptcp_pm_nl_create_listen_socket() Matthieu Baerts
2023-03-11  5:42 ` [PATCH net v2 0/8] mptcp: fixes for 6.3 Jakub Kicinski
2023-03-11  5:50 ` patchwork-bot+netdevbpf

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=20230227-upstream-net-20230227-mptcp-fixes-v2-1-47c2e95eada9@tessares.net \
    --to=matthieu.baerts@tessares.net \
    --cc=benbjiang@tencent.com \
    --cc=cpaasch@apple.com \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=fw@strlen.de \
    --cc=imagedong@tencent.com \
    --cc=kuba@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=martineau@kernel.org \
    --cc=mengensun@tencent.com \
    --cc=mptcp@lists.linux.dev \
    --cc=netdev@vger.kernel.org \
    --cc=pabeni@redhat.com \
    --cc=shuah@kernel.org \
    --cc=stable@vger.kernel.org \
    /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.