mptcp.lists.linux.dev archive mirror
 help / color / mirror / Atom feed
* [PATCH net 0/2] mptcp: Fixes for v5.13
@ 2021-06-22  0:33 Mat Martineau
  2021-06-22  0:33 ` [PATCH net 1/2] mptcp: avoid race on msk state changes Mat Martineau
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Mat Martineau @ 2021-06-22  0:33 UTC (permalink / raw)
  To: netdev
  Cc: Mat Martineau, davem, kuba, matthieu.baerts, mptcp, pabeni, fw, dcaratti

Here are two MPTCP fixes from Paolo.

Patch 1 fixes some possible connect-time race conditions with
MPTCP-level connection state changes.

Patch 2 deletes a duplicate function declaration.


Paolo Abeni (2):
  mptcp: avoid race on msk state changes
  mptcp: drop duplicate mptcp_setsockopt() declaration

 net/mptcp/protocol.c |  5 +++++
 net/mptcp/protocol.h |  5 ++---
 net/mptcp/subflow.c  | 30 ++++++++++++++++++++++--------
 3 files changed, 29 insertions(+), 11 deletions(-)


base-commit: 0cd58e5c53babb9237b741dbef711f0a9eb6d3fd
-- 
2.32.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH net 1/2] mptcp: avoid race on msk state changes
  2021-06-22  0:33 [PATCH net 0/2] mptcp: Fixes for v5.13 Mat Martineau
@ 2021-06-22  0:33 ` Mat Martineau
  2021-06-22  0:33 ` [PATCH net 2/2] mptcp: drop duplicate mptcp_setsockopt() declaration Mat Martineau
  2021-06-22 17:40 ` [PATCH net 0/2] mptcp: Fixes for v5.13 patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: Mat Martineau @ 2021-06-22  0:33 UTC (permalink / raw)
  To: netdev
  Cc: Paolo Abeni, davem, kuba, matthieu.baerts, mptcp, fw, dcaratti,
	Mat Martineau

From: Paolo Abeni <pabeni@redhat.com>

The msk socket state is currently updated in a few spots without
owning the msk socket lock itself.

Some of such operations are safe, as they happens before exposing
the msk socket to user-space and can't race with other changes.

A couple of them, at connect time, can actually race with close()
or shutdown(), leaving breaking the socket state machine.

This change addresses the issue moving such update under the msk
socket lock with the usual:

<acquire spinlock>
<check sk lock onwers>
<ev defer to release_cb>

scheme.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/56
Fixes: 8fd738049ac3 ("mptcp: fallback in case of simultaneous connect")
Fixes: c3c123d16c0e ("net: mptcp: don't hang in mptcp_sendmsg() after TCP fallback")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 net/mptcp/protocol.c |  5 +++++
 net/mptcp/protocol.h |  2 ++
 net/mptcp/subflow.c  | 30 ++++++++++++++++++++++--------
 3 files changed, 29 insertions(+), 8 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 632350018fb6..8ead550df8b1 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -2946,6 +2946,11 @@ static void mptcp_release_cb(struct sock *sk)
 		spin_lock_bh(&sk->sk_lock.slock);
 	}
 
+	/* be sure to set the current sk state before tacking actions
+	 * depending on sk_state
+	 */
+	if (test_and_clear_bit(MPTCP_CONNECTED, &mptcp_sk(sk)->flags))
+		__mptcp_set_connected(sk);
 	if (test_and_clear_bit(MPTCP_CLEAN_UNA, &mptcp_sk(sk)->flags))
 		__mptcp_clean_una_wakeup(sk);
 	if (test_and_clear_bit(MPTCP_ERROR_REPORT, &mptcp_sk(sk)->flags))
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 5d7c44028e47..7b634568f49c 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -109,6 +109,7 @@
 #define MPTCP_ERROR_REPORT	8
 #define MPTCP_RETRANSMIT	9
 #define MPTCP_WORK_SYNC_SETSOCKOPT 10
+#define MPTCP_CONNECTED		11
 
 static inline bool before64(__u64 seq1, __u64 seq2)
 {
@@ -579,6 +580,7 @@ void mptcp_get_options(const struct sk_buff *skb,
 		       struct mptcp_options_received *mp_opt);
 
 void mptcp_finish_connect(struct sock *sk);
+void __mptcp_set_connected(struct sock *sk);
 static inline bool mptcp_is_fully_established(struct sock *sk)
 {
 	return inet_sk_state_load(sk) == TCP_ESTABLISHED &&
diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index 037fba41e170..9f934603bfe8 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -371,6 +371,24 @@ static bool subflow_use_different_dport(struct mptcp_sock *msk, const struct soc
 	return inet_sk(sk)->inet_dport != inet_sk((struct sock *)msk)->inet_dport;
 }
 
+void __mptcp_set_connected(struct sock *sk)
+{
+	if (sk->sk_state == TCP_SYN_SENT) {
+		inet_sk_state_store(sk, TCP_ESTABLISHED);
+		sk->sk_state_change(sk);
+	}
+}
+
+static void mptcp_set_connected(struct sock *sk)
+{
+	mptcp_data_lock(sk);
+	if (!sock_owned_by_user(sk))
+		__mptcp_set_connected(sk);
+	else
+		set_bit(MPTCP_CONNECTED, &mptcp_sk(sk)->flags);
+	mptcp_data_unlock(sk);
+}
+
 static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
 {
 	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
@@ -379,10 +397,6 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
 
 	subflow->icsk_af_ops->sk_rx_dst_set(sk, skb);
 
-	if (inet_sk_state_load(parent) == TCP_SYN_SENT) {
-		inet_sk_state_store(parent, TCP_ESTABLISHED);
-		parent->sk_state_change(parent);
-	}
 
 	/* be sure no special action on any packet other than syn-ack */
 	if (subflow->conn_finished)
@@ -411,6 +425,7 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
 			 subflow->remote_key);
 		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_MPCAPABLEACTIVEACK);
 		mptcp_finish_connect(sk);
+		mptcp_set_connected(parent);
 	} else if (subflow->request_join) {
 		u8 hmac[SHA256_DIGEST_SIZE];
 
@@ -451,6 +466,7 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
 	} else if (mptcp_check_fallback(sk)) {
 fallback:
 		mptcp_rcv_space_init(mptcp_sk(parent), sk);
+		mptcp_set_connected(parent);
 	}
 	return;
 
@@ -558,6 +574,7 @@ static void mptcp_sock_destruct(struct sock *sk)
 
 static void mptcp_force_close(struct sock *sk)
 {
+	/* the msk is not yet exposed to user-space */
 	inet_sk_state_store(sk, TCP_CLOSE);
 	sk_common_release(sk);
 }
@@ -1474,10 +1491,7 @@ static void subflow_state_change(struct sock *sk)
 		mptcp_rcv_space_init(mptcp_sk(parent), sk);
 		pr_fallback(mptcp_sk(parent));
 		subflow->conn_finished = 1;
-		if (inet_sk_state_load(parent) == TCP_SYN_SENT) {
-			inet_sk_state_store(parent, TCP_ESTABLISHED);
-			parent->sk_state_change(parent);
-		}
+		mptcp_set_connected(parent);
 	}
 
 	/* as recvmsg() does not acquire the subflow socket for ssk selection
-- 
2.32.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* [PATCH net 2/2] mptcp: drop duplicate mptcp_setsockopt() declaration
  2021-06-22  0:33 [PATCH net 0/2] mptcp: Fixes for v5.13 Mat Martineau
  2021-06-22  0:33 ` [PATCH net 1/2] mptcp: avoid race on msk state changes Mat Martineau
@ 2021-06-22  0:33 ` Mat Martineau
  2021-06-22 17:40 ` [PATCH net 0/2] mptcp: Fixes for v5.13 patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: Mat Martineau @ 2021-06-22  0:33 UTC (permalink / raw)
  To: netdev
  Cc: Paolo Abeni, davem, kuba, matthieu.baerts, mptcp, fw, dcaratti,
	Mat Martineau

From: Paolo Abeni <pabeni@redhat.com>

commit 7896248983ef ("mptcp: add skeleton to sync msk socket
options to subflows") introduced a duplicate declaration of
mptcp_setsockopt(), just drop it.

Reported-by: Florian Westphal <fw@strlen.de>
Fixes: 7896248983ef ("mptcp: add skeleton to sync msk socket options to subflows")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
---
 net/mptcp/protocol.h | 3 ---
 1 file changed, 3 deletions(-)

diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 7b634568f49c..78ac28902f55 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -762,9 +762,6 @@ unsigned int mptcp_pm_get_add_addr_accept_max(struct mptcp_sock *msk);
 unsigned int mptcp_pm_get_subflows_max(struct mptcp_sock *msk);
 unsigned int mptcp_pm_get_local_addr_max(struct mptcp_sock *msk);
 
-int mptcp_setsockopt(struct sock *sk, int level, int optname,
-		     sockptr_t optval, unsigned int optlen);
-
 void mptcp_sockopt_sync(struct mptcp_sock *msk, struct sock *ssk);
 void mptcp_sockopt_sync_all(struct mptcp_sock *msk);
 
-- 
2.32.0


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH net 0/2] mptcp: Fixes for v5.13
  2021-06-22  0:33 [PATCH net 0/2] mptcp: Fixes for v5.13 Mat Martineau
  2021-06-22  0:33 ` [PATCH net 1/2] mptcp: avoid race on msk state changes Mat Martineau
  2021-06-22  0:33 ` [PATCH net 2/2] mptcp: drop duplicate mptcp_setsockopt() declaration Mat Martineau
@ 2021-06-22 17:40 ` patchwork-bot+netdevbpf
  2 siblings, 0 replies; 4+ messages in thread
From: patchwork-bot+netdevbpf @ 2021-06-22 17:40 UTC (permalink / raw)
  To: Mat Martineau
  Cc: netdev, davem, kuba, matthieu.baerts, mptcp, pabeni, fw, dcaratti

Hello:

This series was applied to netdev/net.git (refs/heads/master):

On Mon, 21 Jun 2021 17:33:07 -0700 you wrote:
> Here are two MPTCP fixes from Paolo.
> 
> Patch 1 fixes some possible connect-time race conditions with
> MPTCP-level connection state changes.
> 
> Patch 2 deletes a duplicate function declaration.
> 
> [...]

Here is the summary with links:
  - [net,1/2] mptcp: avoid race on msk state changes
    https://git.kernel.org/netdev/net/c/490274b47468
  - [net,2/2] mptcp: drop duplicate mptcp_setsockopt() declaration
    https://git.kernel.org/netdev/net/c/597dbae77ee5

You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2021-06-22 17:40 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-06-22  0:33 [PATCH net 0/2] mptcp: Fixes for v5.13 Mat Martineau
2021-06-22  0:33 ` [PATCH net 1/2] mptcp: avoid race on msk state changes Mat Martineau
2021-06-22  0:33 ` [PATCH net 2/2] mptcp: drop duplicate mptcp_setsockopt() declaration Mat Martineau
2021-06-22 17:40 ` [PATCH net 0/2] mptcp: Fixes for v5.13 patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).