Commit 2b3e981a authored by David S. Miller's avatar David S. Miller

Merge branch 'mptcp-Fix-for-32-bit-DATA_FIN'

Mat Martineau says:

====================
mptcp: Fix for 32-bit DATA_FIN

The main fix is contained in patch 2, and that commit message explains
the issue with not properly converting truncated DATA_FIN sequence
numbers sent by the peer.

With patch 2 adding an unlocked read of msk->ack_seq, patch 1 cleans up
access to that data with READ_ONCE/WRITE_ONCE.

This does introduce two merge conflicts with net-next, but both have
straightforward resolution. Patch 1 modifies a line that got removed in
net-next so the modification can be dropped when merging. Patch 2 will
require a trivial conflict resolution for a modified function
declaration.
====================
Signed-off-by: default avatarDavid S. Miller <davem@davemloft.net>
parents 4972c6cc 1a49b2c2
...@@ -518,11 +518,11 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb, ...@@ -518,11 +518,11 @@ static bool mptcp_established_options_dss(struct sock *sk, struct sk_buff *skb,
if (subflow->use_64bit_ack) { if (subflow->use_64bit_ack) {
ack_size = TCPOLEN_MPTCP_DSS_ACK64; 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; opts->ext_copy.ack64 = 1;
} else { } else {
ack_size = TCPOLEN_MPTCP_DSS_ACK32; 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.ack64 = 0;
} }
opts->ext_copy.use_ack = 1; opts->ext_copy.use_ack = 1;
...@@ -782,7 +782,7 @@ static void update_una(struct mptcp_sock *msk, ...@@ -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. /* Skip if DATA_FIN was already received.
* If updating simultaneously with the recvmsg loop, values * 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) ...@@ -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)) if (READ_ONCE(msk->rcv_data_fin) || !READ_ONCE(msk->first))
return false; 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); WRITE_ONCE(msk->rcv_data_fin, 1);
return true; return true;
...@@ -875,7 +876,7 @@ void mptcp_incoming_options(struct sock *sk, struct sk_buff *skb, ...@@ -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 (TCP_SKB_CB(skb)->seq == TCP_SKB_CB(skb)->end_seq) {
if (mp_opt.data_fin && mp_opt.data_len == 1 && 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)) schedule_work(&msk->work))
sock_hold(subflow->conn); sock_hold(subflow->conn);
......
...@@ -123,7 +123,7 @@ static void __mptcp_move_skb(struct mptcp_sock *msk, struct sock *ssk, ...@@ -123,7 +123,7 @@ static void __mptcp_move_skb(struct mptcp_sock *msk, struct sock *ssk,
skb_ext_reset(skb); skb_ext_reset(skb);
skb_orphan(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); tail = skb_peek_tail(&sk->sk_receive_queue);
if (offset == 0 && tail) { if (offset == 0 && tail) {
...@@ -261,7 +261,7 @@ static void mptcp_check_data_fin(struct sock *sk) ...@@ -261,7 +261,7 @@ static void mptcp_check_data_fin(struct sock *sk)
if (mptcp_pending_data_fin(sk, &rcv_data_fin_seq)) { if (mptcp_pending_data_fin(sk, &rcv_data_fin_seq)) {
struct mptcp_subflow_context *subflow; struct mptcp_subflow_context *subflow;
msk->ack_seq++; WRITE_ONCE(msk->ack_seq, msk->ack_seq + 1);
WRITE_ONCE(msk->rcv_data_fin, 0); WRITE_ONCE(msk->rcv_data_fin, 0);
sk->sk_shutdown |= RCV_SHUTDOWN; sk->sk_shutdown |= RCV_SHUTDOWN;
...@@ -1720,7 +1720,7 @@ struct sock *mptcp_sk_clone(const struct sock *sk, ...@@ -1720,7 +1720,7 @@ struct sock *mptcp_sk_clone(const struct sock *sk,
msk->remote_key = mp_opt->sndr_key; msk->remote_key = mp_opt->sndr_key;
mptcp_crypto_key_sha(msk->remote_key, NULL, &ack_seq); mptcp_crypto_key_sha(msk->remote_key, NULL, &ack_seq);
ack_seq++; ack_seq++;
msk->ack_seq = ack_seq; WRITE_ONCE(msk->ack_seq, ack_seq);
} }
sock_reset_flag(nsk, SOCK_RCU_FREE); sock_reset_flag(nsk, SOCK_RCU_FREE);
...@@ -2072,7 +2072,7 @@ bool mptcp_finish_join(struct sock *sk) ...@@ -2072,7 +2072,7 @@ bool mptcp_finish_join(struct sock *sk)
parent_sock = READ_ONCE(parent->sk_socket); parent_sock = READ_ONCE(parent->sk_socket);
if (parent_sock && !sk->sk_socket) if (parent_sock && !sk->sk_socket)
mptcp_sock_graft(sk, parent_sock); mptcp_sock_graft(sk, parent_sock);
subflow->map_seq = msk->ack_seq; subflow->map_seq = READ_ONCE(msk->ack_seq);
return true; return true;
} }
......
...@@ -387,7 +387,7 @@ void mptcp_data_ready(struct sock *sk, struct sock *ssk); ...@@ -387,7 +387,7 @@ void mptcp_data_ready(struct sock *sk, struct sock *ssk);
bool mptcp_finish_join(struct sock *sk); bool mptcp_finish_join(struct sock *sk);
void mptcp_data_acked(struct sock *sk); void mptcp_data_acked(struct sock *sk);
void mptcp_subflow_eof(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); void __init mptcp_token_init(void);
static inline void mptcp_token_init_request(struct request_sock *req) static inline void mptcp_token_init_request(struct request_sock *req)
......
...@@ -731,7 +731,8 @@ static enum mapping_status get_mapping_status(struct sock *ssk, ...@@ -731,7 +731,8 @@ static enum mapping_status get_mapping_status(struct sock *ssk,
if (mpext->data_fin == 1) { if (mpext->data_fin == 1) {
if (data_len == 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); pr_debug("DATA_FIN with no payload seq=%llu", mpext->data_seq);
if (subflow->map_valid) { if (subflow->map_valid) {
/* A DATA_FIN might arrive in a DSS /* A DATA_FIN might arrive in a DSS
...@@ -748,8 +749,17 @@ static enum mapping_status get_mapping_status(struct sock *ssk, ...@@ -748,8 +749,17 @@ static enum mapping_status get_mapping_status(struct sock *ssk,
return MAPPING_DATA_FIN; return MAPPING_DATA_FIN;
} }
} else { } else {
mptcp_update_rcv_data_fin(msk, mpext->data_seq + data_len); u64 data_fin_seq = mpext->data_seq + data_len;
pr_debug("DATA_FIN with mapping seq=%llu", 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 */ /* Adjust for DATA_FIN using 1 byte of sequence space */
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment