mptcp_add_addr_len helper was called twice: in mptcp_pm_add_addr_signal, then just after in mptcp_established_options_add_addr. Both to check the remaining space. The second call is not needed: if there is not enough space, mptcp_pm_add_addr_signal will return false, and the caller, mptcp_established_options_add_addr, will do the same without re-checking the size again. Instead, mptcp_pm_add_addr_signal can directly set the size. Note that the returned size can be negative when other suboptions are dropped, e.g. to send an echo ADD_ADDR with a v4 address, and no port. While at it: - move mptcp_add_addr_len to pm.c, as it is now only used from there - use 'int' in mptcp_add_addr_len for the size, instead of having a mix - use a bool for 'ret' in mptcp_pm_add_addr_signal Reviewed-by: Mat Martineau Signed-off-by: Matthieu Baerts (NGI0) --- net/mptcp/options.c | 16 +++------------- net/mptcp/pm.c | 26 ++++++++++++++++++++++---- net/mptcp/protocol.h | 17 +---------------- 3 files changed, 26 insertions(+), 33 deletions(-) diff --git a/net/mptcp/options.c b/net/mptcp/options.c index 05c08034a15d..be85607733f3 100644 --- a/net/mptcp/options.c +++ b/net/mptcp/options.c @@ -657,34 +657,25 @@ static bool mptcp_established_options_add_addr(struct sock *sk, 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; - int len; /* add addr will strip the existing options, be sure to avoid breaking * MPC/MPJ handshakes */ 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)) + !mptcp_pm_add_addr_signal(msk, skb, size, remaining, &addr, &echo, + &drop_other_suboptions)) 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) + if (!drop_other_suboptions && opts->suboptions & OPTION_MPTCP_DSS) return false; - 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; @@ -695,7 +686,6 @@ static bool mptcp_established_options_add_addr(struct sock *sk, * options */ opts->ahmac = 0; - *size -= opt_size; } opts->addr = addr; opts->suboptions |= OPTION_MPTCP_ADD_ADDR; diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c index 48299c8fe2a4..6a2cbe8616d3 100644 --- a/net/mptcp/pm.c +++ b/net/mptcp/pm.c @@ -887,14 +887,30 @@ void mptcp_pm_mp_fail_received(struct sock *sk, u64 fail_seq) } } +static int mptcp_add_addr_len(int family, bool echo, bool port) +{ + int len = TCPOLEN_MPTCP_ADD_ADDR_BASE; + + if (family == AF_INET6) + len = TCPOLEN_MPTCP_ADD_ADDR6_BASE; + if (!echo) + len += MPTCPOPT_THMAC_LEN; + /* account for 2 trailing 'nop' options */ + if (port) + len += TCPOLEN_MPTCP_PORT_LEN + TCPOLEN_MPTCP_PORT_ALIGN; + + return len; +} + bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, const struct sk_buff *skb, - unsigned int opt_size, unsigned int remaining, + int *size, int remaining, struct mptcp_addr_info *addr, bool *echo, bool *drop_other_suboptions) { bool skip_add_addr = false; - int ret = false; + bool ret = false; u8 add_addr; + int len = 0; u8 family; bool port; @@ -909,7 +925,7 @@ bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, const struct sk_buff *skb, * if any, will be carried by the 'original' TCP ack */ if (skb && skb_is_tcp_pure_ack(skb)) { - remaining += opt_size; + len -= *size; *drop_other_suboptions = true; } @@ -926,7 +942,8 @@ bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, const struct sk_buff *skb, family = msk->pm.local.family; } - if (remaining < mptcp_add_addr_len(family, *echo, port)) { + len += mptcp_add_addr_len(family, *echo, port); + if (len > remaining) { struct net *net = sock_net((struct sock *)msk); if (!*drop_other_suboptions) @@ -942,6 +959,7 @@ bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, const struct sk_buff *skb, } ret = true; + *size = len; drop_signal_mark: WRITE_ONCE(msk->pm.addr_signal, add_addr); diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h index da677f5cef71..e0ffebaa6795 100644 --- a/net/mptcp/protocol.h +++ b/net/mptcp/protocol.h @@ -1206,23 +1206,8 @@ static inline bool mptcp_pm_is_kernel(const struct mptcp_sock *msk) return READ_ONCE(msk->pm.pm_type) == MPTCP_PM_TYPE_KERNEL; } -static inline unsigned int mptcp_add_addr_len(int family, bool echo, bool port) -{ - u8 len = TCPOLEN_MPTCP_ADD_ADDR_BASE; - - if (family == AF_INET6) - len = TCPOLEN_MPTCP_ADD_ADDR6_BASE; - if (!echo) - len += MPTCPOPT_THMAC_LEN; - /* account for 2 trailing 'nop' options */ - if (port) - len += TCPOLEN_MPTCP_PORT_LEN + TCPOLEN_MPTCP_PORT_ALIGN; - - return len; -} - bool mptcp_pm_add_addr_signal(struct mptcp_sock *msk, const struct sk_buff *skb, - unsigned int opt_size, unsigned int remaining, + int *size, int remaining, struct mptcp_addr_info *addr, bool *echo, bool *drop_other_suboptions); bool mptcp_pm_rm_addr_signal(struct mptcp_sock *msk, unsigned int remaining, -- 2.53.0