Skip to content

Commit 30e51b9

Browse files
menglongdongkuba-moo
authored andcommitted
mptcp: fix unreleased socket in accept queue
The mptcp socket and its subflow sockets in accept queue can't be released after the process exit. While the release of a mptcp socket in listening state, the corresponding tcp socket will be released too. Meanwhile, the tcp socket in the unaccept queue will be released too. However, only init subflow is in the unaccept queue, and the joined subflow is not in the unaccept queue, which makes the joined subflow won't be released, and therefore the corresponding unaccepted mptcp socket will not be released to. This can be reproduced easily with following steps: 1. create 2 namespace and veth: $ ip netns add mptcp-client $ ip netns add mptcp-server $ sysctl -w net.ipv4.conf.all.rp_filter=0 $ ip netns exec mptcp-client sysctl -w net.mptcp.enabled=1 $ ip netns exec mptcp-server sysctl -w net.mptcp.enabled=1 $ ip link add red-client netns mptcp-client type veth peer red-server \ netns mptcp-server $ ip -n mptcp-server address add 10.0.0.1/24 dev red-server $ ip -n mptcp-server address add 192.168.0.1/24 dev red-server $ ip -n mptcp-client address add 10.0.0.2/24 dev red-client $ ip -n mptcp-client address add 192.168.0.2/24 dev red-client $ ip -n mptcp-server link set red-server up $ ip -n mptcp-client link set red-client up 2. configure the endpoint and limit for client and server: $ ip -n mptcp-server mptcp endpoint flush $ ip -n mptcp-server mptcp limits set subflow 2 add_addr_accepted 2 $ ip -n mptcp-client mptcp endpoint flush $ ip -n mptcp-client mptcp limits set subflow 2 add_addr_accepted 2 $ ip -n mptcp-client mptcp endpoint add 192.168.0.2 dev red-client id \ 1 subflow 3. listen and accept on a port, such as 9999. The nc command we used here is modified, which makes it use mptcp protocol by default. $ ip netns exec mptcp-server nc -l -k -p 9999 4. open another *two* terminal and use each of them to connect to the server with the following command: $ ip netns exec mptcp-client nc 10.0.0.1 9999 Input something after connect to trigger the connection of the second subflow. So that there are two established mptcp connections, with the second one still unaccepted. 5. exit all the nc command, and check the tcp socket in server namespace. And you will find that there is one tcp socket in CLOSE_WAIT state and can't release forever. Fix this by closing all of the unaccepted mptcp socket in mptcp_subflow_queue_clean() with __mptcp_close(). Now, we can ensure that all unaccepted mptcp sockets will be cleaned by __mptcp_close() before they are released, so mptcp_sock_destruct(), which is used to clean the unaccepted mptcp socket, is not needed anymore. The selftests for mptcp is ran for this commit, and no new failures. Fixes: f296234 ("mptcp: Add handling of incoming MP_JOIN requests") Fixes: 6aeed90 ("mptcp: fix race on unaccepted mptcp sockets") Cc: stable@vger.kernel.org Reviewed-by: Jiang Biao <benbjiang@tencent.com> Reviewed-by: Mengen Sun <mengensun@tencent.com> Acked-by: Paolo Abeni <pabeni@redhat.com> Signed-off-by: Menglong Dong <imagedong@tencent.com> Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com> Signed-off-by: Jakub Kicinski <kuba@kernel.org>
1 parent 26d3e21 commit 30e51b9

File tree

3 files changed

+9
-27
lines changed

3 files changed

+9
-27
lines changed

net/mptcp/protocol.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -2662,7 +2662,7 @@ static void __mptcp_clear_xmit(struct sock *sk)
26622662
dfrag_clear(sk, dfrag);
26632663
}
26642664

2665-
static void mptcp_cancel_work(struct sock *sk)
2665+
void mptcp_cancel_work(struct sock *sk)
26662666
{
26672667
struct mptcp_sock *msk = mptcp_sk(sk);
26682668

net/mptcp/protocol.h

+1
Original file line numberDiff line numberDiff line change
@@ -613,6 +613,7 @@ void mptcp_subflow_queue_clean(struct sock *ssk);
613613
void mptcp_sock_graft(struct sock *sk, struct socket *parent);
614614
struct socket *__mptcp_nmpc_socket(const struct mptcp_sock *msk);
615615
bool __mptcp_close(struct sock *sk, long timeout);
616+
void mptcp_cancel_work(struct sock *sk);
616617

617618
bool mptcp_addresses_equal(const struct mptcp_addr_info *a,
618619
const struct mptcp_addr_info *b, bool use_port);

net/mptcp/subflow.c

+7-26
Original file line numberDiff line numberDiff line change
@@ -602,30 +602,6 @@ static bool subflow_hmac_valid(const struct request_sock *req,
602602
return !crypto_memneq(hmac, mp_opt->hmac, MPTCPOPT_HMAC_LEN);
603603
}
604604

605-
static void mptcp_sock_destruct(struct sock *sk)
606-
{
607-
/* if new mptcp socket isn't accepted, it is free'd
608-
* from the tcp listener sockets request queue, linked
609-
* from req->sk. The tcp socket is released.
610-
* This calls the ULP release function which will
611-
* also remove the mptcp socket, via
612-
* sock_put(ctx->conn).
613-
*
614-
* Problem is that the mptcp socket will be in
615-
* ESTABLISHED state and will not have the SOCK_DEAD flag.
616-
* Both result in warnings from inet_sock_destruct.
617-
*/
618-
if ((1 << sk->sk_state) & (TCPF_ESTABLISHED | TCPF_CLOSE_WAIT)) {
619-
sk->sk_state = TCP_CLOSE;
620-
WARN_ON_ONCE(sk->sk_socket);
621-
sock_orphan(sk);
622-
}
623-
624-
/* We don't need to clear msk->subflow, as it's still NULL at this point */
625-
mptcp_destroy_common(mptcp_sk(sk), 0);
626-
inet_sock_destruct(sk);
627-
}
628-
629605
static void mptcp_force_close(struct sock *sk)
630606
{
631607
/* the msk is not yet exposed to user-space */
@@ -768,7 +744,6 @@ static struct sock *subflow_syn_recv_sock(const struct sock *sk,
768744
/* new mpc subflow takes ownership of the newly
769745
* created mptcp socket
770746
*/
771-
new_msk->sk_destruct = mptcp_sock_destruct;
772747
mptcp_sk(new_msk)->setsockopt_seq = ctx->setsockopt_seq;
773748
mptcp_pm_new_connection(mptcp_sk(new_msk), child, 1);
774749
mptcp_token_accept(subflow_req, mptcp_sk(new_msk));
@@ -1763,13 +1738,19 @@ void mptcp_subflow_queue_clean(struct sock *listener_ssk)
17631738

17641739
for (msk = head; msk; msk = next) {
17651740
struct sock *sk = (struct sock *)msk;
1766-
bool slow;
1741+
bool slow, do_cancel_work;
17671742

1743+
sock_hold(sk);
17681744
slow = lock_sock_fast_nested(sk);
17691745
next = msk->dl_next;
17701746
msk->first = NULL;
17711747
msk->dl_next = NULL;
1748+
1749+
do_cancel_work = __mptcp_close(sk, 0);
17721750
unlock_sock_fast(sk, slow);
1751+
if (do_cancel_work)
1752+
mptcp_cancel_work(sk);
1753+
sock_put(sk);
17731754
}
17741755

17751756
/* we are still under the listener msk socket lock */

0 commit comments

Comments
 (0)