mptcp: re-check dsn before reading from subflow
authorFlorian Westphal <fw@strlen.de>
Thu, 2 Apr 2020 11:44:53 +0000 (13:44 +0200)
committerDavid S. Miller <davem@davemloft.net>
Thu, 2 Apr 2020 13:59:21 +0000 (06:59 -0700)
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@strlen.de>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/mptcp/protocol.c

index 8cc9dd2..939a504 100644 (file)
@@ -158,6 +158,27 @@ static void __mptcp_move_skb(struct mptcp_sock *msk, struct sock *ssk,
        MPTCP_SKB_CB(skb)->offset = offset;
 }
 
+/* both sockets must be locked */
+static bool mptcp_subflow_dsn_valid(const struct mptcp_sock *msk,
+                                   struct sock *ssk)
+{
+       struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(ssk);
+       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 true;
+
+       subflow->data_avail = 0;
+       return mptcp_subflow_data_available(ssk);
+}
+
 static bool __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,
                                           struct sock *ssk,
                                           unsigned int *bytes)
@@ -169,6 +190,11 @@ static bool __mptcp_move_skbs_from_subflow(struct mptcp_sock *msk,
        struct tcp_sock *tp;
        bool done = false;
 
+       if (!mptcp_subflow_dsn_valid(msk, ssk)) {
+               *bytes = 0;
+               return false;
+       }
+
        if (!(sk->sk_userlocks & SOCK_RCVBUF_LOCK)) {
                int rcvbuf = max(ssk->sk_rcvbuf, sk->sk_rcvbuf);