From 494ff5ec1dd60c1640fcb41a6b62259bf0d386ff Mon Sep 17 00:00:00 2001 From: Paolo Abeni Date: Thu, 16 Dec 2021 15:33:37 +0100 Subject: [PATCH] Squash-to: "mptcp: cleanup MPJ subflow list handling" The self-tests in a loop triggered a UaF similar to: https://github.com/multipath-tcp/mptcp_net-next/issues/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 Reviewed-by: Mat Martineau Signed-off-by: Matthieu Baerts --- net/mptcp/protocol.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index 862468d12cab7..234fa0dd63273 100644 --- a/net/mptcp/protocol.c +++ b/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: