public inbox for stable@vger.kernel.org
 help / color / mirror / Atom feed
* FAILED: patch "[PATCH] mptcp: don't always assume copied data in" failed to apply to 5.15-stable tree
@ 2025-01-06 12:28 gregkh
  2025-01-22 11:15 ` [PATCH 5.15.y] mptcp: don't always assume copied data in mptcp_cleanup_rbuf() Matthieu Baerts (NGI0)
  0 siblings, 1 reply; 3+ messages in thread
From: gregkh @ 2025-01-06 12:28 UTC (permalink / raw)
  To: pabeni, kuba, martineau, matttbe; +Cc: stable


The patch below does not apply to the 5.15-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 <stable@vger.kernel.org>.

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-5.15.y
git checkout FETCH_HEAD
git cherry-pick -x 551844f26da2a9f76c0a698baaffa631d1178645
# <resolve conflicts, build, test, etc.>
git commit -s
git send-email --to '<stable@vger.kernel.org>' --in-reply-to '2025010605-obscurity-buckshot-fc5f@gregkh' --subject-prefix 'PATCH 5.15.y' HEAD^..

Possible dependencies:



thanks,

greg k-h

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

From 551844f26da2a9f76c0a698baaffa631d1178645 Mon Sep 17 00:00:00 2001
From: Paolo Abeni <pabeni@redhat.com>
Date: Mon, 30 Dec 2024 19:12:31 +0100
Subject: [PATCH] mptcp: don't always assume copied data in
 mptcp_cleanup_rbuf()

Under some corner cases the MPTCP protocol can end-up invoking
mptcp_cleanup_rbuf() when no data has been copied, but such helper
assumes the opposite condition.

Explicitly drop such assumption and performs the costly call only
when strictly needed - before releasing the msk socket lock.

Fixes: fd8976790a6c ("mptcp: be careful on MPTCP-level ack.")
Cc: stable@vger.kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Mat Martineau <martineau@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Link: https://patch.msgid.link/20241230-net-mptcp-rbuf-fixes-v1-2-8608af434ceb@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 27afdb7e2071..5307fff9d995 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -528,13 +528,13 @@ static void mptcp_send_ack(struct mptcp_sock *msk)
 		mptcp_subflow_send_ack(mptcp_subflow_tcp_sock(subflow));
 }
 
-static void mptcp_subflow_cleanup_rbuf(struct sock *ssk)
+static void mptcp_subflow_cleanup_rbuf(struct sock *ssk, int copied)
 {
 	bool slow;
 
 	slow = lock_sock_fast(ssk);
 	if (tcp_can_send_ack(ssk))
-		tcp_cleanup_rbuf(ssk, 1);
+		tcp_cleanup_rbuf(ssk, copied);
 	unlock_sock_fast(ssk, slow);
 }
 
@@ -551,7 +551,7 @@ static bool mptcp_subflow_could_cleanup(const struct sock *ssk, bool rx_empty)
 			      (ICSK_ACK_PUSHED2 | ICSK_ACK_PUSHED)));
 }
 
-static void mptcp_cleanup_rbuf(struct mptcp_sock *msk)
+static void mptcp_cleanup_rbuf(struct mptcp_sock *msk, int copied)
 {
 	int old_space = READ_ONCE(msk->old_wspace);
 	struct mptcp_subflow_context *subflow;
@@ -559,14 +559,14 @@ static void mptcp_cleanup_rbuf(struct mptcp_sock *msk)
 	int space =  __mptcp_space(sk);
 	bool cleanup, rx_empty;
 
-	cleanup = (space > 0) && (space >= (old_space << 1));
-	rx_empty = !__mptcp_rmem(sk);
+	cleanup = (space > 0) && (space >= (old_space << 1)) && copied;
+	rx_empty = !__mptcp_rmem(sk) && copied;
 
 	mptcp_for_each_subflow(msk, subflow) {
 		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
 
 		if (cleanup || mptcp_subflow_could_cleanup(ssk, rx_empty))
-			mptcp_subflow_cleanup_rbuf(ssk);
+			mptcp_subflow_cleanup_rbuf(ssk, copied);
 	}
 }
 
@@ -2220,9 +2220,6 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 
 		copied += bytes_read;
 
-		/* be sure to advertise window change */
-		mptcp_cleanup_rbuf(msk);
-
 		if (skb_queue_empty(&msk->receive_queue) && __mptcp_move_skbs(msk))
 			continue;
 
@@ -2271,6 +2268,7 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 		}
 
 		pr_debug("block timeout %ld\n", timeo);
+		mptcp_cleanup_rbuf(msk, copied);
 		err = sk_wait_data(sk, &timeo, NULL);
 		if (err < 0) {
 			err = copied ? : err;
@@ -2278,6 +2276,8 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 		}
 	}
 
+	mptcp_cleanup_rbuf(msk, copied);
+
 out_err:
 	if (cmsg_flags && copied >= 0) {
 		if (cmsg_flags & MPTCP_CMSG_TS)


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

* [PATCH 5.15.y] mptcp: don't always assume copied data in mptcp_cleanup_rbuf()
  2025-01-06 12:28 FAILED: patch "[PATCH] mptcp: don't always assume copied data in" failed to apply to 5.15-stable tree gregkh
@ 2025-01-22 11:15 ` Matthieu Baerts (NGI0)
  2025-01-22 14:16   ` Sasha Levin
  0 siblings, 1 reply; 3+ messages in thread
From: Matthieu Baerts (NGI0) @ 2025-01-22 11:15 UTC (permalink / raw)
  To: stable, gregkh
  Cc: MPTCP Upstream, Paolo Abeni, Mat Martineau,
	Matthieu Baerts (NGI0), Jakub Kicinski

From: Paolo Abeni <pabeni@redhat.com>

commit 551844f26da2a9f76c0a698baaffa631d1178645 upstream.

Under some corner cases the MPTCP protocol can end-up invoking
mptcp_cleanup_rbuf() when no data has been copied, but such helper
assumes the opposite condition.

Explicitly drop such assumption and performs the costly call only
when strictly needed - before releasing the msk socket lock.

Fixes: fd8976790a6c ("mptcp: be careful on MPTCP-level ack.")
Cc: stable@vger.kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Mat Martineau <martineau@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Link: https://patch.msgid.link/20241230-net-mptcp-rbuf-fixes-v1-2-8608af434ceb@kernel.org
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
[ Conflicts in this version, because commit 581302298524 ("mptcp: error
  out earlier on disconnect") has not been backported to this version,
  and there was no need to do so. The only conflict was in protocol.c,
  and easy to resolve: the context was different, but the same addition
  can still be made at the same spot in mptcp_recvmsg(). ]
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/protocol.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index bcbb1f92ce24..f337dd3323a0 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -462,13 +462,13 @@ static void mptcp_send_ack(struct mptcp_sock *msk)
 		mptcp_subflow_send_ack(mptcp_subflow_tcp_sock(subflow));
 }
 
-static void mptcp_subflow_cleanup_rbuf(struct sock *ssk)
+static void mptcp_subflow_cleanup_rbuf(struct sock *ssk, int copied)
 {
 	bool slow;
 
 	slow = lock_sock_fast(ssk);
 	if (tcp_can_send_ack(ssk))
-		tcp_cleanup_rbuf(ssk, 1);
+		tcp_cleanup_rbuf(ssk, copied);
 	unlock_sock_fast(ssk, slow);
 }
 
@@ -485,7 +485,7 @@ static bool mptcp_subflow_could_cleanup(const struct sock *ssk, bool rx_empty)
 			      (ICSK_ACK_PUSHED2 | ICSK_ACK_PUSHED)));
 }
 
-static void mptcp_cleanup_rbuf(struct mptcp_sock *msk)
+static void mptcp_cleanup_rbuf(struct mptcp_sock *msk, int copied)
 {
 	int old_space = READ_ONCE(msk->old_wspace);
 	struct mptcp_subflow_context *subflow;
@@ -493,14 +493,14 @@ static void mptcp_cleanup_rbuf(struct mptcp_sock *msk)
 	int space =  __mptcp_space(sk);
 	bool cleanup, rx_empty;
 
-	cleanup = (space > 0) && (space >= (old_space << 1));
-	rx_empty = !__mptcp_rmem(sk);
+	cleanup = (space > 0) && (space >= (old_space << 1)) && copied;
+	rx_empty = !__mptcp_rmem(sk) && copied;
 
 	mptcp_for_each_subflow(msk, subflow) {
 		struct sock *ssk = mptcp_subflow_tcp_sock(subflow);
 
 		if (cleanup || mptcp_subflow_could_cleanup(ssk, rx_empty))
-			mptcp_subflow_cleanup_rbuf(ssk);
+			mptcp_subflow_cleanup_rbuf(ssk, copied);
 	}
 }
 
@@ -2098,9 +2098,6 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 
 		copied += bytes_read;
 
-		/* be sure to advertise window change */
-		mptcp_cleanup_rbuf(msk);
-
 		if (skb_queue_empty(&msk->receive_queue) && __mptcp_move_skbs(msk))
 			continue;
 
@@ -2152,9 +2149,12 @@ static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
 		}
 
 		pr_debug("block timeout %ld\n", timeo);
+		mptcp_cleanup_rbuf(msk, copied);
 		sk_wait_data(sk, &timeo, NULL);
 	}
 
+	mptcp_cleanup_rbuf(msk, copied);
+
 out_err:
 	if (cmsg_flags && copied >= 0) {
 		if (cmsg_flags & MPTCP_CMSG_TS)
-- 
2.47.1


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

* Re: [PATCH 5.15.y] mptcp: don't always assume copied data in mptcp_cleanup_rbuf()
  2025-01-22 11:15 ` [PATCH 5.15.y] mptcp: don't always assume copied data in mptcp_cleanup_rbuf() Matthieu Baerts (NGI0)
@ 2025-01-22 14:16   ` Sasha Levin
  0 siblings, 0 replies; 3+ messages in thread
From: Sasha Levin @ 2025-01-22 14:16 UTC (permalink / raw)
  To: stable; +Cc: Matthieu Baerts (NGI0), Sasha Levin

[ Sasha's backport helper bot ]

Hi,

The upstream commit SHA1 provided is correct: 551844f26da2a9f76c0a698baaffa631d1178645

WARNING: Author mismatch between patch and upstream commit:
Backport author: "Matthieu Baerts (NGI0)"<matttbe@kernel.org>
Commit author: Paolo Abeni<pabeni@redhat.com>


Status in newer kernel trees:
6.12.y | Present (different SHA1: 10a641cad465)
6.6.y | Present (different SHA1: f61e663d78ff)
6.1.y | Present (different SHA1: 91b493f15d65)
5.15.y | Not found

Note: The patch differs from the upstream commit:
---
1:  551844f26da2a ! 1:  3fe05f72a15ff mptcp: don't always assume copied data in mptcp_cleanup_rbuf()
    @@ Metadata
      ## Commit message ##
         mptcp: don't always assume copied data in mptcp_cleanup_rbuf()
     
    +    commit 551844f26da2a9f76c0a698baaffa631d1178645 upstream.
    +
         Under some corner cases the MPTCP protocol can end-up invoking
         mptcp_cleanup_rbuf() when no data has been copied, but such helper
         assumes the opposite condition.
    @@ Commit message
         Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
         Link: https://patch.msgid.link/20241230-net-mptcp-rbuf-fixes-v1-2-8608af434ceb@kernel.org
         Signed-off-by: Jakub Kicinski <kuba@kernel.org>
    +    [ Conflicts in this version, because commit 581302298524 ("mptcp: error
    +      out earlier on disconnect") has not been backported to this version,
    +      and there was no need to do so. The only conflict was in protocol.c,
    +      and easy to resolve: the context was different, but the same addition
    +      can still be made at the same spot in mptcp_recvmsg(). ]
    +    Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
     
      ## net/mptcp/protocol.c ##
     @@ net/mptcp/protocol.c: static void mptcp_send_ack(struct mptcp_sock *msk)
    @@ net/mptcp/protocol.c: static int mptcp_recvmsg(struct sock *sk, struct msghdr *m
      
      		pr_debug("block timeout %ld\n", timeo);
     +		mptcp_cleanup_rbuf(msk, copied);
    - 		err = sk_wait_data(sk, &timeo, NULL);
    - 		if (err < 0) {
    - 			err = copied ? : err;
    -@@ net/mptcp/protocol.c: static int mptcp_recvmsg(struct sock *sk, struct msghdr *msg, size_t len,
    - 		}
    + 		sk_wait_data(sk, &timeo, NULL);
      	}
      
     +	mptcp_cleanup_rbuf(msk, copied);
---

Results of testing on various branches:

| Branch                    | Patch Apply | Build Test |
|---------------------------|-------------|------------|
| stable/linux-5.15.y       |  Success    |  Success   |

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

end of thread, other threads:[~2025-01-22 14:16 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-01-06 12:28 FAILED: patch "[PATCH] mptcp: don't always assume copied data in" failed to apply to 5.15-stable tree gregkh
2025-01-22 11:15 ` [PATCH 5.15.y] mptcp: don't always assume copied data in mptcp_cleanup_rbuf() Matthieu Baerts (NGI0)
2025-01-22 14:16   ` Sasha Levin

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox