[PATCH net v2 00/15] mptcp: misc fixes for v7.3-rc1

[PATCH net v2 00/15] mptcp: misc fixes for v7.3-rc1

[Matthieu Baerts (NGI0)]

Here are various unrelated fixes:

- Patch 1: Do not reschedule the RTX timer for sockets that fell back to
  TCP. A fix for v5.7.

- Patch 2: Avoid copying thmac which will not be used and could be
  uninitialised. A fix for v5.7.

- Patch 3: Re-set the request backup flag when SYN cookies are used. A
  fix for v5.9.

- Patch 4: Drop pending ADD_ADDR when removing ID0, and avoid a WARN. A
  fix for v5.13.

- Patch 5: Handle invalid suboptions where the checksum is requested in
  the MP_CAPABLE 4th ACK with data, but not added in the option. A fix
  for v5.14.

- Patch 6: Prevent a race between mptcp_disconnect() and the retransmit
  timer. A fix for v5.17.

- Patch 7: Fix a use-after-free in the selftests that could lead to
  false positive. A fix for v5.17.

- Patch 8: Limit new addresses with the userspace PM to avoid an address
  ID overflow. A fix for v5.19.

- Patch 9: Reset the ADD_ADDR retransmission counter when the timer is
  reused. A fix for v5.19.

- Patch 10: Remove unneeded and confusing READ_ONCE() annotations. A fix
  for v6.13.

- Patches 11-12: Get nstat counters for the current test, not since the
  creation of the netns. A fix for v6.19.

- Patch 13: Fix an uninit-value in mptcp_write_data_fin for a corner
  case now that only a part of the tcp_out_options struct is reset. A
  fix for v7.1.

- Patches 14-15: Two follow-up patches addressing minor comments
  discovered after the human review. A fix for v7.3-rc1.

Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Changes in v2:
- Patch 1: Invert RTX disabled/enabled logic to catch more cases.
- Patch 2: Note for Sashiko's false positive.
- Patch 3: Update Fixes tag.
- Patch 4: Fix new helpers name + note.
- Patch 5: Note for Clashiko.
- Patch 6: New, addressing a previous comment from Clashiko.
- Patch 10: Remove Cc stable.
- Note: I guess I should stop mentioning the version target: because of
  Clashiko's reviews delaying the fixes, it is hard to predict it.
- Link to v1: https://patch.msgid.link/20260824-net-mptcp-misc-fixes-7-3-rc1-v1-0-a92309d121ca@kernel.org

---
Gang Yan (1):
      selftests: mptcp: fix an UAF in mptcp_connect.c

Kalpan Jani (1):
      mptcp: pm: kernel: drop pending ADD_ADDR when removing ID0

Matthieu Baerts (NGI0) (7):
      mptcp: subflow: no need to copy thmac during ulp_clone
      mptcp: syncookies: remember the request backup flag
      mptcp: options: handle MPC data + csum reqd + no csum
      mptcp: pm: reset retrans_time when ADD_ADDR entry is reused
      selftests: mptcp: lib: dump nstat for the right test
      selftests: mptcp: lib: get counters for the right test
      mptcp: options: fix uninit-value in mptcp_write_data_fin

Paolo Abeni (5):
      mptcp: do not reschedule the RTX timer for fallback sockets
      mptcp: prevent race between disconnect() and rtx
      mptcp: remove unneeded READ_ONCE() annotation
      mptcp: being below memory limit is a likely() condition
      mptcp: avoid pruning for OoW data

Qing Luo (1):
      mptcp: pm: userspace: fix address ID overflow

 net/mptcp/options.c                               |  8 +++--
 net/mptcp/pm.c                                    |  2 +-
 net/mptcp/pm_kernel.c                             |  8 +++++
 net/mptcp/pm_userspace.c                          | 18 +++++++---
 net/mptcp/protocol.c                              | 41 +++++++++++++++--------
 net/mptcp/protocol.h                              |  1 +
 net/mptcp/subflow.c                               |  1 -
 net/mptcp/syncookies.c                            |  5 ++-
 tools/testing/selftests/net/mptcp/mptcp_connect.c |  4 ++-
 tools/testing/selftests/net/mptcp/mptcp_lib.sh    | 26 ++++++++------
 10 files changed, 78 insertions(+), 36 deletions(-)
---
base-commit: e0554c6276da957b6e72849520c70a97404cd1ae
change-id: 20260824-net-mptcp-misc-fixes-7-3-rc1-6a4d6223ec41

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

[PATCH net v2 01/15] mptcp: do not reschedule the RTX timer for fallback sockets

[Matthieu Baerts (NGI0)]

From: Paolo Abeni <pabeni@redhat.com>

On fallback socket the retrans timer is a quite convoluted no-op, but
currently nothing prevents the MPTCP core to keep rescheduling it.

Additionally gate RTX timer reset to the msk not being fallen back to
TCP yet. To avoid adding multiple tests in fast-path, use a new flags
bit for such condition.

The RTX enable bit is clear at close time and set before the msk could
start retransmitting, with a couple of caveats:

- passive sockets inherit the bit from the listener msk; set the bit on
  such socket to avoid flipping it in the fast-path, even if the
  listener will obviously never retransmit.

- while fastopening (MPTFO), mptcp_sendmsg_fastopen still ends-up
  calling mptcp_connect via tcp_sendmsg_fastopen ->
  __inet_stream_connect(ssk->sk_socket), and the first subflow's
  sk_socket points to the msk one.

Fixes: b51f9b80c032 ("mptcp: introduce MPTCP retransmission timer")
Cc: stable@vger.kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
v2:
 - Sashiko noted that the 'RTX disabled' status is carried over across
   connect() failures, potentially to subsequent successful connect()
   or listen().
   Explicitly control the RTX enabling status across the whole msk
   life-cycle. To make the code more straight forward switch the newly
   introduced flag semantic.
---
 net/mptcp/protocol.c | 13 ++++++++++---
 net/mptcp/protocol.h |  1 +
 2 files changed, 11 insertions(+), 3 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index e1f08f71cdb1..be59651e708e 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -95,6 +95,7 @@ bool __mptcp_try_fallback(struct mptcp_sock *msk, int fb_mib)
 
 	msk->allow_subflows = false;
 	set_bit(MPTCP_FALLBACK_DONE, &msk->flags);
+	clear_bit(MPTCP_RTX_ENABLED, &msk->flags);
 	__MPTCP_INC_STATS(net, fb_mib);
 	spin_unlock_bh(&msk->fallback_lock);
 	return true;
@@ -1084,13 +1085,14 @@ static bool mptcp_rtx_timer_pending(struct sock *sk)
 
 static void mptcp_reset_rtx_timer(struct sock *sk)
 {
+	struct mptcp_sock *msk = mptcp_sk(sk);
 	unsigned long tout;
 
-	/* prevent rescheduling on close */
-	if (unlikely(inet_sk_state_load(sk) == TCP_CLOSE))
+	/* Prevent rescheduling on close and in case of fallback. */
+	if (!test_bit(MPTCP_RTX_ENABLED, &msk->flags))
 		return;
 
-	tout = mptcp_sk(sk)->timer_ival;
+	tout = msk->timer_ival;
 	sk_reset_timer(sk, &sk->mptcp_retransmit_timer, jiffies + tout);
 }
 
@@ -3323,6 +3325,9 @@ void mptcp_set_state(struct sock *sk, int state)
 		 * transition from TCP_SYN_RECV to TCP_CLOSE_WAIT.
 		 */
 		break;
+	case TCP_CLOSE:
+		clear_bit(MPTCP_RTX_ENABLED, &mptcp_sk(sk)->flags);
+		fallthrough;
 	default:
 		if (oldstate == TCP_ESTABLISHED || oldstate == TCP_CLOSE_WAIT)
 			MPTCP_DEC_STATS(sock_net(sk), MPTCP_MIB_CURRESTAB);
@@ -4141,6 +4146,7 @@ static int mptcp_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
 	if (IS_ERR(ssk))
 		return PTR_ERR(ssk);
 
+	set_bit(MPTCP_RTX_ENABLED, &msk->flags);
 	mptcp_set_state(sk, TCP_SYN_SENT);
 	subflow = mptcp_subflow_ctx(ssk);
 #ifdef CONFIG_TCP_MD5SIG
@@ -4288,6 +4294,7 @@ static int mptcp_listen(struct socket *sock, int backlog)
 		goto unlock;
 	}
 
+	set_bit(MPTCP_RTX_ENABLED, &msk->flags);
 	mptcp_set_state(sk, TCP_LISTEN);
 	sock_set_flag(sk, SOCK_RCU_FREE);
 
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 87ccb84e9927..2b4c27426477 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -116,6 +116,7 @@
 #define MPTCP_WORK_RTX		1
 #define MPTCP_FALLBACK_DONE	2
 #define MPTCP_WORK_CLOSE_SUBFLOW 3
+#define MPTCP_RTX_ENABLED	4
 
 /* MPTCP socket release cb flags */
 #define MPTCP_PUSH_PENDING	1

-- 
2.55.0

Re: [PATCH net v2 01/15] mptcp: do not reschedule the RTX timer for fallback sockets

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] Incomplete gate: mptcp_connect() (net/mptcp/protocol.c:4149)
  unconditionally does set_bit(MPTCP_RTX_ENABLED, &msk->flags) before…

--

mptcp: do not reschedule the RTX timer for fallback sockets

This adds a new MPTCP_RTX_ENABLED flag bit so that mptcp_reset_rtx_timer()
only arms the MPTCP retransmission timer while the msk has not fallen back
to plain TCP and is not closed.  The bit is set in mptcp_connect() and
mptcp_listen(), and cleared on fallback and on the transition to TCP_CLOSE.

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index e1f08f71cdb1..be59651e708e 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -95,6 +95,7 @@ bool __mptcp_try_fallback(struct mptcp_sock *msk, int fb_mib)
>  
>  	msk->allow_subflows = false;
>  	set_bit(MPTCP_FALLBACK_DONE, &msk->flags);
> +	clear_bit(MPTCP_RTX_ENABLED, &msk->flags);
>  	__MPTCP_INC_STATS(net, fb_mib);
>  	spin_unlock_bh(&msk->fallback_lock);
>  	return true;
> @@ -1084,13 +1085,14 @@ static bool mptcp_rtx_timer_pending(struct sock *sk)
>  
>  static void mptcp_reset_rtx_timer(struct sock *sk)
>  {
> +	struct mptcp_sock *msk = mptcp_sk(sk);
>  	unsigned long tout;
>  
> -	/* prevent rescheduling on close */
> -	if (unlikely(inet_sk_state_load(sk) == TCP_CLOSE))
> +	/* Prevent rescheduling on close and in case of fallback. */
> +	if (!test_bit(MPTCP_RTX_ENABLED, &msk->flags))
>  		return;
>  
> -	tout = mptcp_sk(sk)->timer_ival;
> +	tout = msk->timer_ival;
>  	sk_reset_timer(sk, &sk->mptcp_retransmit_timer, jiffies + tout);
>  }
>  

[ ... ]

> @@ -4141,6 +4146,7 @@ static int mptcp_connect(struct sock *sk, struct sockaddr_unsized *uaddr,
>  	if (IS_ERR(ssk))
>  		return PTR_ERR(ssk);
>  
> +	set_bit(MPTCP_RTX_ENABLED, &msk->flags);
>  	mptcp_set_state(sk, TCP_SYN_SENT);
>  	subflow = mptcp_subflow_ctx(ssk);

[Severity: Medium]
Can this set_bit() re-enable the RTX timer on a msk that is already in
fallback?  It runs unconditionally, before any of the fallback checks below
it, and the fallback state is sticky across a failed connect().

Sequence that seems reachable from userspace on its own fd:

1) First connect(): mptcp_active_should_disable() is true (or
   mptcp_token_new_connect() fails), so mptcp_connect() calls
   mptcp_early_fallback() -> __mptcp_try_fallback(), which sets
   MPTCP_FALLBACK_DONE, clears MPTCP_RTX_ENABLED and leaves
   subflow->request_mptcp == 0.

2) ssk->sk_prot->connect() fails immediately (for instance -EAFNOSUPPORT on
   a bad sin_family, or -ENETUNREACH from the route lookup), so
   mptcp_connect() takes:

	if (unlikely(err)) {
		/* avoid leaving a dangling token in an unconnected socket */
		mptcp_token_destroy(msk);
		mptcp_set_state(sk, TCP_CLOSE);
		return err;
	}

   MPTCP_RTX_ENABLED is cleared again by the TCP_CLOSE transition, but
   MPTCP_FALLBACK_DONE stays set.

3) __inet_stream_connect() returns the error directly:

	err = sk->sk_prot->connect(sk, uaddr, addr_len);
	if (err < 0)
		goto out;

   so sk->sk_prot->disconnect() (mptcp_disconnect(), which does
   WRITE_ONCE(msk->flags, 0)) is never called, and the socket stays
   SS_UNCONNECTED with sk_state == TCP_CLOSE.

4) Userspace calls connect() again.  mptcp_connect() sets
   MPTCP_RTX_ENABLED unconditionally, and since request_mptcp is already 0
   the fallback branches are skipped.  Even if mptcp_early_fallback() were
   reached, __mptcp_try_fallback() returns early at:

	if (__mptcp_check_fallback(msk))
		return true;

   before the new clear_bit(MPTCP_RTX_ENABLED, ...).

The result is a live fallback msk with MPTCP_RTX_ENABLED set, so
mptcp_reset_rtx_timer() passes the new gate and the send paths
(__mptcp_push_pending() and friends) arm the MPTCP RTX timer just as they
did before the patch:

		if (!mptcp_rtx_timer_pending(sk))
			mptcp_reset_rtx_timer(sk);

Would gating the set_bit() on !__mptcp_check_fallback(msk), or moving it
below the early-fallback checks, close this case and also make the new
comment "Prevent rescheduling on close and in case of fallback" hold for a
msk whose fallback predates the current connect()?

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908-net-mptcp-misc-fixes-7-3-rc1-v2-0-df1de70348b6%40kernel.org

Re: [PATCH net v2 01/15] mptcp: do not reschedule the RTX timer for fallback sockets

[Paolo Abeni]

On 9/9/26 4:49 PM, netdev-bot+sashiko@kernel.org wrote:
>> @@ -4141,6 +4146,7 @@ static int mptcp_connect(struct sock *sk,
struct sockaddr_unsized *uaddr,
>>  	if (IS_ERR(ssk))
>>  		return PTR_ERR(ssk);
>>
>> +	set_bit(MPTCP_RTX_ENABLED, &msk->flags);
>>  	mptcp_set_state(sk, TCP_SYN_SENT);
>>  	subflow = mptcp_subflow_ctx(ssk);
>
> [Severity: Medium]
> Can this set_bit() re-enable the RTX timer on a msk that is already in
> fallback?  It runs unconditionally, before any of the fallback checks
> below it, and the fallback state is sticky across a failed connect().

This is an unrelated/different issue: when (mptcp_)connect() fails early
it returns a TCP_CLOSE/SS_DISCONNECTED socket but the status is not
fully cleared. Should be handled with a follow-up/unrelated series

> Would gating the set_bit() on !__mptcp_check_fallback(msk), or moving
> it below the early-fallback checks, close this case and also make the
> new comment "Prevent rescheduling on close and in case of fallback"
> hold for a msk whose fallback predates the current connect()?

FTR, I can be low of coffee, but the above suggestion is crazy wrong.

/P

[PATCH net v2 02/15] mptcp: subflow: no need to copy thmac during ulp_clone

[Matthieu Baerts (NGI0)]

'thmac' is not used after that point.

Indeed, subflow_ulp_clone() is called when the request on the passive
side is over, so when the truncated HMAC is no longer needed.

Note that in case of SYN cookies, thmac will not be initialised. So
better to remove it to avoid a warning from debug tools like KMSAN for
reading uninitialised data.

Fixes: f296234c98a8 ("mptcp: Add handling of incoming MP_JOIN requests")
Cc: stable@vger.kernel.org
Reviewed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Cc: Florian Westphal <fw@strlen.de>
Note: Peter Krystad's email address is bouncing.
@Sashiko: yes request_bkup's initialisation is not fixed in this commit
          but in the next one, no need to complain about that: different
          Fixes tag...
---
 net/mptcp/subflow.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index af81ad5e699d..01db7edce18a 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -2084,7 +2084,6 @@ static void subflow_ulp_clone(const struct request_sock *req,
 		new_ctx->request_bkup = subflow_req->request_bkup;
 		WRITE_ONCE(new_ctx->remote_id, subflow_req->remote_id);
 		new_ctx->token = subflow_req->token;
-		new_ctx->thmac = subflow_req->thmac;
 
 		/* the subflow req id is valid, fetched via subflow_check_req()
 		 * and subflow_token_join_request()

-- 
2.55.0

[PATCH net v2 03/15] mptcp: syncookies: remember the request backup flag

[Matthieu Baerts (NGI0)]

Instead of using an uninitialised bit when copying the info in
subflow_ulp_clone().

To fix this, no need to extend the join_entry structure: backup is
coming from struct mptcp_subflow_request_sock, only one bit. Do the same
here by using one bit for both.

Fixes: efd340bf3d77 ("mptcp: distinguish rcv vs sent backup flag in requests")
Cc: stable@vger.kernel.org
Reviewed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
v2: update the Fixes tag (even if the target version is the same)
---
 net/mptcp/syncookies.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/syncookies.c b/net/mptcp/syncookies.c
index b5cac5701122..9474706641c1 100644
--- a/net/mptcp/syncookies.c
+++ b/net/mptcp/syncookies.c
@@ -26,7 +26,8 @@ struct join_entry {
 	u32 local_nonce;
 	u8 join_id;
 	u8 local_id;
-	u8 backup;
+	u8 backup:1,
+	   request_bkup:1;
 	u8 valid;
 };
 
@@ -63,6 +64,7 @@ static void mptcp_join_store_state(struct join_entry *entry,
 	entry->remote_nonce = subflow_req->remote_nonce;
 	entry->local_nonce = subflow_req->local_nonce;
 	entry->backup = subflow_req->backup;
+	entry->request_bkup = subflow_req->request_bkup;
 	entry->join_id = subflow_req->remote_id;
 	entry->local_id = subflow_req->local_id;
 	entry->valid = 1;
@@ -117,6 +119,7 @@ bool mptcp_token_join_cookie_init_state(struct mptcp_subflow_request_sock *subfl
 	subflow_req->remote_nonce = e->remote_nonce;
 	subflow_req->local_nonce = e->local_nonce;
 	subflow_req->backup = e->backup;
+	subflow_req->request_bkup = e->request_bkup;
 	subflow_req->remote_id = e->join_id;
 	subflow_req->local_id = e->local_id;
 	subflow_req->token = e->token;

-- 
2.55.0

[PATCH net v2 04/15] mptcp: pm: kernel: drop pending ADD_ADDR when removing ID0

[Matthieu Baerts (NGI0)]

From: Kalpan Jani <kalpan.jani@mpiricsoftware.com>

The in-kernel MPTCP path manager can leave a stale ADD_ADDR announcement
entry alive when removing the id 0 endpoint. This happens because the id 0
removal path does not tear down pending announcements, unlike the non-zero
id path.

When the PM later reselects id 0 after adding another signal endpoint, it
finds the stale anno_list entry and hits WARN_ON_ONCE(mptcp_pm_is_kernel())
in mptcp_pm_announced_alloc().

Root cause: asymmetry between removal paths.
- Non-zero id path: mptcp_nl_remove_subflow_and_signal_addr() calls
  mptcp_pm_remove_announced() to clean up.
- Id 0 path: mptcp_nl_remove_id_zero_address() skips cleanup entirely.

Fix by making the id 0 path symmetric: call mptcp_pm_announced_remove()
and decrement add_addr_signaled before queuing the RM_ADDR.

Subtle detail: signal endpoints are stored in anno_list with port 0, but
msk_local carries the connection's local port. In other words, entries
linked to ID0 paths should have port == 0. A follow-up patch will ensure
that. mptcp_pm_announced_remove() uses use_port=true for comparison. So
clear the port before the lookup.

Fixes: 740d798e8767 ("mptcp: remove id 0 address")
Cc: stable@vger.kernel.org
Reported-by: syzbot+55c2a5c871441261ed14@syzkaller.appspotmail.com
Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/620
Suggested-by: Tao Cui <cuitao@kylinos.cn>
Signed-off-by: Kalpan Jani <kalpan.jani@mpiricsoftware.com>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
v2: use new helpers name, note about "port = 0".
---
 net/mptcp/pm_kernel.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index 424f1a7f9248..1a7750813235 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -1137,6 +1137,8 @@ static int mptcp_nl_remove_id_zero_address(struct net *net,
 	while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
 		struct sock *sk = (struct sock *)msk;
 		struct mptcp_addr_info msk_local;
+		struct mptcp_addr_info anno_addr;
+		bool announced;
 
 		if (list_empty(&msk->conn_list) || mptcp_pm_is_userspace(msk))
 			goto next;
@@ -1146,7 +1148,13 @@ static int mptcp_nl_remove_id_zero_address(struct net *net,
 			goto next;
 
 		lock_sock(sk);
+		/* Drop a possibly pending ADD_ADDR for this address. */
+		anno_addr = msk_local;
+		anno_addr.port = 0;
+		announced = mptcp_pm_announced_remove(msk, &anno_addr);
 		spin_lock_bh(&msk->pm.lock);
+		if (announced)
+			msk->pm.add_addr_signaled--;
 		mptcp_pm_remove_addr(msk, &list);
 		mptcp_pm_rm_subflow(msk, &list);
 		__mark_subflow_endp_available(msk, 0);

-- 
2.55.0

Re: [PATCH net v2 04/15] mptcp: pm: kernel: drop pending ADD_ADDR when removing ID0

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 1 · Medium: 1 · Low: 0

- [High] Incomplete fix: in mptcp_nl_remove_id_zero_address()
  (net/mptcp/pm_kernel.c) the new cleanup forces `anno_addr.port = 0`…
- [Medium] Newly introduced wrong-entry teardown in
  mptcp_nl_remove_id_zero_address() (net/mptcp/pm_kernel.c:1152-1157):…

--

mptcp: pm: kernel: drop pending ADD_ADDR when removing ID0

This makes the in-kernel path manager's id 0 removal path symmetric with the
non-zero id path by dropping a pending ADD_ADDR announcement and decrementing
add_addr_signaled before queuing the RM_ADDR.  The lookup key is the msk local
address with the port forced to 0.

Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/620

> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
> index 424f1a7f9248..1a7750813235 100644
> --- a/net/mptcp/pm_kernel.c
> +++ b/net/mptcp/pm_kernel.c
> @@ -1137,6 +1137,8 @@ static int mptcp_nl_remove_id_zero_address(struct net *net,
>  	while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
>  		struct sock *sk = (struct sock *)msk;
>  		struct mptcp_addr_info msk_local;
> +		struct mptcp_addr_info anno_addr;
> +		bool announced;
>  
>  		if (list_empty(&msk->conn_list) || mptcp_pm_is_userspace(msk))
>  			goto next;
> @@ -1146,7 +1148,13 @@ static int mptcp_nl_remove_id_zero_address(struct net *net,
>  			goto next;
>  
>  		lock_sock(sk);
> +		/* Drop a possibly pending ADD_ADDR for this address. */
> +		anno_addr = msk_local;
> +		anno_addr.port = 0;
> +		announced = mptcp_pm_announced_remove(msk, &anno_addr);

[Severity: High]
Does zeroing the port here make the lookup miss the announcements this
patch is meant to remove?

The commit message says:

    Subtle detail: signal endpoints are stored in anno_list with port 0, but
    msk_local carries the connection's local port. In other words, entries
    linked to ID0 paths should have port == 0. A follow-up patch will ensure
    that.

Since that invariant does not exist yet, a SIGNAL-only endpoint keeps its
port.  mptcp_pm_nl_append_new_local_addr() only clears it when
address_use_port() is false:

	if (!address_use_port(entry))
		entry->addr.port = 0;

and address_use_port() returns true for SIGNAL-without-SUBFLOW endpoints.
mptcp_pm_create_subflow_or_signal_addr() then copies that endpoint verbatim
from select_signal_address() and only rewrites the id:

		endp_id = local.addr.id;
		if (endp_id == msk->mpc_endpoint_id)
			local.addr.id = 0;

		if (!mptcp_pm_announced_alloc(msk, &local.addr))
			return;

so the anno_list entry can be {A, port P, id 0}.  The removal side compares
with the port:

net/mptcp/pm.c:mptcp_pm_announced_lookup() {
	list_for_each_entry(entry, &msk->pm.anno_list, list) {
		if (mptcp_addresses_equal(&entry->addr, addr, true))
			return entry;
	}
}

msk_local is A:P and would have matched that entry exactly, but the zeroed
key does not.  In that case announced stays false, the entry stays linked and
add_addr_signaled is never decremented.

__mark_subflow_endp_available(msk, 0) then re-sets the endpoint's id bit, so
after another signal endpoint is added the PM reselects the same endpoint,
rewrites its id to 0, and mptcp_pm_announced_alloc() still finds the stale
entry:

net/mptcp/pm.c:mptcp_pm_announced_alloc() {
	add_entry = mptcp_pm_announced_lookup(msk, addr);
	if (add_entry) {
		if (WARN_ON_ONCE(mptcp_pm_is_kernel(msk)))
			return false;
}

Is the syzbot-reported warning therefore still reachable for port-based
signal endpoints, both here and in the stable backport of this patch alone?
A diff over the rest of the series shows nothing later touches
mptcp_nl_remove_id_zero_address() or normalizes anno_list ports.

Would matching on the entry id 0, or passing msk_local unmodified, be closer
to what the non-zero id path does?

> +		spin_lock_bh(&msk->pm.lock);
> +		if (announced)
> +			msk->pm.add_addr_signaled--;
>  		mptcp_pm_remove_addr(msk, &list);

[Severity: Medium]
Can the address-only key match and free the pending ADD_ADDR of a different
endpoint?

mptcp_pm_announced_remove() passes check_id=false, and in that case the id of
the found entry is never verified before it is unlinked and freed:

net/mptcp/pm.c:mptcp_pm_announced_del_timer() {
	entry = mptcp_pm_announced_lookup(msk, addr);
	if (entry && (!check_id || entry->addr.id == addr->id)) {
		entry->retrans_times = ADD_ADDR_RETRANS_MAX;
		stop_timer = true;
	}
	if (!check_id && entry)
		list_del(&entry->list);
}

Two endpoints with the same IP can coexist when they differ only by port,
because mptcp_pm_nl_append_new_local_addr() compares them with:

		if (mptcp_addresses_equal(&cur->addr, &entry->addr,
					  cur->addr.port || entry->addr.port)) {

So with E1 = {A, port P, SIGNAL} as the msk's mpc/ID0 endpoint and
E2 = {A, port 0, SIGNAL, id N} owning a pending announcement stored as
{A, port 0, id N}, the zeroed key matches E2's entry and destroys it.  The
same happens if msk->mpc_endpoint_id stayed 0 because no endpoint matched
when mptcp_mpc_endpoint_setup() ran.

The RM_ADDR queued right below carries only id 0 (list.ids[0] = 0), so the
peer is never told about id N.  __mark_subflow_endp_available(msk, 0) only
sets the bit for msk->mpc_endpoint_id:

	if (!__test_and_set_bit(id ? : msk->mpc_endpoint_id, msk->pm.id_avail_bitmap) &&

so id N stays cleared in id_avail_bitmap and is never re-announced, and a
later DEL_ADDR of endpoint N takes mptcp_pm_remove_announced() with
announced == false.  Do the local and peer views of announced addresses
diverge permanently in that case?

The peer call site keeps the key and the signalled id paired:

net/mptcp/pm_kernel.c:mptcp_pm_remove_announced() {
	list.ids[list.nr++] = mptcp_endp_get_local_id(msk, addr);

	announced = mptcp_pm_announced_remove(msk, addr);

Would matching on the entry's id 0 instead of a fabricated port preserve
that pairing here too?  The id 0 path touched no anno_list entry before this
patch, and this collateral teardown is not covered in the commit message.

>  		mptcp_pm_rm_subflow(msk, &list);
>  		__mark_subflow_endp_available(msk, 0);

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908-net-mptcp-misc-fixes-7-3-rc1-v2-0-df1de70348b6%40kernel.org

Re: [PATCH net v2 04/15] mptcp: pm: kernel: drop pending ADD_ADDR when removing ID0

[Matthieu Baerts]

Hello,

On 09/09/2026 16:49, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider:
> 
> Critical: 0 · High: 1 · Medium: 1 · Low: 0
> 
> - [High] Incomplete fix: in mptcp_nl_remove_id_zero_address()
>   (net/mptcp/pm_kernel.c) the new cleanup forces `anno_addr.port = 0`…
> - [Medium] Newly introduced wrong-entry teardown in
>   mptcp_nl_remove_id_zero_address() (net/mptcp/pm_kernel.c:1152-1157):…
> 
> --
> 
> mptcp: pm: kernel: drop pending ADD_ADDR when removing ID0
> 
> This makes the in-kernel path manager's id 0 removal path symmetric with the
> non-zero id path by dropping a pending ADD_ADDR announcement and decrementing
> add_addr_signaled before queuing the RM_ADDR.  The lookup key is the msk local
> address with the port forced to 0.
> 
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/620
> 
>> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
>> index 424f1a7f9248..1a7750813235 100644
>> --- a/net/mptcp/pm_kernel.c
>> +++ b/net/mptcp/pm_kernel.c
>> @@ -1137,6 +1137,8 @@ static int mptcp_nl_remove_id_zero_address(struct net *net,
>>  	while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
>>  		struct sock *sk = (struct sock *)msk;
>>  		struct mptcp_addr_info msk_local;
>> +		struct mptcp_addr_info anno_addr;
>> +		bool announced;
>>  
>>  		if (list_empty(&msk->conn_list) || mptcp_pm_is_userspace(msk))
>>  			goto next;
>> @@ -1146,7 +1148,13 @@ static int mptcp_nl_remove_id_zero_address(struct net *net,
>>  			goto next;
>>  
>>  		lock_sock(sk);
>> +		/* Drop a possibly pending ADD_ADDR for this address. */
>> +		anno_addr = msk_local;
>> +		anno_addr.port = 0;
>> +		announced = mptcp_pm_announced_remove(msk, &anno_addr);
> 
> [Severity: High]
> Does zeroing the port here make the lookup miss the announcements this
> patch is meant to remove?
> 
> The commit message says:
> 
>     Subtle detail: signal endpoints are stored in anno_list with port 0, but
>     msk_local carries the connection's local port. In other words, entries
>     linked to ID0 paths should have port == 0. A follow-up patch will ensure
>     that.
As mentioned, a follow-up patch to fix this non-directly related issue
is in preparation. The series was already 15 patches long, it has not
been included and will be sent later on.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

[PATCH net v2 05/15] mptcp: options: handle MPC data + csum reqd + no csum

[Matthieu Baerts (NGI0)]

Before this modification, a remote peer could send an MP_CAPABLE with
data, with the checksum flag set, but without adding the actual 2 bytes
of checksum. As a result, uninitialised bytes could be used for the
'csum' field.

That was not a critical issue, because this 'csum' field is only used to
compare with the expected one, if previously negotiated in the 3WHS.
Worst case, the checksum is likely wrong, a fallback is done without a
reject if the negotiation was done earlier. That's OK.

Yet, better to take the expected path with this case: only look at the
checksum flag for MP_CAPABLEs not carrying a data-len.

Such packet can be seen as a 3rd or 4th ACK. The RFC8684 mentions [1]
that the 3rd packet should have the checksum flag set. When an MPC + ACK
contains data, the checksum flag is redundant with the checksum field.
It is not clear what should be done for the 4th ACK, nor if the flag has
to be set if the checksum field is set.

Therefore, it seems fine to only look at the presence of the checksum
field, not to break the interaction with stacks that were not setting
both.

Note that linked to this checksum flag on the 3rd ACK, with the current
implementation, we can have a situation where the SYN packets have no
checksum flag, but the 3rd ACK has one, and this is the one that will be
taken into account. First, that's clearly not directly linked to this
patch, but Clashiko forced us to look at that. At the end, that seems
fine to act like that: yes that's not how the negotiation should work,
but being flexible without introducing side effects is also fine: fixing
this would mean increasing the complexity, and that's not worth it.

Fixes: 208e8f66926c ("mptcp: receive checksum for MP_CAPABLE with data")
Cc: stable@vger.kernel.org
Link: https://datatracker.ietf.org/doc/html/rfc8684#section-3.1-23 [1]
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260803-net-mptcp-misc-fixes-7-2-rc6-v2-0-b8f496d71664%40kernel.org?part=1
Reviewed-by: Mat Martineau <martineau@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Note about a non directly related case spotted by Clashiko.
---
 net/mptcp/options.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index b8318e030138..92f27b9e087a 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -93,7 +93,8 @@ static void mptcp_parse_option(const struct sk_buff *skb,
 		 * In other words, the only way for checksums not to be used
 		 * is if both hosts in their SYNs set A=0."
 		 */
-		if (flags & MPTCP_CAP_CHECKSUM_REQD)
+		if ((flags & MPTCP_CAP_CHECKSUM_REQD) &&
+		    opsize < TCPOLEN_MPTCP_MPC_ACK_DATA)
 			mp_opt->suboptions |= OPTION_MPTCP_CSUMREQD;
 
 		mp_opt->deny_join_id0 = !!(flags & MPTCP_CAP_DENY_JOIN_ID0);

-- 
2.55.0

Re: [PATCH net v2 05/15] mptcp: options: handle MPC data + csum reqd + no csum

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 1 (1)

- [Low] Residual uninitialised read of `mp_opt->csum`: the patch closes
  the functional consumer of the uninitialised checksum…

Pre-existing issues:
- [Low] Reported concern (assessed as an intentional, documented
  behaviour change): a passive endpoint whose local checksum sysctl is…

--

mptcp: options: handle MPC data + csum reqd + no csum

The MP_CAPABLE parser now only honours the checksum required flag for
options that do not carry a data-len, so a peer that sets the flag on a
data-bearing MP_CAPABLE without appending the 2 bytes of checksum no
longer makes the receiver use an uninitialised csum field.

Link: https://datatracker.ietf.org/doc/html/rfc8684#section-3.1-23 [1]
Closes: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260803-net-mptcp-misc-fixes-7-2-rc6-v2-0-b8f496d71664%40kernel.org?part=1

> diff --git a/net/mptcp/options.c b/net/mptcp/options.c
> index b8318e030138..92f27b9e087a 100644
> --- a/net/mptcp/options.c
> +++ b/net/mptcp/options.c
> @@ -93,7 +93,8 @@ static void mptcp_parse_option(const struct sk_buff *skb,
>  		 * In other words, the only way for checksums not to be used
>  		 * is if both hosts in their SYNs set A=0."
>  		 */
> -		if (flags & MPTCP_CAP_CHECKSUM_REQD)
> +		if ((flags & MPTCP_CAP_CHECKSUM_REQD) &&
> +		    opsize < TCPOLEN_MPTCP_MPC_ACK_DATA)
>  			mp_opt->suboptions |= OPTION_MPTCP_CSUMREQD;

[Severity: Low]
The commit message says "uninitialised bytes could be used for the 'csum'
field".  Is the uninitialised csum field itself still read after this
change?

In mptcp_parse_option(), mp_opt->csum is only assigned in the 24-byte
form:

	if (opsize == TCPOLEN_MPTCP_MPC_ACK_DATA_CSUM) {
		mp_opt->csum = get_unaligned((__force __sum16 *)ptr);
		mp_opt->suboptions |= OPTION_MPTCP_CSUMREQD;
		ptr += 2;
	}
	pr_debug("MP_CAPABLE version=%x, flags=%x, optlen=%d sndr=%llu, rcvr=%llu len=%d csum=%u\n",
		 version, flags, opsize, mp_opt->sndr_key,
		 mp_opt->rcvr_key, mp_opt->data_len, mp_opt->csum);

but the pr_debug() passes mp_opt->csum for every opsize, including the
22-byte MP_CAPABLE + data form this patch is about.

The struct is an uninitialised stack object in mptcp_incoming_options()
and in net/mptcp/subflow.c, and mptcp_get_options() only clears the
4-byte status group:

	*(u32 *)&mp_opt->status = 0;

while protocol.h declares csum outside of that group:

	u16	data_len;
	__sum16	csum;
	struct_group(status,

So when that dynamic-debug callsite is enabled, 2 bytes of stale stack
are formatted into dmesg on a remotely reachable path.  The same
pr_debug() also prints sndr_key/rcvr_key/data_len for short opsizes,
so the debug-print read predates this commit; it is mentioned here
because the commit message frames the fix as removing the uninitialised
use of csum, and a stable backporter may read it that way.

Would adding mp_opt->csum = 0 (or extending the region cleared in
mptcp_get_options()) address this at the root, rather than relying on
every consumer testing OPTION_MPTCP_CSUMREQD?

[Severity: Low]
This isn't a bug introduced by this patch, but a question about the
passive side losing the peer's checksum requirement in one corner case.

subflow_init_req() derives the requirement from the local sysctl only:

	subflow_req->csum_reqd = mptcp_is_checksum_enabled(sock_net(sk_listener));

and subflow_check_req() does not preserve OPTION_MPTCP_CSUMREQD from the
SYN, so mptcp_sk_clone_init() is the only place where the passive side
picks up the peer's flag:

	if (mp_opt->suboptions & OPTION_MPTCP_CSUMREQD)
		WRITE_ONCE(msk->csum_enabled, true);

With the new opsize test, a 22-byte MP_CAPABLE + data with the A bit set
no longer sets that suboption, so msk->csum_enabled stays false and
get_mapping_status() accepts the mapping without validation:

	if (unlikely(subflow->map_csum_reqd != csum_reqd))
		return MAPPING_INVALID;

For a Linux sender the two signals cannot diverge, since
mptcp_established_options_mp() and mptcp_write_options() both derive the
A flag and the checksum field from opts->csum_reqd, and RFC 8684 omits
the checksum field only when checksums are not in use, so only a
self-contradictory peer reaches this path.  The commit message already
covers that trade-off.  The gap where the peer's SYN A bit is never
stored in subflow_req is pre-existing and unchanged here.

Would it be worth a short note in the commit message that the passive
side's only source for the peer's A bit is the third ACK, so the new
condition also drops that signal for a data-bearing third ACK?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908-net-mptcp-misc-fixes-7-3-rc1-v2-0-df1de70348b6%40kernel.org

Re: [PATCH net v2 05/15] mptcp: options: handle MPC data + csum reqd + no csum

[Matthieu Baerts]

Hello,

On 09/09/2026 16:49, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> 
> Critical: 0 · High: 0 · Medium: 0 · Low: 1 (1)
> 
> - [Low] Residual uninitialised read of `mp_opt->csum`: the patch closes
>   the functional consumer of the uninitialised checksum…

There is a pr_debug() reading mp_opt->csum. Harmless, but should be
fixed to silence such report. A follow-up patch will follow:

https://lore.kernel.org/20260908-mptcp-opt-read-debug-v1-2-b3e74549b6f6@kernel.org

> Pre-existing issues:
> - [Low] Reported concern (assessed as an intentional, documented
>   behaviour change): a passive endpoint whose local checksum sysctl is…

(...)

> The commit message already
> covers that trade-off.  The gap where the peer's SYN A bit is never
> stored in subflow_req is pre-existing and unchanged here.

Indeed ...

> Would it be worth a short note in the commit message that the passive
> side's only source for the peer's A bit is the third ACK, so the new
> condition also drops that signal for a data-bearing third ACK?

... but there is already a note about that. Maybe the note is not clear
enough, or my English is not good enough, but I don't think we need to
add an even longer text.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

[PATCH net v2 06/15] mptcp: prevent race between disconnect() and rtx

[Matthieu Baerts (NGI0)]

From: Paolo Abeni <pabeni@redhat.com>

Sashiko noted that the two event can race, leading to inconsistent
status. Prevent the race using the synchronous timer stop operation.

Fixes: b29fcfb54cd7 ("mptcp: full disconnect implementation")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/protocol.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index be59651e708e..d611af2eb74f 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3588,6 +3588,7 @@ static void mptcp_destroy_common(struct mptcp_sock *msk)
 
 static int mptcp_disconnect(struct sock *sk, int flags)
 {
+	struct inet_connection_sock *icsk = inet_csk(sk);
 	struct mptcp_sock *msk = mptcp_sk(sk);
 
 	/* We are on the fastopen error path. We can't call straight into the
@@ -3600,8 +3601,13 @@ static int mptcp_disconnect(struct sock *sk, int flags)
 	mptcp_check_listen_stop(sk);
 	mptcp_set_state(sk, TCP_CLOSE);
 
-	mptcp_stop_rtx_timer(sk);
-	mptcp_stop_tout_timer(sk);
+	/* The later subflow close can not kick again the tout timer,
+	 * as the msk is already in closed status.
+	 */
+	msk->timer_ival = icsk->icsk_rto_min;
+	sk_stop_timer_sync(sk, &sk->mptcp_retransmit_timer);
+	icsk->icsk_mtup.probe_timestamp = 0;
+	sk_stop_timer_sync(sk, &icsk->mptcp_tout_timer);
 
 	mptcp_pm_connection_closed(msk);
 

-- 
2.55.0

Re: [PATCH net v2 06/15] mptcp: prevent race between disconnect() and rtx

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 2 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 0 · Low: 2

- [Low] The commit message of "mptcp: prevent race between disconnect()
  and rtx" describes the change as only "Prevent the race using the…
- [Low] The commit carries "Fixes: b29fcfb54cd7 (\"mptcp: full disconnect
  implementation\")" (v5.18) for a real race that can leave a reused msk…

--

mptcp: prevent race between disconnect() and rtx

This replaces the mptcp_stop_rtx_timer() and mptcp_stop_tout_timer() calls in
mptcp_disconnect() with open-coded sk_stop_timer_sync() calls, so a
concurrently running retransmit timer cannot resurrect state after the
disconnect path has reset it.  No Link: or Closes: tags are present in the
commit header.

A couple of questions below, one about the changelog and one about the tags.

>     mptcp: prevent race between disconnect() and rtx
>     
>     Sashiko noted that the two event can race, leading to inconsistent
>     status. Prevent the race using the synchronous timer stop operation.
>     
>     Fixes: b29fcfb54cd7 ("mptcp: full disconnect implementation")
>     Signed-off-by: Paolo Abeni <pabeni@redhat.com>

[Severity: Low]
The Fixes: tag points at b29fcfb54cd7, which went in during v5.18, and the
race described can leave a reused msk with MPTCP_RETRANSMIT / MPTCP_WORK_RTX
set after mptcp_disconnect() has already cleared msk->flags and msk->cb_flags.

Should this also carry Cc: stable@vger.kernel.org?  Most of the other fixes in
this series do, including "mptcp: do not reschedule the RTX timer for fallback
sockets", whose MPTCP_RTX_ENABLED bit this change relies on so the timer cannot
be re-armed once the socket is in TCP_CLOSE.

Also, the report is credited to "Sashiko" in prose only.  Would it be possible
to add the matching Closes: or Link: tag, the way the sibling commits in the
series do?

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index be59651e708e..d611af2eb74f 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -3588,6 +3588,7 @@ static void mptcp_destroy_common(struct mptcp_sock *msk)
>  
>  static int mptcp_disconnect(struct sock *sk, int flags)
>  {
> +	struct inet_connection_sock *icsk = inet_csk(sk);
>  	struct mptcp_sock *msk = mptcp_sk(sk);
>  
>  	/* We are on the fastopen error path. We can't call straight into the
> @@ -3600,8 +3601,13 @@ static int mptcp_disconnect(struct sock *sk, int flags)
>  	mptcp_check_listen_stop(sk);
>  	mptcp_set_state(sk, TCP_CLOSE);
>  
> -	mptcp_stop_rtx_timer(sk);
> -	mptcp_stop_tout_timer(sk);
> +	/* The later subflow close can not kick again the tout timer,
> +	 * as the msk is already in closed status.
> +	 */
> +	msk->timer_ival = icsk->icsk_rto_min;
> +	sk_stop_timer_sync(sk, &sk->mptcp_retransmit_timer);
> +	icsk->icsk_mtup.probe_timestamp = 0;
> +	sk_stop_timer_sync(sk, &icsk->mptcp_tout_timer);

[Severity: Low]
The changelog says the race is prevented "using the synchronous timer stop
operation", which reads like a straight sk_stop_timer() to sk_stop_timer_sync()
swap, but the open-coded version in mptcp_disconnect() also changes two other
pieces of state.  Could the message mention them?

First, msk->timer_ival now ends up as icsk->icsk_rto_min, while the helper it
replaces sets it to zero:

net/mptcp/protocol.c:mptcp_stop_rtx_timer() {
	sk_stop_timer(sk, &sk->mptcp_retransmit_timer);
	mptcp_sk(sk)->timer_ival = 0;
}

The new value is what __mptcp_init_sock() seeds, so it looks intentional and
harmless, and the only reader, mptcp_reset_rtx_timer(), is gated on
MPTCP_RTX_ENABLED which mptcp_set_state(TCP_CLOSE) has just cleared.  Is that
the reasoning?

Second, the tout timer is now stopped and probe_timestamp cleared
unconditionally, whereas the replaced helper bailed out early when
probe_timestamp was already zero:

net/mptcp/protocol.h:mptcp_stop_tout_timer() {
	if (!inet_csk(sk)->icsk_mtup.probe_timestamp)
		return;

	sk_stop_timer(sk, &inet_csk(sk)->mptcp_tout_timer);
	inet_csk(sk)->icsk_mtup.probe_timestamp = 0;
}

That early return means a tout timer armed only for an MP_FAIL timeout, where
mptcp_reset_tout_timer() left probe_timestamp at zero, stayed pending across
disconnect().  Is that a separate fix folded in here, and would it be worth
calling out in the changelog so stable backporters can see it?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908-net-mptcp-misc-fixes-7-3-rc1-v2-0-df1de70348b6%40kernel.org

Re: [PATCH net v2 06/15] mptcp: prevent race between disconnect() and rtx

[Paolo Abeni]

On 9/9/26 4:49 PM, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider:
> 
> Critical: 0 · High: 0 · Medium: 0 · Low: 2
> 
> - [Low] The commit message of "mptcp: prevent race between disconnect()
>   and rtx" describes the change as only "Prevent the race using the…
> - [Low] The commit carries "Fixes: b29fcfb54cd7 (\"mptcp: full disconnect
>   implementation\")" (v5.18) for a real race that can leave a reused msk…

Here the code is actually just fine and sashiko don't like much my prose.

/P

Re: [PATCH net v2 06/15] mptcp: prevent race between disconnect() and rtx

[Matthieu Baerts]

Hello,

On 08/09/2026 16:07, Matthieu Baerts (NGI0) wrote:
> From: Paolo Abeni <pabeni@redhat.com>
> 
> Sashiko noted that the two event can race, leading to inconsistent
> status. Prevent the race using the synchronous timer stop operation.
> 
> Fixes: b29fcfb54cd7 ("mptcp: full disconnect implementation")

It looks like I forgot to add, sorry about that:

Cc: stable@vger.kernel.org

Done :)

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

[PATCH net v2 07/15] selftests: mptcp: fix an UAF in mptcp_connect.c

[Matthieu Baerts (NGI0)]

From: Gang Yan <yangang@kylinos.cn>

At the end of 'sock_connect_mptcp()', it calls 'freeaddrinfo(addr)',
the 'peer' pointer (which points into 'addr') remains. Later, the main
loop uses this peer pointer for reconnection attempts. If the memory has
been freed and reused, the address data could be overwritten, resulting
in an invalid remote address.

This patch keeps the addrinfo list allocated for the whole process
lifetime so "peer" remains valid across reconnects; the memory will be
released at exit() time.

Fixes: 05be5e273c84 ("selftests: mptcp: add disconnect tests")
Cc: stable@vger.kernel.org
Suggested-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Gang Yan <yangang@kylinos.cn>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Cc: Shuah Khan <shuah@kernel.org>
Cc: linux-kselftest@vger.kernel.org
---
 tools/testing/selftests/net/mptcp/mptcp_connect.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_connect.c b/tools/testing/selftests/net/mptcp/mptcp_connect.c
index ea4cb6c1bd5e..178d98d91fea 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_connect.c
+++ b/tools/testing/selftests/net/mptcp/mptcp_connect.c
@@ -381,6 +381,9 @@ static int sock_connect_mptcp(const char * const remoteaddr,
 
 	hints.ai_family = pf;
 
+	/* Keep the resolved address alive for the whole execution: it is
+	 * used again when reconnecting, and will be released at exit time.
+	 */
 	xgetaddrinfo(remoteaddr, port, &hints, &addr);
 	for (a = addr; a; a = a->ai_next) {
 		sock = socket(a->ai_family, a->ai_socktype, proto);
@@ -421,7 +424,6 @@ static int sock_connect_mptcp(const char * const remoteaddr,
 		sock = -1;
 	}
 
-	freeaddrinfo(addr);
 	if (sock != -1)
 		SOCK_TEST_TCPULP(sock, proto);
 	return sock;

-- 
2.55.0

[PATCH net v2 08/15] mptcp: pm: userspace: fix address ID overflow

[Matthieu Baerts (NGI0)]

From: Qing Luo <luoqing@kylinos.cn>

When all MPTCP address IDs (1-255) are exhausted in the userspace PM,
find_next_zero_bit() returns MPTCP_PM_MAX_ADDR_ID + 1 (256). This value
overflows when stored in the u8 field e->addr.id, resulting in ID 0
being stored and the entry being incorrectly added to the list.

ID 0 is reserved for the initial connection in MPTCP, so this overflow
can cause address conflicts.

Note: the in-kernel PM already has an 'endpoints == MPTCP_PM_MAX_ADDR_ID'
check in mptcp_pm_nl_append_new_local_addr() that returns -ERANGE before
reaching find_next_zero_bit(), preventing this overflow. So this fix only
addresses the userspace PM path.

Check the find_next_zero_bit() result against MPTCP_PM_MAX_ADDR_ID and
return -ENOSPC if all IDs are truly exhausted. Move the ID allocation
check before the memory allocation so that the error path does not need
to free the allocated entry.

Fixes: 4638de5aefe5 ("mptcp: handle local addrs announced by userspace PMs")
Cc: stable@vger.kernel.org
Assisted-by: LLM:0
Signed-off-by: Qing Luo <luoqing@kylinos.cn>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Cc: Kishen Maloor <kishen.maloor@intel.com>
---
 net/mptcp/pm_userspace.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index b94fbb483bf9..fab16d953dbf 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -69,6 +69,19 @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
 	}
 
 	if (!addr_match && !id_match) {
+		unsigned int id;
+
+		if (!entry->addr.id && needs_id) {
+			id = find_next_zero_bit(id_bitmap,
+						MPTCP_PM_MAX_ADDR_ID + 1, 1);
+			if (id > MPTCP_PM_MAX_ADDR_ID) {
+				ret = -ENOSPC;
+				goto append_err;
+			}
+		} else {
+			id = entry->addr.id;
+		}
+
 		/* Memory for the entry is allocated from the
 		 * sock option buffer.
 		 */
@@ -78,10 +91,7 @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
 			goto append_err;
 		}
 
-		if (!e->addr.id && needs_id)
-			e->addr.id = find_next_zero_bit(id_bitmap,
-							MPTCP_PM_MAX_ADDR_ID + 1,
-							1);
+		e->addr.id = id;
 		list_add_tail_rcu(&e->list, &msk->pm.userspace_pm_local_addr_list);
 		msk->pm.local_addr_used++;
 		ret = e->addr.id;

-- 
2.55.0

[PATCH net v2 09/15] mptcp: pm: reset retrans_time when ADD_ADDR entry is reused

[Matthieu Baerts (NGI0)]

When an ADD_ADDR entry is reused, the timer is re-armed, because the
goal is to re-announce an ADD_ADDR, and eventually retransmit it if
needed.

In this case, the retransmission counter should be reset as well, so the
re-announced address gets its retransmissions back instead of relying on
what was left before, and possibly not being able to retransmit it.

Fixes: 304ab97f4c7c ("mptcp: allow ADD_ADDR reissuance by userspace PMs")
Cc: stable@vger.kernel.org
Link: https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260803-net-mptcp-misc-fixes-7-2-rc6-v2-0-b8f496d71664%40kernel.org?part=4
Reviewed-by: Mat Martineau <martineau@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Cc: Kishen Maloor <kishen.maloor@intel.com>
---
 net/mptcp/pm.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
index 8b68868255c5..b0b71adefb8f 100644
--- a/net/mptcp/pm.c
+++ b/net/mptcp/pm.c
@@ -462,10 +462,10 @@ bool mptcp_pm_announced_alloc(struct mptcp_sock *msk,
 
 	add_entry->addr = *addr;
 	add_entry->sock = msk;
-	add_entry->retrans_times = 0;
 
 	timer_setup(&add_entry->timer, mptcp_pm_add_addr_timer, 0);
 reset_timer:
+	add_entry->retrans_times = 0;
 	add_entry->timer_done = false;
 	timeout = mptcp_adjust_add_addr_timeout(msk);
 	if (timeout)

-- 
2.55.0

[PATCH net v2 10/15] mptcp: remove unneeded READ_ONCE() annotation

[Matthieu Baerts (NGI0)]

From: Paolo Abeni <pabeni@redhat.com>

The subflow->fully_established flag is always written under the subflow
socket lock. Reading such value under the same lock does not require any
ONCE annotation.

Fixes: 581c8cbfa934 ("mptcp: annotate data-races around subflow->fully_established")
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Cc: Gang Yan <yangang@kylinos.cn>
v2: no need to cc stable.
---
 net/mptcp/options.c  | 4 ++--
 net/mptcp/protocol.c | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 92f27b9e087a..196a46e7467d 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -530,7 +530,7 @@ static bool mptcp_established_options_mp(struct sock *sk, struct sk_buff *skb,
 		return false;
 
 	/* MPC/MPJ needed only on 3rd ack packet, DATA_FIN and TCP shutdown take precedence */
-	if (READ_ONCE(subflow->fully_established) || snd_data_fin_enable ||
+	if (subflow->fully_established || snd_data_fin_enable ||
 	    subflow->snd_isn != TCP_SKB_CB(skb)->seq ||
 	    sk->sk_state != TCP_ESTABLISHED)
 		return false;
@@ -981,7 +981,7 @@ static bool check_fully_established(struct mptcp_sock *msk, struct sock *ssk,
 	/* here we can process OoO, in-window pkts, only in-sequence 4th ack
 	 * will make the subflow fully established
 	 */
-	if (likely(READ_ONCE(subflow->fully_established))) {
+	if (likely(subflow->fully_established)) {
 		/* on passive sockets, check for 3rd ack retransmission
 		 * note that msk is always set by subflow_syn_recv_sock()
 		 * for mp_join subflows
diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index d611af2eb74f..302936ff456a 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -3886,7 +3886,7 @@ static void schedule_3rdack_retransmission(struct sock *ssk)
 	struct tcp_sock *tp = tcp_sk(ssk);
 	unsigned long timeout;
 
-	if (READ_ONCE(mptcp_subflow_ctx(ssk)->fully_established))
+	if (mptcp_subflow_ctx(ssk)->fully_established)
 		return;
 
 	/* reschedule with a timeout above RTT, as we must look only for drop */

-- 
2.55.0

[PATCH net v2 11/15] selftests: mptcp: lib: dump nstat for the right test

[Matthieu Baerts (NGI0)]

In case of errors, mptcp_lib_pr_nstat is called to dump the nstat
counters, but for some tests, it was dumping the counters for all
subtests, not just the current one.

That's an issue for tests that don't recreate the netns for each
subtest, e.g. mptcp_connect.sh. In this case, 'nstat -a' will look at
the absolute counters since the creation of the netns, making
debugging harder.

Instead, it should dump the counters for the current test, by using the
history recorded in /tmp/<ns>.nstat if available, and not using '-a'
which was dumping the absolute values instead of calculating increments.

While at it, rename the previous 'hist' variable to 'cache' as it was
used to look at the cache, not the nstat history.

Fixes: 658e53141780 ("selftests: mptcp: join: dump stats from history")
Cc: stable@vger.kernel.org
Reviewed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Cc: Shuah Khan <shuah@kernel.org>
Cc: linux-kselftest@vger.kernel.org
---
 tools/testing/selftests/net/mptcp/mptcp_lib.sh | 10 ++++++----
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index 5ef6033775c8..da1da414c30f 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -108,12 +108,14 @@ mptcp_lib_pr_info() {
 
 mptcp_lib_pr_nstat() {
 	local ns="${1}"
-	local hist="/tmp/${ns}.out"
+	local cache="/tmp/${ns}.out"
+	local hist="/tmp/${ns}.nstat"
 
-	if [ -f "${hist}" ]; then
-		awk '$2 != 0 { print "  "$0 }' "${hist}"
+	if [ -f "${cache}" ]; then
+		awk '$2 != 0 { print "  "$0 }' "${cache}"
 	else
-		ip netns exec "${ns}" nstat -as | grep Tcp
+		NSTAT_HISTORY="${hist}" ip netns exec "${ns}" nstat -s |
+			grep Tcp
 	fi
 }
 

-- 
2.55.0

[PATCH net v2 12/15] selftests: mptcp: lib: get counters for the right test

[Matthieu Baerts (NGI0)]

When the value for a MIB counter is required, mptcp_lib_get_counter is
called. It tries to use the cache, if available. If not it falls back to
calling 'nstat' directly by looking at the absolute counters.

That's an issue for tests that don't recreate the netns for each
subtest. In this case, 'nstat -a' will look at the counters for the
netns.

Instead, it should look at the increment for the current test, by using
the history recorded in /tmp/<ns>.nstat, if available, and not using
'-a' which was dumping the absolute values.

While at it, rename the previous 'hist' variable to 'cache' as it was
used to look at the cache, not the nstat history.

Fixes: 71388a9f331d ("selftests: mptcp: lib: get counters from nstat history")
Cc: stable@vger.kernel.org
Reviewed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Cc: Shuah Khan <shuah@kernel.org>
Cc: linux-kselftest@vger.kernel.org
---
 tools/testing/selftests/net/mptcp/mptcp_lib.sh | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/tools/testing/selftests/net/mptcp/mptcp_lib.sh b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
index da1da414c30f..b9d14647f401 100644
--- a/tools/testing/selftests/net/mptcp/mptcp_lib.sh
+++ b/tools/testing/selftests/net/mptcp/mptcp_lib.sh
@@ -416,19 +416,21 @@ mptcp_lib_nstat_get() {
 }
 
 # $1: ns, $2: MIB counter
-# Get the counter from the history (mptcp_lib_nstat_{init,get}()) if available.
-# If not, get the counter from nstat ignoring any history.
+# Get the counter from the cache (mptcp_lib_nstat_{init,get}()) if available.
+# If not, get the counter from nstat ignoring any cache, but using the history.
 mptcp_lib_get_counter() {
 	local ns="${1}"
 	local counter="${2}"
-	local hist="/tmp/${ns}.out"
+	local cache="/tmp/${ns}.out"
+	local hist="/tmp/${ns}.nstat"
 	local count
 
-	if [[ -s "${hist}" && "${counter}" == *"Tcp"* ]]; then
-		count=$(awk "/^${counter} / {print \$2; exit}" "${hist}")
+	if [[ -s "${cache}" && "${counter}" == *"Tcp"* ]]; then
+		count=$(awk "/^${counter} / {print \$2; exit}" "${cache}")
 	else
-		count=$(ip netns exec "${ns}" nstat -asz "${counter}" |
-			awk 'NR==1 {next} {print $2}')
+		count=$(NSTAT_HISTORY="${hist}" ip netns exec "${ns}" \
+			nstat -sz "${counter}" |
+				awk 'NR==1 {next} {print $2}')
 	fi
 	if [ -z "${count}" ]; then
 		mptcp_lib_fail_if_expected_feature "${counter} counter"

-- 
2.55.0

[PATCH net v2 13/15] mptcp: options: fix uninit-value in mptcp_write_data_fin

[Matthieu Baerts (NGI0)]

When sending a DATA_FIN without data, and because the DATA_FIN occupies
1 octet of the connection-level sequence space [1], it is then required
to add a DSS mapping with specific values.

If the checksum has been negotiated, it also needs to be computed, and
included in the outgoing packet, and thus the initial csum data needs to
be reset to 0 as well. This is no longer the case since commit
cfcceb7a39fc ("tcp: shrink per-packet memset in __tcp_transmit_skb()"),
because the whole ext_copy structure is no longer zeroed by default.

This seems to be the only case where use_map is changed and set
afterwards, so initialising the csum field only in this case, along with
other fields for this specific case.

Fixes: cfcceb7a39fc ("tcp: shrink per-packet memset in __tcp_transmit_skb()")
Cc: stable@vger.kernel.org
Link: https://datatracker.ietf.org/doc/html/rfc8684#section-3.3.3 [1]
Link: https://sashiko.dev/#/patchset/20260812-net-next-mptcp-misc-feat-7-3-v1-0-1905a818f6cb%40kernel.org?part=2
Reviewed-by: Geliang Tang <geliang@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Cc: Kuniyuki Iwashima <kuniyu@google.com>
Cc: Keita Morisaki <kmta1236@gmail.com>
Cc: Jakub Sitnicki <jakub@cloudflare.com>
---
 net/mptcp/options.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/net/mptcp/options.c b/net/mptcp/options.c
index 196a46e7467d..ce0de02f5a3a 100644
--- a/net/mptcp/options.c
+++ b/net/mptcp/options.c
@@ -612,6 +612,7 @@ static void mptcp_write_data_fin(struct mptcp_subflow_context *subflow,
 		ext->data_seq = data_fin_tx_seq;
 		ext->subflow_seq = 0;
 		ext->data_len = 1;
+		ext->csum = 0;
 	} else if (ext->data_seq + ext->data_len == data_fin_tx_seq) {
 		/* If there's an existing DSS mapping and it is the
 		 * final mapping, DATA_FIN consumes 1 additional byte of

-- 
2.55.0

[PATCH net v2 14/15] mptcp: being below memory limit is a likely() condition

[Matthieu Baerts (NGI0)]

From: Paolo Abeni <pabeni@redhat.com>

The current compiler hint annotation is wrong, due to inverted
logic in the previous revision of the relevant code.

Fixes: e468d371180d ("mptcp: implemented OoO queue pruning")
Cc: stable@vger.kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/protocol.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 302936ff456a..4309fca6b119 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -289,8 +289,8 @@ static void mptcp_prune_ofo_queue(struct sock *sk,
  */
 static bool mptcp_can_ingest(const struct sock *sk)
 {
-	return unlikely(sk_rmem_alloc_get(sk) <= READ_ONCE(sk->sk_rcvbuf)) ||
-			__mptcp_check_fallback(mptcp_sk(sk));
+	return likely(sk_rmem_alloc_get(sk) <= READ_ONCE(sk->sk_rcvbuf)) ||
+	       __mptcp_check_fallback(mptcp_sk(sk));
 }
 
 static bool mptcp_try_rmem_schedule(struct sock *sk, const struct sk_buff *skb)

-- 
2.55.0

[PATCH net v2 15/15] mptcp: avoid pruning for OoW data

[Matthieu Baerts (NGI0)]

From: Paolo Abeni <pabeni@redhat.com>

Pruning is expansive and destructive, do it only when we expect
to accept the skb triggering the cleanup.

Fixes: e468d371180d ("mptcp: implemented OoO queue pruning")
Cc: stable@vger.kernel.org
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Reviewed-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
 net/mptcp/protocol.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
index 4309fca6b119..0098e2830931 100644
--- a/net/mptcp/protocol.c
+++ b/net/mptcp/protocol.c
@@ -313,12 +313,6 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
 	u64 seq, end_seq, max_seq;
 	struct sk_buff *skb1;
 
-	if (!mptcp_try_rmem_schedule(sk, skb)) {
-		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
-		mptcp_drop(sk, skb);
-		return;
-	}
-
 	seq = MPTCP_SKB_CB(skb)->map_seq;
 	end_seq = MPTCP_SKB_CB(skb)->end_seq;
 	max_seq = atomic64_read(&msk->rcv_wnd_sent);
@@ -335,6 +329,12 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
 		return;
 	}
 
+	if (!mptcp_try_rmem_schedule(sk, skb)) {
+		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
+		mptcp_drop(sk, skb);
+		return;
+	}
+
 	p = &msk->out_of_order_queue.rb_node;
 	MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_OFOQUEUE);
 	if (RB_EMPTY_ROOT(&msk->out_of_order_queue)) {

-- 
2.55.0

Re: [PATCH net v2 15/15] mptcp: avoid pruning for OoW data

[netdev-bot+sashiko]

Thank you for your contribution! Sashiko AI review found 1 potential
issue(s) to consider:

Critical: 0 · High: 0 · Medium: 1 · Low: 0

- [Medium] Incomplete realization of the patch's stated principle ("do it
  only when we expect to accept the skb triggering the cleanup"): after…

--

mptcp: avoid pruning for OoW data

This moves the mptcp_try_rmem_schedule() call in mptcp_data_queue_ofo()
so that it runs after the out-of-window check instead of before it, which
avoids running the destructive OoO queue pruning for skbs that are going
to be dropped as out of window.

> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
> index 4309fca6b119..0098e2830931 100644
> --- a/net/mptcp/protocol.c
> +++ b/net/mptcp/protocol.c
> @@ -313,12 +313,6 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
>  	u64 seq, end_seq, max_seq;
>  	struct sk_buff *skb1;
>  
> -	if (!mptcp_try_rmem_schedule(sk, skb)) {
> -		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
> -		mptcp_drop(sk, skb);
> -		return;
> -	}
> -
>  	seq = MPTCP_SKB_CB(skb)->map_seq;
>  	end_seq = MPTCP_SKB_CB(skb)->end_seq;
>  	max_seq = atomic64_read(&msk->rcv_wnd_sent);
> @@ -335,6 +329,12 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
>  		return;
>  	}
>  
> +	if (!mptcp_try_rmem_schedule(sk, skb)) {
> +		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
> +		mptcp_drop(sk, skb);
> +		return;
> +	}
> +

[Severity: Medium]
The commit message says pruning should be done "only when we expect
to accept the skb triggering the cleanup".  Does the new placement
fully match that, given the duplicate-data check still happens later in
the rbtree walk of mptcp_data_queue_ofo()?

Consider an OoO queue holding A (map_seq 1000, end_seq 2000), B (2000,
3000), C (3000, 4000) with ooo_last_skb == C, and an in-window skb
(map_seq 1200, end_seq 1500) arriving while sk_rmem_alloc exceeds
sk_rcvbuf.  mptcp_prune_ofo_queue() walks back from ooo_last_skb:

net/mptcp/protocol.c:mptcp_prune_ofo_queue() {
	...
		/* Stop pruning if the incoming skb would land in OoO tail. */
		if (after64(MPTCP_SKB_CB(in_skb)->map_seq,
			    MPTCP_SKB_CB(skb)->map_seq))
			break;

		pruned = true;
		prev = rb_prev(node);
		rb_erase(node, &msk->out_of_order_queue);
		mptcp_drop(sk, skb);
	...
}

after64(1200, 3000) and after64(1200, 2000) are both false, so C and
then B are erased and freed; the loop only breaks at A.

mptcp_can_ingest() then succeeds, and the tree walk reaches:

			if (!after64(end_seq, MPTCP_SKB_CB(skb1)->end_seq)) {
				/* All the bits are present. Drop. */
				mptcp_drop(sk, skb);
				MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_DUPDATA);
				return;
			}

so the incoming skb is discarded as duplicate after two already queued
segments were destroyed for it.  Since neither the rbtree walk nor
rb_link_node()/rb_insert_color() allocates memory, would it work to move
the mptcp_try_rmem_schedule() call below the duplicate and coalesce
classification instead of only below the out-of-window test?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20260908-net-mptcp-misc-fixes-7-3-rc1-v2-0-df1de70348b6%40kernel.org

Re: [PATCH net v2 15/15] mptcp: avoid pruning for OoW data

[Paolo Abeni]

On 9/9/26 4:49 PM, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 1 potential
> issue(s) to consider:
> 
> Critical: 0 · High: 0 · Medium: 1 · Low: 0
> 
> - [Medium] Incomplete realization of the patch's stated principle ("do it
>   only when we expect to accept the skb triggering the cleanup"): after…
> 
> --
> 
> mptcp: avoid pruning for OoW data
> 
> This moves the mptcp_try_rmem_schedule() call in mptcp_data_queue_ofo()
> so that it runs after the out-of-window check instead of before it, which
> avoids running the destructive OoO queue pruning for skbs that are going
> to be dropped as out of window.
> 
>> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
>> index 4309fca6b119..0098e2830931 100644
>> --- a/net/mptcp/protocol.c
>> +++ b/net/mptcp/protocol.c
>> @@ -313,12 +313,6 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
>>  	u64 seq, end_seq, max_seq;
>>  	struct sk_buff *skb1;
>>  
>> -	if (!mptcp_try_rmem_schedule(sk, skb)) {
>> -		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
>> -		mptcp_drop(sk, skb);
>> -		return;
>> -	}
>> -
>>  	seq = MPTCP_SKB_CB(skb)->map_seq;
>>  	end_seq = MPTCP_SKB_CB(skb)->end_seq;
>>  	max_seq = atomic64_read(&msk->rcv_wnd_sent);
>> @@ -335,6 +329,12 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
>>  		return;
>>  	}
>>  
>> +	if (!mptcp_try_rmem_schedule(sk, skb)) {
>> +		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
>> +		mptcp_drop(sk, skb);
>> +		return;
>> +	}
>> +
> 
> [Severity: Medium]
> The commit message says pruning should be done "only when we expect
> to accept the skb triggering the cleanup".  Does the new placement
> fully match that, given the duplicate-data check still happens later in
> the rbtree walk of mptcp_data_queue_ofo()?

The change suggested here would add quite a bit of complexity for little
gain, and would diverge MPTCP behavior from TCP. We learned the lesson
the hard way we need very good reasons to do the latter. I'm pretty sure
we are better off with the proposed code.

/P

Re: [PATCH net v2 15/15] mptcp: avoid pruning for OoW data

[Matthieu Baerts]

Hi Paolo,

Thank you for having checked these comments.

On 09/09/2026 17:50, Paolo Abeni wrote:
> On 9/9/26 4:49 PM, netdev-bot+sashiko@kernel.org wrote:
>> Thank you for your contribution! Sashiko AI review found 1 potential
>> issue(s) to consider:
>>
>> Critical: 0 · High: 0 · Medium: 1 · Low: 0
>>
>> - [Medium] Incomplete realization of the patch's stated principle ("do it
>>   only when we expect to accept the skb triggering the cleanup"): after…
>>
>> --
>>
>> mptcp: avoid pruning for OoW data
>>
>> This moves the mptcp_try_rmem_schedule() call in mptcp_data_queue_ofo()
>> so that it runs after the out-of-window check instead of before it, which
>> avoids running the destructive OoO queue pruning for skbs that are going
>> to be dropped as out of window.
>>
>>> diff --git a/net/mptcp/protocol.c b/net/mptcp/protocol.c
>>> index 4309fca6b119..0098e2830931 100644
>>> --- a/net/mptcp/protocol.c
>>> +++ b/net/mptcp/protocol.c
>>> @@ -313,12 +313,6 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
>>>  	u64 seq, end_seq, max_seq;
>>>  	struct sk_buff *skb1;
>>>  
>>> -	if (!mptcp_try_rmem_schedule(sk, skb)) {
>>> -		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
>>> -		mptcp_drop(sk, skb);
>>> -		return;
>>> -	}
>>> -
>>>  	seq = MPTCP_SKB_CB(skb)->map_seq;
>>>  	end_seq = MPTCP_SKB_CB(skb)->end_seq;
>>>  	max_seq = atomic64_read(&msk->rcv_wnd_sent);
>>> @@ -335,6 +329,12 @@ static void mptcp_data_queue_ofo(struct mptcp_sock *msk, struct sk_buff *skb)
>>>  		return;
>>>  	}
>>>  
>>> +	if (!mptcp_try_rmem_schedule(sk, skb)) {
>>> +		MPTCP_INC_STATS(sock_net(sk), MPTCP_MIB_RCVPRUNED);
>>> +		mptcp_drop(sk, skb);
>>> +		return;
>>> +	}
>>> +
>>
>> [Severity: Medium]
>> The commit message says pruning should be done "only when we expect
>> to accept the skb triggering the cleanup".  Does the new placement
>> fully match that, given the duplicate-data check still happens later in
>> the rbtree walk of mptcp_data_queue_ofo()?
> 
> The change suggested here would add quite a bit of complexity for little
> gain, and would diverge MPTCP behavior from TCP. We learned the lesson
> the hard way we need very good reasons to do the latter. I'm pretty sure
> we are better off with the proposed code.

I agree with you.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH net v2 00/15] mptcp: misc fixes for v7.3-rc1

[Matthieu Baerts]

Hello,

On 08/09/2026 16:07, Matthieu Baerts (NGI0) wrote:
> Here are various unrelated fixes:

Regarding Clashiko's review: I don't think a new version is needed.
Follow-up patches for non-directly related issues will be sent later on.

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.

Re: [PATCH net v2 00/15] mptcp: misc fixes for v7.3-rc1

[patchwork-bot+netdevbpf]

Hello:

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

On Tue, 08 Sep 2026 16:07:05 +0200 you wrote:
> Here are various unrelated fixes:
> 
> - Patch 1: Do not reschedule the RTX timer for sockets that fell back to
>   TCP. A fix for v5.7.
> 
> - Patch 2: Avoid copying thmac which will not be used and could be
>   uninitialised. A fix for v5.7.
> 
> [...]

Here is the summary with links:
  - [net,v2,01/15] mptcp: do not reschedule the RTX timer for fallback sockets
    https://git.kernel.org/netdev/net/c/e2ab913f68c7
  - [net,v2,02/15] mptcp: subflow: no need to copy thmac during ulp_clone
    https://git.kernel.org/netdev/net/c/29f641951be0
  - [net,v2,03/15] mptcp: syncookies: remember the request backup flag
    https://git.kernel.org/netdev/net/c/b76c0e28b392
  - [net,v2,04/15] mptcp: pm: kernel: drop pending ADD_ADDR when removing ID0
    https://git.kernel.org/netdev/net/c/2ac7d6e62076
  - [net,v2,05/15] mptcp: options: handle MPC data + csum reqd + no csum
    https://git.kernel.org/netdev/net/c/ab36b1a80942
  - [net,v2,06/15] mptcp: prevent race between disconnect() and rtx
    https://git.kernel.org/netdev/net/c/85c580b0d859
  - [net,v2,07/15] selftests: mptcp: fix an UAF in mptcp_connect.c
    https://git.kernel.org/netdev/net/c/730444f094b1
  - [net,v2,08/15] mptcp: pm: userspace: fix address ID overflow
    https://git.kernel.org/netdev/net/c/f9f0068e8813
  - [net,v2,09/15] mptcp: pm: reset retrans_time when ADD_ADDR entry is reused
    https://git.kernel.org/netdev/net/c/f968190c0b42
  - [net,v2,10/15] mptcp: remove unneeded READ_ONCE() annotation
    https://git.kernel.org/netdev/net/c/caa4a79f74f3
  - [net,v2,11/15] selftests: mptcp: lib: dump nstat for the right test
    https://git.kernel.org/netdev/net/c/e1a56368eac1
  - [net,v2,12/15] selftests: mptcp: lib: get counters for the right test
    https://git.kernel.org/netdev/net/c/d23c41366e85
  - [net,v2,13/15] mptcp: options: fix uninit-value in mptcp_write_data_fin
    https://git.kernel.org/netdev/net/c/b110f1dd6cb6
  - [net,v2,14/15] mptcp: being below memory limit is a likely() condition
    https://git.kernel.org/netdev/net/c/a4257a91af7a
  - [net,v2,15/15] mptcp: avoid pruning for OoW data
    https://git.kernel.org/netdev/net/c/f01b8275745e

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