mptcp: add-addr: always drop other suboptions

When an ADD_ADDR needs to be sent, it could be prepared if there is
enough remaining space and even if the packet is not a pure ACK. But it
would be dropped soon after.

Indeed, in mptcp_pm_add_addr_signal(), there is enough space to fit a
DSS of 20 octets and an ADD_ADDR echo containing an IPv4 address on 8
octets for example. In this case, the packet would be prepared, the
MPTCP_ADD_ADDR_ECHO bit would be removed from pm->addr_signal, but the
option would be silently dropped in mptcp_established_options_add_addr()
not to override DSS info in the union from 'struct mptcp_out_options',
and also because mptcp_write_options() will enforce mutually exclusion
with DSS.

Instead, don't even try to send an ADD_ADDR if it is not a pure ACK.
Retry for each new packet until a pure-ACK is emitted. That's fine to do
that, because each time an ADD_ADDR (echo) is scheduled, a pure ACK is
queued.

This also simplifies the code, and the skb checks can be done earlier,
before the lock.

Note: also, since commit 6d0060f600ad ("mptcp: Write MPTCP DSS headers
to outgoing data packets"), opts->ahmac would not have been set to 0
when other suboptions were not dropped, and when sending an ADD_ADDR
echo. That would have resulted in sending an ADD_ADDR using garbage
info, where there was not enough space, instead of an echo one without
the ADD_ADDR HMAC.

Fixes: 1bff1e43a30e ("mptcp: optimize out option generation")
Cc: stable@vger.kernel.org
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Link: https://patch.msgid.link/20260602-net-mptcp-misc-fixes-7-1-rc7-v2-11-856831229976@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index f9f587203c3542bd7cb255761b44cfd8bc5fd5d7..b3ea7854818fde8f216246de6d7111ebd8566370 100644 (file)
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -665,7 +665,6 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
 {
        struct mptcp_subflow_context *subflow = mptcp_subflow_ctx(sk);
        struct mptcp_sock *msk = mptcp_sk(subflow->conn);
-       bool drop_other_suboptions = false;
        unsigned int opt_size = *size;
        struct mptcp_addr_info addr;
        bool echo;
@@ -676,36 +675,20 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
         */
        if (!mptcp_pm_should_add_signal(msk) ||
            (opts->suboptions & (OPTION_MPTCP_MPJ_ACK | OPTION_MPTCP_MPC_ACK)) ||
-           !mptcp_pm_add_addr_signal(msk, skb, opt_size, remaining, &addr,
-                   &echo, &drop_other_suboptions))
+           !skb || !skb_is_tcp_pure_ack(skb) ||
+           !mptcp_pm_add_addr_signal(msk, opt_size, remaining, &addr, &echo))
                return false;
 
-       /*
-        * Later on, mptcp_write_options() will enforce mutually exclusion with
-        * DSS, bail out if such option is set and we can't drop it.
-        */
-       if (drop_other_suboptions)
-               remaining += opt_size;
-       else if (opts->suboptions & OPTION_MPTCP_DSS)
-               return false;
+       remaining += opt_size;
 
        len = mptcp_add_addr_len(addr.family, echo, !!addr.port);
        if (remaining < len)
                return false;
 
        *size = len;
-       if (drop_other_suboptions) {
-               pr_debug("drop other suboptions\n");
-               opts->suboptions = 0;
-
-               /* note that e.g. DSS could have written into the memory
-                * aliased by ahmac, we must reset the field here
-                * to avoid appending the hmac even for ADD_ADDR echo
-                * options
-                */
-               opts->ahmac = 0;
-               *size -= opt_size;
-       }
+       pr_debug("drop other suboptions\n");
+       opts->suboptions = 0;
+       *size -= opt_size;
        opts->addr = addr;
        opts->suboptions |= OPTION_MPTCP_ADD_ADDR;
        if (!echo) {
@@ -715,6 +698,7 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
                                                     &opts->addr);
        } else {
                MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ECHOADDTX);
+               opts->ahmac = 0;
        }
        pr_debug("addr_id=%d, ahmac=%llu, echo=%d, port=%d\n",
                 opts->addr.id, opts->ahmac, echo, ntohs(opts->addr.port));

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 3e770c7407e1fd359cca69aa86eaf7c2695b59b0..470501470fe5430781a46f4035e3b38ef8a2e42f 100644 (file)
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -887,10 +887,9 @@ void mptcp_pm_mp_fail_received(struct sock *sk, u64 fail_seq)
        }
 }
 
-bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, const struct sk_buff *skb,
-                             unsigned int opt_size, unsigned int remaining,
-                             struct mptcp_addr_info *addr, bool *echo,
-                             bool *drop_other_suboptions)
+bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, unsigned int opt_size,
+                             unsigned int remaining,
+                             struct mptcp_addr_info *addr, bool *echo)
 {
        bool skip_add_addr = false;
        int ret = false;
@@ -908,10 +907,7 @@ bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, const struct sk_buff *skb,
         * 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;
-       }
+       remaining += opt_size;
 
        *echo = mptcp_pm_should_add_signal_echo(msk);
        if (*echo) {
@@ -929,9 +925,6 @@ bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, const struct sk_buff *skb,
        if (remaining < mptcp_add_addr_len(family, *echo, port)) {
                struct net *net = sock_net((struct sock *)msk);
 
-               if (!*drop_other_suboptions)
-                       goto out_unlock;
-
                if (*echo) {
                        MPTCP_INC_STATS(net, MPTCP_MIB_ECHOADDTXDROP);
                } else {

diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index e4f5aba24da7db9fd4c3a076b53d3b8d9df0c500..b93b878478d2620a4731a4d8ba15bf3be8cb0a04 100644 (file)
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -1229,10 +1229,9 @@ 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, const struct sk_buff *skb,
-                             unsigned int opt_size, unsigned int remaining,
-                             struct mptcp_addr_info *addr, bool *echo,
-                             bool *drop_other_suboptions);
+bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, unsigned int opt_size,
+                             unsigned int remaining,
+                             struct mptcp_addr_info *addr, bool *echo);
 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);