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

* [PATCH mptcp-net v2] mptcp: ensure snd_una is properly initialized on connect
@ 2024-04-15 14:05 Paolo Abeni
  2024-04-15 20:48 ` Christoph Paasch
  2024-04-15 21:20 ` MPTCP CI
  0 siblings, 2 replies; 3+ messages in thread
From: Paolo Abeni @ 2024-04-15 14:05 UTC (permalink / raw)
  To: MPTCP Upstream; +Cc: Christoph Paasch

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 client socket, and the mptcp
worker (dumbly) tries mptcp-level re-injection, the snd_una is
not yet initialized, and the worker tries to use it to clean-up
the send buffer.

Address the issue always initializing snd_una for active sockets
at establish .

Reported-by: Christoph Paasch <cpaasch@apple.com>
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/485
Fixes: 8fd738049ac3 ("mptcp: fallback in case of simultaneous connect")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
---
v1 -> v2:
 - moved the initialization at established time (Mat)
 - dropped debug code wrongly landed in v1
 - added fixes tag

@Christoph: could you please add also the following chunk, for debug
purpouse?

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 3e1b15d76442..46fd84697495 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -1002,8 +1002,12 @@ static void __mptcp_clean_una(struct sock *sk)

                if (unlikely(dfrag == msk->first_pending)) {
                        /* in recovery mode can see ack after the current snd head */
-                       if (WARN_ON_ONCE(!msk->recovery))
+                       if (WARN_ON_ONCE(!msk->recovery)) {
+                               pr_err("snd_una %llx dfrag end seq %llx frag len %d bytes sent %lld acked %lld",
+                                       msk->snd_una, dfrag->data_seq + dfrag->data_len, dfrag->data_len,
+                                       msk->bytes_sent, msk->bytes_acked);
                                break;
+                       }

                        WRITE_ONCE(msk->first_pending, mptcp_send_next(sk));
                }
---
 net/mptcp/subflow.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index b94d1dca1094..25004473e7ef 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -475,12 +475,12 @@ static void mptcp_propagate_state(struct sock *sk, struct sock *ssk,
 	struct mptcp_sock *msk = mptcp_sk(sk);
 
 	mptcp_data_lock(sk);
+	WRITE_ONCE(msk->snd_una, subflow->idsn + 1);
+	WRITE_ONCE(msk->wnd_end, subflow->idsn + 1 + tcp_sk(ssk)->snd_wnd);
 	if (mp_opt) {
 		/* Options are available only in the non fallback cases
 		 * avoid updating rx path fields otherwise
 		 */
-		WRITE_ONCE(msk->snd_una, subflow->idsn + 1);
-		WRITE_ONCE(msk->wnd_end, subflow->idsn + 1 + tcp_sk(ssk)->snd_wnd);
 		subflow_set_remote_key(msk, subflow, mp_opt);
 	}
 
-- 
2.43.2


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