mptcp: properly account bulk freed memory
authorPaolo Abeni <pabeni@redhat.com>
Sat, 10 Jul 2021 00:20:51 +0000 (17:20 -0700)
committerDavid S. Miller <davem@davemloft.net>
Sat, 10 Jul 2021 01:38:53 +0000 (18:38 -0700)
After commit 879526030c8b ("mptcp: protect the rx path with
the msk socket spinlock") the rmem currently used by a given
msk is really sk_rmem_alloc - rmem_released.

The safety check in mptcp_data_ready() does not take the above
in due account, as a result legit incoming data is kept in
subflow receive queue with no reason, delaying or blocking
MPTCP-level ack generation.

This change addresses the issue introducing a new helper to fetch
the rmem memory and using it as needed. Additionally add a MIB
counter for the exceptional event described above - the peer is
misbehaving.

Finally, introduce the required annotation when rmem_released is
updated.

Fixes: 879526030c8b ("mptcp: protect the rx path with the msk socket spinlock")
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/211
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>
net/mptcp/mib.c
net/mptcp/mib.h
net/mptcp/protocol.c
net/mptcp/protocol.h

index 52ea2517e85602e52de7d24759ff1b6ae39b8f7b..ff2cc0e3273df14f8a6afd5deb8330872b19f016 100644 (file)
@@ -44,6 +44,7 @@ static const struct snmp_mib mptcp_snmp_list[] = {
        SNMP_MIB_ITEM("RmSubflow", MPTCP_MIB_RMSUBFLOW),
        SNMP_MIB_ITEM("MPPrioTx", MPTCP_MIB_MPPRIOTX),
        SNMP_MIB_ITEM("MPPrioRx", MPTCP_MIB_MPPRIORX),
+       SNMP_MIB_ITEM("RcvPruned", MPTCP_MIB_RCVPRUNED),
        SNMP_MIB_SENTINEL
 };
 
index 193466c9b5494aa4bca3461a8d628eca4c74dc64..0663cb12b448bf91e716f541b62550a0b76436a1 100644 (file)
@@ -37,6 +37,7 @@ enum linux_mptcp_mib_field {
        MPTCP_MIB_RMSUBFLOW,            /* Remove a subflow */
        MPTCP_MIB_MPPRIOTX,             /* Transmit a MP_PRIO */
        MPTCP_MIB_MPPRIORX,             /* Received a MP_PRIO */
+       MPTCP_MIB_RCVPRUNED,            /* Incoming packet dropped due to memory limit */
        __MPTCP_MIB_MAX
 };
 
index 7a5afa8c686686dcf878efcb47baa1695521e9cf..a8892494781522a16492169eb0746d626a09ecea 100644 (file)
@@ -474,7 +474,7 @@ static void mptcp_cleanup_rbuf(struct mptcp_sock *msk)
        bool cleanup, rx_empty;
 
        cleanup = (space > 0) && (space >= (old_space << 1));
-       rx_empty = !atomic_read(&sk->sk_rmem_alloc);
+       rx_empty = !__mptcp_rmem(sk);
 
        mptcp_for_each_subflow(msk, subflow) {
                struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
@@ -720,8 +720,10 @@ void mptcp_data_ready(struct sock *sk, struct sock *ssk)
                sk_rbuf = ssk_rbuf;
 
        /* over limit? can't append more skbs to msk, Also, no need to wake-up*/
-       if (atomic_read(&sk->sk_rmem_alloc) > sk_rbuf)
+       if (__mptcp_rmem(sk) > sk_rbuf) {
+               MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
                return;
+       }
 
        /* Wake-up the reader only for in-sequence data */
        mptcp_data_lock(sk);
@@ -1754,7 +1756,7 @@ static int __mptcp_recvmsg_mskq(struct mptcp_sock *msk,
                if (!(flags & MSG_PEEK)) {
                        /* we will bulk release the skb memory later */
                        skb->destructor = NULL;
-                       msk->rmem_released += skb->truesize;
+                       WRITE_ONCE(msk->rmem_released, msk->rmem_released + skb->truesize);
                        __skb_unlink(skb, &msk->receive_queue);
                        __kfree_skb(skb);
                }
@@ -1873,7 +1875,7 @@ static void __mptcp_update_rmem(struct sock *sk)
 
        atomic_sub(msk->rmem_released, &sk->sk_rmem_alloc);
        sk_mem_uncharge(sk, msk->rmem_released);
-       msk->rmem_released = 0;
+       WRITE_ONCE(msk->rmem_released, 0);
 }
 
 static void __mptcp_splice_receive_queue(struct sock *sk)
@@ -2380,7 +2382,7 @@ static int __mptcp_init_sock(struct sock *sk)
        msk->out_of_order_queue = RB_ROOT;
        msk->first_pending = NULL;
        msk->wmem_reserved = 0;
-       msk->rmem_released = 0;
+       WRITE_ONCE(msk->rmem_released, 0);
        msk->tx_pending_data = 0;
 
        msk->first = NULL;
index 426ed80fe72f747f28f43970a96fe347c3219c2d..0f0c026c5f8bb208e18bf0a1e42087a38ecb12bf 100644 (file)
@@ -296,9 +296,17 @@ static inline struct mptcp_sock *mptcp_sk(const struct sock *sk)
        return (struct mptcp_sock *)sk;
 }
 
+/* the msk socket don't use the backlog, also account for the bulk
+ * free memory
+ */
+static inline int __mptcp_rmem(const struct sock *sk)
+{
+       return atomic_read(&sk->sk_rmem_alloc) - READ_ONCE(mptcp_sk(sk)->rmem_released);
+}
+
 static inline int __mptcp_space(const struct sock *sk)
 {
-       return tcp_space(sk) + READ_ONCE(mptcp_sk(sk)->rmem_released);
+       return tcp_win_from_space(sk, READ_ONCE(sk->sk_rcvbuf) - __mptcp_rmem(sk));
 }
 
 static inline struct mptcp_data_frag *mptcp_send_head(const struct sock *sk)