Catcrafts/milos-linux
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

mptcp: add-addr: always drop other suboptions

Browse source 
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 6d0060f600 ("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: 1bff1e43a3 ("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>
This commit is contained in:
Matthieu Baerts (NGI0) 2026-06-02 22:14:18 +10:00 • committed by Jakub Kicinski
commit bd34fa0257
3 changed files with 14 additions and 38 deletions
Download patch file Download diff file Expand all files Collapse all files

30
net/mptcp/options.c
View file

@ -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));

15
net/mptcp/pm.c
View file

@ -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 {

7
net/mptcp/protocol.h
View file

@ -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);