mptcp: move drop_other_suboptions check under pm lock

This patch moved the drop_other_suboptions check from
mptcp_established_options_add_addr() into mptcp_pm_add_addr_signal(), do
it under the PM lock to avoid the race between this check and
mptcp_pm_add_addr_signal().

For this, added a new parameter for mptcp_pm_add_addr_signal() to get
the drop_other_suboptions value. And drop the other suboptions after the
option length check if drop_other_suboptions is true.

Additionally, always drop the other suboption for TCP pure ack:
that makes both the code simpler and the MPTCP behaviour more
consistent.

Co-developed-by: Geliang Tang <geliangtang@gmail.com>
Signed-off-by: Geliang Tang <geliangtang@gmail.com>
Co-developed-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Yonglong Li <liyonglong@chinatelecom.cn>
Signed-off-by: Mat Martineau <mathew.j.martineau@linux.intel.com>
Signed-off-by: David S. Miller <davem@davemloft.net>

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index bebb759..4c37f4b 100644 (file)
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -667,29 +667,29 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
        bool port;
        int len;
 
-       if ((mptcp_pm_should_add_signal_ipv6(msk) ||
-            mptcp_pm_should_add_signal_port(msk) ||
-            mptcp_pm_should_add_signal_echo(msk)) &&
-           skb && skb_is_tcp_pure_ack(skb)) {
-               pr_debug("drop other suboptions");
-               opts->suboptions = 0;
-               opts->ext_copy.use_ack = 0;
-               opts->ext_copy.use_map = 0;
-               remaining += opt_size;
-               drop_other_suboptions = true;
-       }
-
+       /* add addr will strip the existing options, be sure to avoid breaking
+        * MPC/MPJ handshakes
+        */
        if (!mptcp_pm_should_add_signal(msk) ||
-           !(mptcp_pm_add_addr_signal(msk, remaining, &opts->addr, &echo, &port)))
+           (opts->suboptions & (OPTION_MPTCP_MPJ_ACK | OPTION_MPTCP_MPC_ACK)) ||
+           !mptcp_pm_add_addr_signal(msk, skb, opt_size, remaining, &opts->addr,
+                   &echo, &port, &drop_other_suboptions))
                return false;
 
+       if (drop_other_suboptions)
+               remaining += opt_size;
        len = mptcp_add_addr_len(opts->addr.family, echo, port);
        if (remaining < len)
                return false;
 
        *size = len;
-       if (drop_other_suboptions)
+       if (drop_other_suboptions) {
+               pr_debug("drop other suboptions");
+               opts->suboptions = 0;
+               opts->ext_copy.use_ack = 0;
+               opts->ext_copy.use_map = 0;
                *size -= opt_size;
+       }
        opts->suboptions |= OPTION_MPTCP_ADD_ADDR;
        if (!echo) {
                opts->ahmac = add_addr_generate_hmac(msk->local_key,

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 0ed3e56..24e2f6f 100644 (file)
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -251,8 +251,10 @@ void mptcp_pm_mp_prio_received(struct sock *sk, u8 bkup)
 
 /* path manager helpers */
 
-bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
-                             struct mptcp_addr_info *saddr, bool *echo, bool *port)
+bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, struct sk_buff *skb,
+                             unsigned int opt_size, unsigned int remaining,
+                             struct mptcp_addr_info *saddr, bool *echo,
+                             bool *port, bool *drop_other_suboptions)
 {
        int ret = false;
 
@@ -262,6 +264,15 @@ bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
        if (!mptcp_pm_should_add_signal(msk))
                goto out_unlock;
 
+       /* always drop every other options for pure ack ADD_ADDR; this is a
+        * plain dup-ack from TCP perspective. The other MPTCP-relevant info,
+        * if any, will be carried by the 'original' TCP ack
+        */
+       if (skb && skb_is_tcp_pure_ack(skb)) {
+               remaining += opt_size;
+               *drop_other_suboptions = true;
+       }
+
        *echo = mptcp_pm_should_add_signal_echo(msk);
        *port = mptcp_pm_should_add_signal_port(msk);
 

diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index bc1bfd7..40bc9d3 100644 (file)
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -794,8 +794,10 @@ static inline int mptcp_rm_addr_len(const struct mptcp_rm_list *rm_list)
        return TCPOLEN_MPTCP_RM_ADDR_BASE + roundup(rm_list->nr - 1, 4) + 1;
 }
 
-bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
-                             struct mptcp_addr_info *saddr, bool *echo, bool *port);
+bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, struct sk_buff *skb,
+                             unsigned int opt_size, unsigned int remaining,
+                             struct mptcp_addr_info *saddr, bool *echo,
+                             bool *port, bool *drop_other_suboptions);
 bool mptcp_pm_rm_addr_signal(struct mptcp_sock *msk, unsigned int remaining,
                             struct mptcp_rm_list *rm_list);
 int mptcp_pm_get_local_id(struct mptcp_sock *msk, struct sock_common *skc);