[PATCH mptcp-net] mptcp: ensure snd_una is properly initialized on connect

Paolo Abeni posted 1 patch 4 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/61aae3730b8c35c2e0b8b70f5f2fe5170067f5c8.1717512099.git.pabeni@redhat.com
There is a newer version of this series
net/mptcp/protocol.c | 1 +
1 file changed, 1 insertion(+)
[PATCH mptcp-net] mptcp: ensure snd_una is properly initialized on connect
Posted by Paolo Abeni 4 months, 2 weeks ago
This is strictly related to commit fb7a0d334894 ("mptcp: ensure snd_nxt
is properly initialized on connect"). It turns out that syzkaller can
trigger the retransmit after fallback and before processing any other
incoming packet - so that snd_una is still left uninitialized.

Address the issue explicitly initializing snd_una together with snd_nxt
and write_seq.

Suggested-by: Mat Martineau <martineau@kernel.org>
Fixes: 8fd738049ac3 ("mptcp: fallback in case of simultaneous connect")
Cc: stable@vger.kernel.org
Reported-by: Christoph Paasch <cpaasch@apple.com>
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/485
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
 net/mptcp/protocol.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 2f8f32cd31d3..1c5b7796daaf 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3735,6 +3735,7 @@ static int mptcp_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
 
 	WRITE_ONCE(msk->write_seq, subflow->idsn);
 	WRITE_ONCE(msk->snd_nxt, subflow->idsn);
+	WRITE_ONCE(msk->snd_una, subflow->idsn);
 	if (likely(!__mptcp_check_fallback(msk)))
 		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_MPCAPABLEACTIVE);
 
-- 
2.43.2
Re: [PATCH mptcp-net] mptcp: ensure snd_una is properly initialized on connect
Posted by Matthieu Baerts 4 months, 2 weeks ago
Hi Paolo, Mat,

On 04/06/2024 17:12, Paolo Abeni wrote:
> This is strictly related to commit fb7a0d334894 ("mptcp: ensure snd_nxt
> is properly initialized on connect"). It turns out that syzkaller can
> trigger the retransmit after fallback and before processing any other
> incoming packet - so that snd_una is still left uninitialized.
> 
> Address the issue explicitly initializing snd_una together with snd_nxt
> and write_seq.

Thank you for the patch and the review!

Now in our tree (fixes for -net):

New patches for t/upstream-net and t/upstream:
- cb8305364a4e: mptcp: ensure snd_una is properly initialized on connect
- Results: 6a73724ecdb9..1445e7997ac5 (export-net)
- Results: 4b2b4ad31b2f..62017f8a5840 (export)

Tests are now in progress:

- export-net:
https://github.com/multipath-tcp/mptcp_net-next/commit/8fb2c487a1ff37b885f724482fc82d85608b7ab5/checks
- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/8aa3bd2d65c8827d0f8907d8dfb8e5623d15c346/checks

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.
Re: [PATCH mptcp-net] mptcp: ensure snd_una is properly initialized on connect
Posted by Mat Martineau 4 months, 2 weeks ago
On Tue, 4 Jun 2024, Paolo Abeni wrote:

> This is strictly related to commit fb7a0d334894 ("mptcp: ensure snd_nxt
> is properly initialized on connect"). It turns out that syzkaller can
> trigger the retransmit after fallback and before processing any other
> incoming packet - so that snd_una is still left uninitialized.
>
> Address the issue explicitly initializing snd_una together with snd_nxt
> and write_seq.
>
> Suggested-by: Mat Martineau <martineau@kernel.org>
> Fixes: 8fd738049ac3 ("mptcp: fallback in case of simultaneous connect")
> Cc: stable@vger.kernel.org
> Reported-by: Christoph Paasch <cpaasch@apple.com>
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/485
> Signed-off-by: Paolo Abeni <pabeni@redhat.com>
> ---
> net/mptcp/protocol.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 2f8f32cd31d3..1c5b7796daaf 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -3735,6 +3735,7 @@ static int mptcp_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
>
> 	WRITE_ONCE(msk->write_seq, subflow->idsn);
> 	WRITE_ONCE(msk->snd_nxt, subflow->idsn);
> +	WRITE_ONCE(msk->snd_una, subflow->idsn);
> 	if (likely(!__mptcp_check_fallback(msk)))
> 		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_MPCAPABLEACTIVE);

Thanks Paolo, LGTM:

Reviewed-by: Mat Martineau <martineau@kernel.org>