[MPTCP] [PATCH v3 5/6] mptcp: protocol: re-check dsn before reading from subflow

[MPTCP] [PATCH v3 5/6] mptcp: protocol: re-check dsn before reading from subflow

[Florian Westphal]

[-- Attachment #1: Type: text/plain, Size: 1779 bytes --]

mptcp_subflow_data_available() is commonly called via
ssk->sk_data_ready(), in this case the mptcp socket lock
cannot be acquired.

Therefore, while we can safely discard subflow data that
was already received up to msk->ack_seq, we cannot be sure
that 'subflow->data_avail' will still be valid at the time
userspace wants to read the data -- a previous read on a
different subflow might have carried this data already.

In that (unlikely) event, msk->ack_seq will have been updated
and will be ahead of the subflow dsn.

We can check for this condition and skip/resync to the expected
sequence number.

Signed-off-by: Florian Westphal <fw(a)strlen.de>
---
 net/mptcp/protocol.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 75ae03931963..02aba8b31f1f 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -205,8 +205,24 @@ static struct sock *mptcp_subflow_recv_lookup(const struct mptcp_sock *msk)
 	sock_owned_by_me(sk);
 
 	mptcp_for_each_subflow(msk, subflow) {
-		if (subflow->data_avail)
-			return mptcp_subflow_tcp_sock(subflow);
+		if (subflow->data_avail) {
+			struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+			u64 dsn = mptcp_subflow_get_mapped_dsn(subflow);
+
+			/* revalidate data sequence number.
+			 *
+			 * mptcp_subflow_data_available() is usually called
+			 * without msk lock.  Its unlikely (but possible)
+			 * that msk->ack_seq has been advanced since the last
+			 * call found in-sequence data.
+			 */
+			if (likely(dsn == msk->ack_seq))
+				return ssk;
+
+			subflow->data_avail = 0;
+			if (mptcp_subflow_data_available(ssk))
+				return ssk;
+		}
 	}
 
 	return NULL;
-- 
2.24.1

[MPTCP] [PATCH v3 5/6] mptcp: protocol: re-check dsn before reading from subflow

[Florian Westphal]

[-- Attachment #1: Type: text/plain, Size: 1779 bytes --]

mptcp_subflow_data_available() is commonly called via
ssk->sk_data_ready(), in this case the mptcp socket lock
cannot be acquired.

Therefore, while we can safely discard subflow data that
was already received up to msk->ack_seq, we cannot be sure
that 'subflow->data_avail' will still be valid at the time
userspace wants to read the data -- a previous read on a
different subflow might have carried this data already.

In that (unlikely) event, msk->ack_seq will have been updated
and will be ahead of the subflow dsn.

We can check for this condition and skip/resync to the expected
sequence number.

Signed-off-by: Florian Westphal <fw(a)strlen.de>
---
 net/mptcp/protocol.c | 20 ++++++++++++++++++--
 1 file changed, 18 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 75ae03931963..02aba8b31f1f 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -205,8 +205,24 @@ static struct sock *mptcp_subflow_recv_lookup(const struct mptcp_sock *msk)
 	sock_owned_by_me(sk);
 
 	mptcp_for_each_subflow(msk, subflow) {
-		if (subflow->data_avail)
-			return mptcp_subflow_tcp_sock(subflow);
+		if (subflow->data_avail) {
+			struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
+			u64 dsn = mptcp_subflow_get_mapped_dsn(subflow);
+
+			/* revalidate data sequence number.
+			 *
+			 * mptcp_subflow_data_available() is usually called
+			 * without msk lock.  Its unlikely (but possible)
+			 * that msk->ack_seq has been advanced since the last
+			 * call found in-sequence data.
+			 */
+			if (likely(dsn == msk->ack_seq))
+				return ssk;
+
+			subflow->data_avail = 0;
+			if (mptcp_subflow_data_available(ssk))
+				return ssk;
+		}
 	}
 
 	return NULL;
-- 
2.24.1