Later patches need to ensure that all MPJ subflows are grafted to the
msk socket before accept() completion.
Currently the grafting happens under the msk socket lock: potentially
at msk release_cb time which make satisfying the above condition a bit
tricky.
Move the MPJ subflow grafting earlier, under the msk data lock, so that
we can use such lock as a synchronization point.
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v3 -> v4:
- clarified it's not a fix
- move the graft under the msk socket lock
- no need to graft for active subflows
---
net/mptcp/protocol.c | 30 +++++++++++++++++++++++-------
1 file changed, 23 insertions(+), 7 deletions(-)
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 78ac8ba80e59..4a4cb9952596 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -933,12 +933,6 @@ static bool __mptcp_finish_join(struct mptcp_sock *msk, struct sock *ssk)
mptcp_subflow_joined(msk, ssk);
spin_unlock_bh(&msk->fallback_lock);
- /* attach to msk socket only after we are sure we will deal with it
- * at close time
- */
- if (sk->sk_socket && !ssk->sk_socket)
- mptcp_sock_graft(ssk, sk->sk_socket);
-
mptcp_subflow_ctx(ssk)->subflow_id = msk->subflow_id++;
mptcp_sockopt_sync_locked(msk, ssk);
mptcp_stop_tout_timer(sk);
@@ -3760,6 +3754,20 @@ void mptcp_sock_graft(struct sock *sk, struct socket *parent)
write_unlock_bh(&sk->sk_callback_lock);
}
+/* Can be called without holding the msk socket lock; use the callback lock
+ * to avoid {READ_,WRITE_}ONCE annotations on sk_socket.
+ */
+static void mptcp_sock_check_graft(struct sock *sk, struct sock *ssk)
+{
+ struct socket *sock;
+
+ write_lock_bh(&sk->sk_callback_lock);
+ sock = sk->sk_socket;
+ write_unlock_bh(&sk->sk_callback_lock);
+ if (sock)
+ mptcp_sock_graft(ssk, sock);
+}
+
bool mptcp_finish_join(struct sock *ssk)
{
struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
@@ -3775,7 +3783,9 @@ bool mptcp_finish_join(struct sock *ssk)
return false;
}
- /* active subflow, already present inside the conn_list */
+ /* Active subflow, already present inside the conn_list; is grafted
+ * either by __mptcp_subflow_connect() or accept.
+ */
if (!list_empty(&subflow->node)) {
spin_lock_bh(&msk->fallback_lock);
if (!msk->allow_subflows) {
@@ -3802,11 +3812,17 @@ bool mptcp_finish_join(struct sock *ssk)
if (ret) {
sock_hold(ssk);
list_add_tail(&subflow->node, &msk->conn_list);
+ mptcp_sock_check_graft(parent, ssk);
}
} else {
sock_hold(ssk);
list_add_tail(&subflow->node, &msk->join_list);
__set_bit(MPTCP_FLUSH_JOIN_LIST, &msk->cb_flags);
+
+ /* In case of later failures, __mptcp_flush_join_list() will
+ * properly orphan the ssk via mptcp_close_ssk().
+ */
+ mptcp_sock_check_graft(parent, ssk);
}
mptcp_data_unlock(parent);
--
2.51.1