mptcp: Fix data stream corruption in the address announcement
commit 2c1f97a52cb827a5f2768e67a9dddffae1ed47ab upstream.
Because of the size restriction in the TCP options space, the MPTCP
ADD_ADDR option is exclusive and cannot be sent with other MPTCP ones.
For this reason, in the linked mptcp_out_options structure, group of
fields linked to different options are part of the same union.
There is a case where the mptcp_pm_add_addr_signal() function can modify
opts->addr, but not ended up sending an ADD_ADDR. Later on, back in
mptcp_established_options, other options will be sent, but with
unexpected data written in other fields due to the union, e.g. in
opts->ext_copy. This could lead to a data stream corruption in the next
packet.
Using an intermediate variable, prevents from corrupting previously
established DSS option. The assignment of the ADD_ADDR option
parameters is now done once we are sure this ADD_ADDR option can be set
in the packet, e.g. after having dropped other suboptions.
Fixes: 1bff1e43a3
("mptcp: optimize out option generation")
Cc: stable@vger.kernel.org
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Arthur Mongodin <amongodin@randorisec.fr>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
[ Matt: the commit message has been updated: long lines splits and some
clarifications. ]
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Reviewed-by: Simon Horman <horms@kernel.org>
Link: https://patch.msgid.link/20250314-net-mptcp-fix-data-stream-corr-sockopt-v1-1-122dbb249db3@kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
parent
19e85e06a0
commit
4375eee347
@ -649,6 +649,7 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
|
|||||||
struct mptcp_sock *msk = mptcp_sk(subflow->conn);
|
struct mptcp_sock *msk = mptcp_sk(subflow->conn);
|
||||||
bool drop_other_suboptions = false;
|
bool drop_other_suboptions = false;
|
||||||
unsigned int opt_size = *size;
|
unsigned int opt_size = *size;
|
||||||
|
struct mptcp_addr_info addr;
|
||||||
bool echo;
|
bool echo;
|
||||||
int len;
|
int len;
|
||||||
|
|
||||||
@ -657,7 +658,7 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
|
|||||||
*/
|
*/
|
||||||
if (!mptcp_pm_should_add_signal(msk) ||
|
if (!mptcp_pm_should_add_signal(msk) ||
|
||||||
(opts->suboptions & (OPTION_MPTCP_MPJ_ACK | OPTION_MPTCP_MPC_ACK)) ||
|
(opts->suboptions & (OPTION_MPTCP_MPJ_ACK | OPTION_MPTCP_MPC_ACK)) ||
|
||||||
!mptcp_pm_add_addr_signal(msk, skb, opt_size, remaining, &opts->addr,
|
!mptcp_pm_add_addr_signal(msk, skb, opt_size, remaining, &addr,
|
||||||
&echo, &drop_other_suboptions))
|
&echo, &drop_other_suboptions))
|
||||||
return false;
|
return false;
|
||||||
|
|
||||||
@ -670,7 +671,7 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
|
|||||||
else if (opts->suboptions & OPTION_MPTCP_DSS)
|
else if (opts->suboptions & OPTION_MPTCP_DSS)
|
||||||
return false;
|
return false;
|
||||||
|
|
||||||
len = mptcp_add_addr_len(opts->addr.family, echo, !!opts->addr.port);
|
len = mptcp_add_addr_len(addr.family, echo, !!addr.port);
|
||||||
if (remaining < len)
|
if (remaining < len)
|
||||||
return false;
|
return false;
|
||||||
|
|
||||||
@ -687,6 +688,7 @@ static bool mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
|
|||||||
opts->ahmac = 0;
|
opts->ahmac = 0;
|
||||||
*size -= opt_size;
|
*size -= opt_size;
|
||||||
}
|
}
|
||||||
|
opts->addr = addr;
|
||||||
opts->suboptions |= OPTION_MPTCP_ADD_ADDR;
|
opts->suboptions |= OPTION_MPTCP_ADD_ADDR;
|
||||||
if (!echo) {
|
if (!echo) {
|
||||||
opts->ahmac = add_addr_generate_hmac(msk->local_key,
|
opts->ahmac = add_addr_generate_hmac(msk->local_key,
|
||||||
|
Loading…
Reference in New Issue
Block a user