netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] mptcp: ensure snd_nxt is properly initialized on connect
@ 2024-04-29 18:00 Matthieu Baerts (NGI0)
  2024-05-01  2:10 ` patchwork-bot+netdevbpf
  0 siblings, 1 reply; 2+ messages in thread
From: Matthieu Baerts (NGI0) @ 2024-04-29 18:00 UTC (permalink / raw)
  To: mptcp, Mat Martineau, Geliang Tang, David S. Miller, Eric Dumazet,
	Jakub Kicinski, Paolo Abeni, Davide Caratti
  Cc: netdev, linux-kernel, stable, Christoph Paasch,
	Matthieu Baerts (NGI0)

From: Paolo Abeni <pabeni@redhat.com>

Christoph reported a splat hinting at a corrupted snd_una:

  WARNING: CPU: 1 PID: 38 at net/mptcp/protocol.c:1005 __mptcp_clean_una+0x4b3/0x620 net/mptcp/protocol.c:1005
  Modules linked in:
  CPU: 1 PID: 38 Comm: kworker/1:1 Not tainted 6.9.0-rc1-gbbeac67456c9 #59
  Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.el7 04/01/2014
  Workqueue: events mptcp_worker
  RIP: 0010:__mptcp_clean_una+0x4b3/0x620 net/mptcp/protocol.c:1005
  Code: be 06 01 00 00 bf 06 01 00 00 e8 a8 12 e7 fe e9 00 fe ff ff e8
  	8e 1a e7 fe 0f b7 ab 3e 02 00 00 e9 d3 fd ff ff e8 7d 1a e7 fe
  	<0f> 0b 4c 8b bb e0 05 00 00 e9 74 fc ff ff e8 6a 1a e7 fe 0f 0b e9
  RSP: 0018:ffffc9000013fd48 EFLAGS: 00010293
  RAX: 0000000000000000 RBX: ffff8881029bd280 RCX: ffffffff82382fe4
  RDX: ffff8881003cbd00 RSI: ffffffff823833c3 RDI: 0000000000000001
  RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000000
  R10: 0000000000000000 R11: fefefefefefefeff R12: ffff888138ba8000
  R13: 0000000000000106 R14: ffff8881029bd908 R15: ffff888126560000
  FS:  0000000000000000(0000) GS:ffff88813bd00000(0000) knlGS:0000000000000000
  CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
  CR2: 00007f604a5dae38 CR3: 0000000101dac002 CR4: 0000000000170ef0
  Call Trace:
   <TASK>
   __mptcp_clean_una_wakeup net/mptcp/protocol.c:1055 [inline]
   mptcp_clean_una_wakeup net/mptcp/protocol.c:1062 [inline]
   __mptcp_retrans+0x7f/0x7e0 net/mptcp/protocol.c:2615
   mptcp_worker+0x434/0x740 net/mptcp/protocol.c:2767
   process_one_work+0x1e0/0x560 kernel/workqueue.c:3254
   process_scheduled_works kernel/workqueue.c:3335 [inline]
   worker_thread+0x3c7/0x640 kernel/workqueue.c:3416
   kthread+0x121/0x170 kernel/kthread.c:388
   ret_from_fork+0x44/0x50 arch/x86/kernel/process.c:147
   ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:243
   </TASK>

When fallback to TCP happens early on a client socket, snd_nxt
is not yet initialized and any incoming ack will copy such value
into snd_una. If the mptcp worker (dumbly) tries mptcp-level
re-injection after such ack, that would unconditionally trigger a send
buffer cleanup using 'bad' snd_una values.

We could easily disable re-injection for fallback sockets, but such
dumb behavior already helped catching a few subtle issues and a very
low to zero impact in practice.

Instead address the issue always initializing snd_nxt (and write_seq,
for consistency) at connect time.

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
Tested-by: Christoph Paasch <cpaasch@apple.com>
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Mat Martineau <martineau@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/protocol.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 7e74b812e366..965eb69dc5de 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3723,6 +3723,9 @@ static int mptcp_connect(struct sock *sk, struct sockaddr *uaddr, int addr_len)
 		MPTCP_INC_STATS(sock_net(ssk), MPTCP_MIB_TOKENFALLBACKINIT);
 		mptcp_subflow_early_fallback(msk, subflow);
 	}
+
+	WRITE_ONCE(msk->write_seq, subflow->idsn);
+	WRITE_ONCE(msk->snd_nxt, subflow->idsn);
 	if (likely(!__mptcp_check_fallback(msk)))
 		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_MPCAPABLEACTIVE);
 

---
base-commit: ba1cb99b559e3b12db8b65ca9ff03358ea318064
change-id: 20240429-upstream-net-20240429-mptcp-snd_nxt-init-connect-d9844d880f48

Best regards,
-- 
Matthieu Baerts (NGI0) <matttbe@kernel.org>


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH net] mptcp: ensure snd_nxt is properly initialized on connect
  2024-04-29 18:00 [PATCH net] mptcp: ensure snd_nxt is properly initialized on connect Matthieu Baerts (NGI0)
@ 2024-05-01  2:10 ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 2+ messages in thread
From: patchwork-bot+netdevbpf @ 2024-05-01  2:10 UTC (permalink / raw)
  To: Matthieu Baerts
  Cc: mptcp, martineau, geliang, davem, edumazet, kuba, pabeni,
	dcaratti, netdev, linux-kernel, stable, cpaasch

Hello:

This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel.org>:

On Mon, 29 Apr 2024 20:00:31 +0200 you wrote:
> From: Paolo Abeni <pabeni@redhat.com>
> 
> Christoph reported a splat hinting at a corrupted snd_una:
> 
>   WARNING: CPU: 1 PID: 38 at net/mptcp/protocol.c:1005 __mptcp_clean_una+0x4b3/0x620 net/mptcp/protocol.c:1005
>   Modules linked in:
>   CPU: 1 PID: 38 Comm: kworker/1:1 Not tainted 6.9.0-rc1-gbbeac67456c9 #59
>   Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.11.0-2.el7 04/01/2014
>   Workqueue: events mptcp_worker
>   RIP: 0010:__mptcp_clean_una+0x4b3/0x620 net/mptcp/protocol.c:1005
>   Code: be 06 01 00 00 bf 06 01 00 00 e8 a8 12 e7 fe e9 00 fe ff ff e8
>   	8e 1a e7 fe 0f b7 ab 3e 02 00 00 e9 d3 fd ff ff e8 7d 1a e7 fe
>   	<0f> 0b 4c 8b bb e0 05 00 00 e9 74 fc ff ff e8 6a 1a e7 fe 0f 0b e9
>   RSP: 0018:ffffc9000013fd48 EFLAGS: 00010293
>   RAX: 0000000000000000 RBX: ffff8881029bd280 RCX: ffffffff82382fe4
>   RDX: ffff8881003cbd00 RSI: ffffffff823833c3 RDI: 0000000000000001
>   RBP: 0000000000000000 R08: 0000000000000001 R09: 0000000000000000
>   R10: 0000000000000000 R11: fefefefefefefeff R12: ffff888138ba8000
>   R13: 0000000000000106 R14: ffff8881029bd908 R15: ffff888126560000
>   FS:  0000000000000000(0000) GS:ffff88813bd00000(0000) knlGS:0000000000000000
>   CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>   CR2: 00007f604a5dae38 CR3: 0000000101dac002 CR4: 0000000000170ef0
>   Call Trace:
>    <TASK>
>    __mptcp_clean_una_wakeup net/mptcp/protocol.c:1055 [inline]
>    mptcp_clean_una_wakeup net/mptcp/protocol.c:1062 [inline]
>    __mptcp_retrans+0x7f/0x7e0 net/mptcp/protocol.c:2615
>    mptcp_worker+0x434/0x740 net/mptcp/protocol.c:2767
>    process_one_work+0x1e0/0x560 kernel/workqueue.c:3254
>    process_scheduled_works kernel/workqueue.c:3335 [inline]
>    worker_thread+0x3c7/0x640 kernel/workqueue.c:3416
>    kthread+0x121/0x170 kernel/kthread.c:388
>    ret_from_fork+0x44/0x50 arch/x86/kernel/process.c:147
>    ret_from_fork_asm+0x1a/0x30 arch/x86/entry/entry_64.S:243
>    </TASK>
> 
> [...]

Here is the summary with links:
  - [net] mptcp: ensure snd_nxt is properly initialized on connect
    https://git.kernel.org/netdev/net/c/fb7a0d334894

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2024-05-01  2:10 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-04-29 18:00 [PATCH net] mptcp: ensure snd_nxt is properly initialized on connect Matthieu Baerts (NGI0)
2024-05-01  2:10 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).