netdev - Re: [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-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <2f11274e-de22-4291-9172-4ad96d215a41@kernel.org>
Date: Thu, 13 Mar 2025 18:10:50 +0100
From: Matthieu Baerts <matttbe@...nel.org>
To: Arthur Mongodin <amongodin@...dorisec.fr>, netdev@...r.kernel.org
Cc: 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: Re: [PATCH net] mptcp: Fix data stream corruption in the address
 announcement

Hi Arthur,

Thank you for the patch.

On 13/03/2025 17:26, Arthur Mongodin wrote:
> 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.

It looks like you forgot to wrap this long line. I guess checkpatch.pl
should have complained. (Tip: 'b4' is a good handy tool to send patches)

Also, it is a bit difficult to understand this line. If that's OK, I can
update this when applying this patch to our MPTCP tree first. I will
send it back to netdev later on.

pw-bot: cr

Apart from that, the modification of the code looks good to me!

Reviewed-by: Matthieu Baerts (NGI0) <matttbe@...nel.org>

> 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")

We will need to Cc stable as well.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.