Skip to contentSkip to conversation

FAILED: patch "[PATCH] mptcp: fix rcv space initialization" failed to apply to 6.1-stable tree

PATCH [email protected] 2 participants 2 messages Feb 19, 2024 – Feb 27, 2024

Subsystems (3)
Networking Stable branch Networking MPTCP
lore
Share & tools
lore

[email protected]

author maintainer
#0 lore raw

The patch below does not apply to the 6.1-stable tree.
If someone wants it applied there, or to any other stable or longterm tree, then please email the backport, including the original git commit id to <[email protected]>.

To reproduce the conflict and resubmit, you may use the following commands:

git fetch https://git.kernel.org/pub/scm/linux/kernel/git/stable/linux.git/ linux-6.1.y
git checkout FETCH_HEAD
git cherry-pick -x 013e3179dbd2bc756ce1dd90354abac62f65b739
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<[email protected]>' --in-reply-to '2024021902-cleft-esquire-a9ae@gregkh' --subject-prefix 'PATCH 6.1.y' HEAD^..

Possible dependencies:

013e3179dbd2 ("mptcp: fix rcv space initialization")
4fd19a307016 ("mptcp: fix inconsistent state on fastopen race") d109a7767273 ("mptcp: fix possible NULL pointer dereference on close") 8005184fd1ca ("mptcp: refactor sndbuf auto-tuning")
a5efdbcece83 ("mptcp: fix delegated action races")
27e5ccc2d5a5 ("mptcp: fix dangling connection hang-up") f6909dc1c1f4 ("mptcp: rename timer related helper to less confusing names") ebc1e08f01eb ("mptcp: drop last_snd and MPTCP_RESET_SCHEDULER") e263691773cd ("mptcp: Remove unnecessary test for __mptcp_init_sock()") 39880bd808ad ("mptcp: get rid of msk->subflow")
17ebf8a4c38b ("mptcp: fix the incorrect judgment for msk->cb_flags") a7384f391875 ("Merge git://git.kernel.org/pub/scm/linux/kernel/git/netdev/net")

thanks,

greg k-h

------------------ original commit in Linus's tree ------------------

From 013e3179dbd2bc756ce1dd90354abac62f65b739 Mon Sep 17 00:00:00 2001
From: Paolo Abeni <[email protected]>
Date: Thu, 8 Feb 2024 19:03:50 +0100
Subject: [PATCH] mptcp: fix rcv space initialization

mptcp_rcv_space_init() is supposed to happen under the msk socket lock, but active msk socket does that without such protection.

Leverage the existing mptcp_propagate_state() helper to that extent. We need to ensure mptcp_rcv_space_init will happen before mptcp_rcv_space_adjust(), and the release_cb does not assure that:
explicitly check for such condition.

While at it, move the wnd_end initialization out of mptcp_rcv_space_init(), it never belonged there.

Note that the race does not produce ill effect in practice, but change allows cleaning-up and defying better the locking model.

Fixes a6b118febbab ("mptcp: add receive buffer auto-tuning")
Cc [email protected]
Signed-off-by Paolo Abeni <[email protected]>
Reviewed-by Mat Martineau <[email protected]>
Signed-off-by Matthieu Baerts (NGI0) <[email protected]>
Signed-off-by David S. Miller <[email protected]>
net/mptcp/protocol.c +6 -4
index 2111819016af..7632eafb683b 100644
@@ -1963,6 +1963,9 @@ static void mptcp_rcv_space_adjust(struct mptcp_sock *msk, int copied)
 	if (copied <= 0)
 		return;
 
+	if (!msk->rcvspace_init)
+		mptcp_rcv_space_init(msk, msk->first);
+
 	msk->rcvq_space.copied += copied;
 
 	mstamp = div_u64(tcp_clock_ns(), NSEC_PER_USEC);

@@ -3160,6 +3163,7 @@ static int mptcp_disconnect(struct sock *sk, int flags)
 	msk->bytes_received = 0;
 	msk->bytes_sent = 0;
 	msk->bytes_retrans = 0;
+	msk->rcvspace_init = 0;
 
 	WRITE_ONCE(sk->sk_shutdown, 0);
 	sk_error_report(sk);

@@ -3247,6 +3251,7 @@ void mptcp_rcv_space_init(struct mptcp_sock *msk, const struct sock *ssk)
 {
 	const struct tcp_sock *tp = tcp_sk(ssk);
 
+	msk->rcvspace_init = 1;
 	msk->rcvq_space.copied = 0;
 	msk->rcvq_space.rtt_us = 0;
 

@@ -3257,8 +3262,6 @@ void mptcp_rcv_space_init(struct mptcp_sock *msk, const struct sock *ssk)
 				      TCP_INIT_CWND * tp->advmss);
 	if (msk->rcvq_space.space == 0)
 		msk->rcvq_space.space = TCP_INIT_CWND * TCP_MSS_DEFAULT;
-
-	WRITE_ONCE(msk->wnd_end, msk->snd_nxt + tcp_sk(ssk)->snd_wnd);
 }
 
 void mptcp_destroy_common(struct mptcp_sock *msk, unsigned int flags)

@@ -3478,10 +3481,9 @@ void mptcp_finish_connect(struct sock *ssk)
 	WRITE_ONCE(msk->write_seq, subflow->idsn + 1);
 	WRITE_ONCE(msk->snd_nxt, msk->write_seq);
 	WRITE_ONCE(msk->snd_una, msk->write_seq);
+	WRITE_ONCE(msk->wnd_end, msk->snd_nxt + tcp_sk(ssk)->snd_wnd);
 
 	mptcp_pm_new_connection(msk, ssk, 0);
-
-	mptcp_rcv_space_init(msk, ssk);
 }
 
 void mptcp_sock_graft(struct sock *sk, struct socket *parent)
net/mptcp/protocol.h +2 -1
index b905f1868298..9f5ee82e3473 100644
@@ -304,7 +304,8 @@ struct mptcp_sock {
 			nodelay:1,
 			fastopening:1,
 			in_accept_queue:1,
-			free_first:1;
+			free_first:1,
+			rcvspace_init:1;
 	struct work_struct work;
 	struct sk_buff  *ooo_last_skb;
 	struct rb_root  out_of_order_queue;
net/mptcp/subflow.c +2 -2
index 0dcb721c89d1..56b2ac2f2f22 100644
@@ -424,6 +424,8 @@ void __mptcp_sync_state(struct sock *sk, int state)
 	struct mptcp_sock *msk = mptcp_sk(sk);
 
 	__mptcp_propagate_sndbuf(sk, msk->first);
+	if (!msk->rcvspace_init)
+		mptcp_rcv_space_init(msk, msk->first);
 	if (sk->sk_state == TCP_SYN_SENT) {
 		mptcp_set_state(sk, state);
 		sk->sk_state_change(sk);

@@ -545,7 +547,6 @@ static void subflow_finish_connect(struct sock *sk, const struct sk_buff *skb)
 		}
 	} else if (mptcp_check_fallback(sk)) {
 fallback:
-		mptcp_rcv_space_init(msk, sk);
 		mptcp_propagate_state(parent, sk);
 	}
 	return;

@@ -1744,7 +1745,6 @@ static void subflow_state_change(struct sock *sk)
 	msk = mptcp_sk(parent);
 	if (subflow_simultaneous_connect(sk)) {
 		mptcp_do_fallback(sk);
-		mptcp_rcv_space_init(msk, sk);
 		pr_fallback(msk);
 		subflow->conn_finished = 1;
 		mptcp_propagate_state(parent, sk);
maintainer
#1 lore raw

Hi Greg,

Quoted 2 lines · On 19/02/2024 17:04, [email protected] wrote:

The patch below does not apply to the 6.1-stable tree.

(...)

Quoted 13 lines

From 013e3179dbd2bc756ce1dd90354abac62f65b739 Mon Sep 17 00:00:00 2001
From: Paolo Abeni <[email protected]>
Date: Thu, 8 Feb 2024 19:03:50 +0100
Subject: [PATCH] mptcp: fix rcv space initialization

mptcp_rcv_space_init() is supposed to happen under the msk socket lock, but active msk socket does that without such protection.

Leverage the existing mptcp_propagate_state() helper to that extent. We need to ensure mptcp_rcv_space_init will happen before mptcp_rcv_space_adjust(), and the release_cb does not assure that:
explicitly check for such condition.

Quoted 5 lines

While at it, move the wnd_end initialization out of mptcp_rcv_space_init(), it never belonged there.

Note that the race does not produce ill effect in practice, but change allows cleaning-up and defying better the locking model.

FYI, because of the various conflicts, and because the race explained here "does not produce ill effects in practice", with Paolo, we think it is best not to backport this patch to v6.1 and older.

Cheers,
Matt

Signature · 1 line
Sponsored by the NGI0 Core fund.