Skip to content

Commit

Permalink
mptcp: ensure listener is unhashed before updating the sk status
Browse files Browse the repository at this point in the history
commit 57fc0f1 upstream.

The MPTCP protocol access the listener subflow in a lockless
manner in a couple of places (poll, diag). That works only if
the msk itself leaves the listener status only after that the
subflow itself has been closed/disconnected. Otherwise we risk
deadlock in diag, as reported by Christoph.

Address the issue ensuring that the first subflow (the listener
one) is always disconnected before updating the msk socket status.

Reported-by: Christoph Paasch <cpaasch@apple.com>
Closes: multipath-tcp/mptcp_net-next#407
Fixes: b29fcfb ("mptcp: full disconnect implementation")
Cc: stable@vger.kernel.org
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>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
  • Loading branch information
Paolo Abeni authored and gregkh committed Jul 1, 2023
1 parent 42a018a commit 69925a3
Show file tree
Hide file tree
Showing 2 changed files with 21 additions and 6 deletions.
1 change: 1 addition & 0 deletions net/mptcp/pm_netlink.c
Original file line number Diff line number Diff line change
Expand Up @@ -1039,6 +1039,7 @@ static int mptcp_pm_nl_create_listen_socket(struct sock *sk,
return err;
}

inet_sk_state_store(newsk, TCP_LISTEN);
err = kernel_listen(ssock, backlog);
if (err) {
pr_warn("kernel_listen error, err=%d", err);
Expand Down
26 changes: 20 additions & 6 deletions net/mptcp/protocol.c
Original file line number Diff line number Diff line change
Expand Up @@ -2400,12 +2400,6 @@ static void __mptcp_close_ssk(struct sock *sk, struct sock *ssk,
kfree_rcu(subflow, rcu);
} else {
/* otherwise tcp will dispose of the ssk and subflow ctx */
if (ssk->sk_state == TCP_LISTEN) {
tcp_set_state(ssk, TCP_CLOSE);
mptcp_subflow_queue_clean(sk, ssk);
inet_csk_listen_stop(ssk);
}

__tcp_close(ssk, 0);

/* close acquired an extra ref */
Expand Down Expand Up @@ -2939,6 +2933,24 @@ static __poll_t mptcp_check_readable(struct mptcp_sock *msk)
return EPOLLIN | EPOLLRDNORM;
}

static void mptcp_check_listen_stop(struct sock *sk)
{
struct sock *ssk;

if (inet_sk_state_load(sk) != TCP_LISTEN)
return;

ssk = mptcp_sk(sk)->first;
if (WARN_ON_ONCE(!ssk || inet_sk_state_load(ssk) != TCP_LISTEN))
return;

lock_sock_nested(ssk, SINGLE_DEPTH_NESTING);
mptcp_subflow_queue_clean(sk, ssk);
inet_csk_listen_stop(ssk);
tcp_set_state(ssk, TCP_CLOSE);
release_sock(ssk);
}

bool __mptcp_close(struct sock *sk, long timeout)
{
struct mptcp_subflow_context *subflow;
Expand All @@ -2949,6 +2961,7 @@ bool __mptcp_close(struct sock *sk, long timeout)
WRITE_ONCE(sk->sk_shutdown, SHUTDOWN_MASK);

if ((1 << sk->sk_state) & (TCPF_LISTEN | TCPF_CLOSE)) {
mptcp_check_listen_stop(sk);
inet_sk_state_store(sk, TCP_CLOSE);
goto cleanup;
}
Expand Down Expand Up @@ -3062,6 +3075,7 @@ static int mptcp_disconnect(struct sock *sk, int flags)
if (msk->fastopening)
return -EBUSY;

mptcp_check_listen_stop(sk);
inet_sk_state_store(sk, TCP_CLOSE);

mptcp_stop_timer(sk);
Expand Down

0 comments on commit 69925a3

Please sign in to comment.