netdev - [PATCH net] mptcp: Fix data stream corruption in the address announcement

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <81b2a80e-2a25-4a5f-b235-07a68662aa98@randorisec.fr>
Date: Thu, 13 Mar 2025 17:26:57 +0100
From: Arthur Mongodin <amongodin@...dorisec.fr>
To: netdev@...r.kernel.org
Cc: Matthieu Baerts <matttbe@...nel.org>, martineau@...nel.org,
 geliang@...nel.org, davem@...emloft.net, edumazet@...gle.com,
 kuba@...nel.org, Paolo Abeni <pabeni@...hat.com>, horms@...nel.org,
 mptcp@...ts.linux.dev, hanguelkov@...dorisec.fr,
 Davy Douhine <davy@...dorisec.fr>
Subject: [PATCH net] mptcp: Fix data stream corruption in the address
 announcement

The DSS and ADD_ADDR options should be exclusive and not send together.
The call to the mptcp_pm_add_addr_signal() function in the
mptcp_established_options_add_addr() function could modify opts->addr, 
thus also opts->ext_copy as they belong to distinguish entries of the 
same union field in mptcp_out_options. If the DSS option should not be 
dropped, the check if the DSS option has been previously established and 
thus if we should not establish the ADD_ADDR option is done after 
opts->addr (thus opts->ext_copy) has been modified.
This corruption may modify stream information send in the next packet
with invalid data.
Using an intermediate variable, prevents from corrupting previously
established DSS option. The assignment of the ADD_ADDR option
parameters in done once we are sure that the DSS option has been dropped
or it has not been established previously.

Suggested-by: Paolo Abeni <pabeni@...hat.com>
Fixes: 1bff1e43a30e ("mptcp: optimize out option generation")
Signed-off-by: Arthur Mongodin <amongodin@...dorisec.fr>
---
  net/mptcp/options.c | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index fd2de185bc93..23949ae2a3a8 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -651,6 +651,7 @@ static bool 
mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
  	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;

@@ -659,7 +660,7 @@ 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, &opts->addr,
+	    !mptcp_pm_add_addr_signal(msk, skb, opt_size, remaining, &addr,
  		    &echo, &drop_other_suboptions))
  		return false;

@@ -672,7 +673,7 @@ static bool 
mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
  	else if (opts->suboptions & OPTION_MPTCP_DSS)
  		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)
  		return false;

@@ -689,6 +690,7 @@ static bool 
mptcp_established_options_add_addr(struct sock *sk, struct sk_buff *
  		opts->ahmac = 0;
  		*size -= opt_size;
  	}
+	opts->addr = addr;
  	opts->suboptions |= OPTION_MPTCP_ADD_ADDR;
  	if (!echo) {
  		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_ADDADDRTX);
-- 
2.47.2