Squash-to: "mptcp: cleanup MPJ subflow list handling"

The self-tests in a loop triggered a UaF similar to:

#250

The critical scenario is actually almost fixed by:

"mptcp: cleanup MPJ subflow list handling"

with a notable exception: if an MPJ handshake races with
mptcp_close(), the subflow enter the join_list and __mptcp_finish_join()
is processed at the msk socket lock release in mptcp_close(),
the subflow will preserver a danfling reference to the msk sk_socket.

Address the issue fragting the subflow only on successful
__mptcp_finish_join()

Note that issues/250 triggers even before
"mptcp: cleanup MPJ subflow list handling", as before such commit the join
list was not spliced by mptcp_close(). We could consider a net-only patch to
address that.

Signed-off-by: Paolo Abeni <[email protected]>
Reviewed-by: Mat Martineau <[email protected]>
Signed-off-by: Matthieu Baerts <[email protected]>

net/mptcp/protocol.c

@@ -810,9 +810,17 @@ void mptcp_data_ready(struct sock *sk, struct sock *ssk)
 
 static bool __mptcp_finish_join(struct mptcp_sock *msk, struct sock *ssk)
 {
-	if (((struct sock *)msk)->sk_state != TCP_ESTABLISHED)
+	struct sock *sk = (struct sock *)msk;
+
+	if (sk->sk_state != TCP_ESTABLISHED)
 		return false;
 
+	/* 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_propagate_sndbuf((struct sock *)msk, ssk);
 	mptcp_sockopt_sync_locked(msk, ssk);
 	WRITE_ONCE(msk->allow_infinite_fallback, false);
@@ -3228,7 +3236,6 @@ bool mptcp_finish_join(struct sock *ssk)
 	struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
 	struct mptcp_sock *msk = mptcp_sk(subflow->conn);
 	struct sock *parent = (void *)msk;
-	struct socket *parent_sock;
 	bool ret = true;
 
 	pr_debug("msk=%p, subflow=%p", msk, subflow);
@@ -3272,13 +3279,6 @@ bool mptcp_finish_join(struct sock *ssk)
 		return false;
 	}
 
-	/* attach to msk socket only after we are sure he will deal with us
-	 * at close time
-	 */
-	parent_sock = READ_ONCE(parent->sk_socket);
-	if (parent_sock && !ssk->sk_socket)
-		mptcp_sock_graft(ssk, parent_sock);
-
 	subflow->map_seq = READ_ONCE(msk->ack_seq);
 
 out: