From ef59b1953c26130ef23e6986b2ceea1efaa472e9 Mon Sep 17 00:00:00 2001 From: Mat Martineau Date: Mon, 21 Sep 2020 16:57:58 +0200 Subject: mptcp: Wake up MPTCP worker when DATA_FIN found on a TCP FIN packet When receiving a DATA_FIN MPTCP option on a TCP FIN packet, the DATA_FIN information would be stored but the MPTCP worker did not get scheduled. In turn, the MPTCP socket state would remain in TCP_ESTABLISHED and no blocked operations would be awakened. TCP FIN packets are seen by the MPTCP socket when moving skbs out of the subflow receive queues, so schedule the MPTCP worker when a skb with DATA_FIN but no data payload is moved from a subflow queue. Other cases (DATA_FIN on a bare TCP ACK or on a packet with data payload) are already handled. Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/84 Fixes: 43b54c6ee382 ("mptcp: Use full MPTCP-level disconnect state machine") Acked-by: Paolo Abeni Signed-off-by: Mat Martineau Signed-off-by: Matthieu Baerts Signed-off-by: David S. Miller --- net/mptcp/subflow.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'net/mptcp') diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index 9ead43f79023..8cbeb68f3775 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -731,7 +731,7 @@ static enum mapping_status get_mapping_status(struct sock *ssk, if (mpext->data_fin == 1) { if (data_len == 1) { - mptcp_update_rcv_data_fin(msk, mpext->data_seq); + bool updated = mptcp_update_rcv_data_fin(msk, mpext->data_seq); pr_debug("DATA_FIN with no payload seq=%llu", mpext->data_seq); if (subflow->map_valid) { /* A DATA_FIN might arrive in a DSS @@ -742,6 +742,9 @@ static enum mapping_status get_mapping_status(struct sock *ssk, skb_ext_del(skb, SKB_EXT_MPTCP); return MAPPING_OK; } else { + if (updated && schedule_work(&msk->work)) + sock_hold((struct sock *)msk); + return MAPPING_DATA_FIN; } } else { -- cgit v1.2.3 From 917944da3bfc7cb5ac3af26725af3371d3a12db0 Mon Sep 17 00:00:00 2001 From: Mat Martineau Date: Tue, 29 Sep 2020 15:08:19 -0700 Subject: mptcp: Consistently use READ_ONCE/WRITE_ONCE with msk->ack_seq The msk->ack_seq value is sometimes read without the msk lock held, so make proper use of READ_ONCE and WRITE_ONCE. Signed-off-by: Mat Martineau Signed-off-by: David S. Miller --- net/mptcp/options.c | 4 ++-- net/mptcp/protocol.c | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) (limited to 'net/mptcp') diff --git a/net/mptcp/options.c b/net/mptcp/options.c index 7fa822b55c34..120ef39fe589 100644 --- a/net/mptcp/options.c +++ b/net/mptcp/options.c @@ -518,11 +518,11 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb, if (subflow->use_64bit_ack) { ack_size = TCPOLEN_MPTCP_DSS_ACK64; - opts->ext_copy.data_ack = msk->ack_seq; + opts->ext_copy.data_ack = READ_ONCE(msk->ack_seq); opts->ext_copy.ack64 = 1; } else { ack_size = TCPOLEN_MPTCP_DSS_ACK32; - opts->ext_copy.data_ack32 = (uint32_t)(msk->ack_seq); + opts->ext_copy.data_ack32 = (uint32_t)READ_ONCE(msk->ack_seq); opts->ext_copy.ack64 = 0; } opts->ext_copy.use_ack = 1; diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c index 365ba96c84b0..5d747c6a610e 100644 --- a/net/mptcp/protocol.c +++ b/net/mptcp/protocol.c @@ -123,7 +123,7 @@ static void __mptcp_move_skb(struct mptcp_sock *msk, struct sock *ssk, skb_ext_reset(skb); skb_orphan(skb); - msk->ack_seq += copy_len; + WRITE_ONCE(msk->ack_seq, msk->ack_seq + copy_len); tail = skb_peek_tail(&sk->sk_receive_queue); if (offset == 0 && tail) { @@ -261,7 +261,7 @@ static void mptcp_check_data_fin(struct sock *sk) if (mptcp_pending_data_fin(sk, &rcv_data_fin_seq)) { struct mptcp_subflow_context *subflow; - msk->ack_seq++; + WRITE_ONCE(msk->ack_seq, msk->ack_seq + 1); WRITE_ONCE(msk->rcv_data_fin, 0); sk->sk_shutdown |= RCV_SHUTDOWN; @@ -1720,7 +1720,7 @@ struct sock *mptcp_sk_clone(const struct sock *sk, msk->remote_key = mp_opt->sndr_key; mptcp_crypto_key_sha(msk->remote_key, NULL, &ack_seq); ack_seq++; - msk->ack_seq = ack_seq; + WRITE_ONCE(msk->ack_seq, ack_seq); } sock_reset_flag(nsk, SOCK_RCU_FREE); @@ -2072,7 +2072,7 @@ bool mptcp_finish_join(struct sock *sk) parent_sock = READ_ONCE(parent->sk_socket); if (parent_sock && !sk->sk_socket) mptcp_sock_graft(sk, parent_sock); - subflow->map_seq = msk->ack_seq; + subflow->map_seq = READ_ONCE(msk->ack_seq); return true; } -- cgit v1.2.3 From 1a49b2c2a501467587504e75bd049d43af38b692 Mon Sep 17 00:00:00 2001 From: Mat Martineau Date: Tue, 29 Sep 2020 15:08:20 -0700 Subject: mptcp: Handle incoming 32-bit DATA_FIN values The peer may send a DATA_FIN mapping with either a 32-bit or 64-bit sequence number. When a 32-bit sequence number is received for the DATA_FIN, it must be expanded to 64 bits before comparing it to the last acked sequence number. This expansion was missing. Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/93 Fixes: 3721b9b64676 ("mptcp: Track received DATA_FIN sequence number and add related helpers") Signed-off-by: Mat Martineau Signed-off-by: David S. Miller --- net/mptcp/options.c | 7 ++++--- net/mptcp/protocol.h | 2 +- net/mptcp/subflow.c | 16 +++++++++++++--- 3 files changed, 18 insertions(+), 7 deletions(-) (limited to 'net/mptcp') diff --git a/net/mptcp/options.c b/net/mptcp/options.c index 120ef39fe589..afa486912f5a 100644 --- a/net/mptcp/options.c +++ b/net/mptcp/options.c @@ -782,7 +782,7 @@ static void update_una(struct mptcp_sock *msk, } } -bool mptcp_update_rcv_data_fin(struct mptcp_sock *msk, u64 data_fin_seq) +bool mptcp_update_rcv_data_fin(struct mptcp_sock *msk, u64 data_fin_seq, bool use_64bit) { /* Skip if DATA_FIN was already received. * If updating simultaneously with the recvmsg loop, values @@ -792,7 +792,8 @@ bool mptcp_update_rcv_data_fin(struct mptcp_sock *msk, u64 data_fin_seq) if (READ_ONCE(msk->rcv_data_fin) || !READ_ONCE(msk->first)) return false; - WRITE_ONCE(msk->rcv_data_fin_seq, data_fin_seq); + WRITE_ONCE(msk->rcv_data_fin_seq, + expand_ack(READ_ONCE(msk->ack_seq), data_fin_seq, use_64bit)); WRITE_ONCE(msk->rcv_data_fin, 1); return true; @@ -875,7 +876,7 @@ void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb, */ if (TCP_SKB_CB(skb)->seq == TCP_SKB_CB(skb)->end_seq) { if (mp_opt.data_fin && mp_opt.data_len == 1 && - mptcp_update_rcv_data_fin(msk, mp_opt.data_seq) && + mptcp_update_rcv_data_fin(msk, mp_opt.data_seq, mp_opt.dsn64) && schedule_work(&msk->work)) sock_hold(subflow->conn); diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index 60b27d44c184..20f04ac85409 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -387,7 +387,7 @@ void mptcp_data_ready(struct sock *sk, struct sock *ssk); bool mptcp_finish_join(struct sock *sk); void mptcp_data_acked(struct sock *sk); void mptcp_subflow_eof(struct sock *sk); -bool mptcp_update_rcv_data_fin(struct mptcp_sock *msk, u64 data_fin_seq); +bool mptcp_update_rcv_data_fin(struct mptcp_sock *msk, u64 data_fin_seq, bool use_64bit); void __init mptcp_token_init(void); static inline void mptcp_token_init_request(struct request_sock *req) diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c index 8cbeb68f3775..5f2fa935022d 100644 --- a/net/mptcp/subflow.c +++ b/net/mptcp/subflow.c @@ -731,7 +731,8 @@ static enum mapping_status get_mapping_status(struct sock *ssk, if (mpext->data_fin == 1) { if (data_len == 1) { - bool updated = mptcp_update_rcv_data_fin(msk, mpext->data_seq); + bool updated = mptcp_update_rcv_data_fin(msk, mpext->data_seq, + mpext->dsn64); pr_debug("DATA_FIN with no payload seq=%llu", mpext->data_seq); if (subflow->map_valid) { /* A DATA_FIN might arrive in a DSS @@ -748,8 +749,17 @@ static enum mapping_status get_mapping_status(struct sock *ssk, return MAPPING_DATA_FIN; } } else { - mptcp_update_rcv_data_fin(msk, mpext->data_seq + data_len); - pr_debug("DATA_FIN with mapping seq=%llu", mpext->data_seq + data_len); + u64 data_fin_seq = mpext->data_seq + data_len; + + /* If mpext->data_seq is a 32-bit value, data_fin_seq + * must also be limited to 32 bits. + */ + if (!mpext->dsn64) + data_fin_seq &= GENMASK_ULL(31, 0); + + mptcp_update_rcv_data_fin(msk, data_fin_seq, mpext->dsn64); + pr_debug("DATA_FIN with mapping seq=%llu dsn64=%d", + data_fin_seq, mpext->dsn64); } /* Adjust for DATA_FIN using 1 byte of sequence space */ -- cgit v1.2.3