Netdev List
 help / color / mirror / Atom feed
* [PATCH net] mptcp: pm: in-kernel: fix C-flag endpoint accounting on family mismatch
@ 2026-10-06 11:42 Yiming Qian
  2026-10-06 11:49 ` netdev-bot+sinfo
  2026-10-06 11:59 ` Matthieu Baerts
  0 siblings, 2 replies; 3+ messages in thread
From: Yiming Qian @ 2026-10-06 11:42 UTC (permalink / raw)
  To: security, mptcp
  Cc: matttbe, martineau, geliang, netdev, stable, yimingqian591

fill_local_addresses_vec_c_flag() clears the endpoint ID bit in
id_avail_bitmap before checking whether the endpoint's address family
matches the remote ADD_ADDR.  If it does not match, the loop skips the
endpoint without incrementing local_addr_used, leaving id_avail_bitmap
and local_addr_used inconsistent.

Removing that SUBFLOW endpoint later calls
__mark_subflow_endp_available(), which sees the cleared bit, expects
local_addr_used to be non-zero, and triggers a WARN and, with
panic_on_warn=1, a panic.

Keep the endpoints that are skipped during this specific C-flag
iteration available by tracking them in a local bitmap passed to
select_local_address(), instead of clearing their IDs in the per-socket
bitmap.  Only IDs of endpoints actually used for a subflow are now
cleared and accounted for.

Fixes: 4b1ff850e0c1 ("mptcp: pm: in-kernel: usable client side with C-flag")
Cc: stable@vger.kernel.org
Signed-off-by: Yiming Qian <yimingqian591@gmail.com>
---
 net/mptcp/pm_kernel.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index 1a77508132354..a5952dd0ae125 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -120,6 +120,7 @@ static bool has_subflow_daddr(const struct mptcp_sock *msk,
 static bool
 select_local_address(const struct pm_nl_pernet *pernet,
 		     const struct mptcp_sock *msk,
+		     const unsigned long *skip,
 		     struct mptcp_pm_local *new_local)
 {
 	struct mptcp_pm_addr_entry *entry;
@@ -135,6 +136,9 @@ select_local_address(const struct pm_nl_pernet *pernet,
 		if (!test_bit(entry->addr.id, msk->pm.id_avail_bitmap))
 			continue;
 
+		if (skip && test_bit(entry->addr.id, skip))
+			continue;
+
 		new_local->addr = entry->addr;
 		new_local->flags = entry->flags;
 		new_local->ifindex = entry->ifindex;
@@ -401,7 +405,7 @@ static void mptcp_pm_create_subflow_or_signal_addr(struct mptcp_sock *msk)
 
 		if (signal_and_subflow)
 			signal_and_subflow = false;
-		else if (!select_local_address(pernet, msk, &local))
+		else if (!select_local_address(pernet, msk, NULL, &local))
 			break;
 
 		fullmesh = !!(local.flags & MPTCP_PM_ADDR_FLAG_FULLMESH);
@@ -574,22 +578,28 @@ fill_local_addresses_vec_c_flag(struct mptcp_sock *msk,
 	u8 endp_subflow_max = mptcp_pm_get_endp_subflow_max(msk);
 	struct sock *sk = (struct sock *)msk;
 	struct mptcp_pm_local *local;
+	DECLARE_BITMAP(skip, MPTCP_PM_MAX_ADDR_ID + 1);
 	int i = 0;
 
+	bitmap_zero(skip, MPTCP_PM_MAX_ADDR_ID + 1);
+
 	while (msk->pm.local_addr_used < endp_subflow_max) {
 		local = &locals[i];
 
-		if (!select_local_address(pernet, msk, local))
+		if (!select_local_address(pernet, msk, skip, local))
 			break;
 
-		__clear_bit(local->addr.id, msk->pm.id_avail_bitmap);
-
-		if (!mptcp_pm_addr_families_match(sk, &local->addr, remote))
+		if (!mptcp_pm_addr_families_match(sk, &local->addr, remote)) {
+			__set_bit(local->addr.id, skip);
 			continue;
+		}
 
-		if (local->addr.id == msk->mpc_endpoint_id)
+		if (local->addr.id == msk->mpc_endpoint_id) {
+			__set_bit(local->addr.id, skip);
 			continue;
+		}
 
+		__clear_bit(local->addr.id, msk->pm.id_avail_bitmap);
 		msk->pm.local_addr_used++;
 		msk->pm.extra_subflows++;
 		i++;
-- 
2.34.1












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

* Re: [PATCH net] mptcp: pm: in-kernel: fix C-flag endpoint accounting on family mismatch
  2026-10-06 11:42 [PATCH net] mptcp: pm: in-kernel: fix C-flag endpoint accounting on family mismatch Yiming Qian
@ 2026-10-06 11:49 ` netdev-bot+sinfo
  2026-10-06 11:59 ` Matthieu Baerts
  1 sibling, 0 replies; 3+ messages in thread
From: netdev-bot+sinfo @ 2026-10-06 11:49 UTC (permalink / raw)
  To: Yiming Qian; +Cc: security, mptcp, matttbe, martineau, geliang, netdev, stable

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

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

* Re: [PATCH net] mptcp: pm: in-kernel: fix C-flag endpoint accounting on family mismatch
  2026-10-06 11:42 [PATCH net] mptcp: pm: in-kernel: fix C-flag endpoint accounting on family mismatch Yiming Qian
  2026-10-06 11:49 ` netdev-bot+sinfo
@ 2026-10-06 11:59 ` Matthieu Baerts
  1 sibling, 0 replies; 3+ messages in thread
From: Matthieu Baerts @ 2026-10-06 11:59 UTC (permalink / raw)
  To: Yiming Qian, mptcp; +Cc: martineau, geliang, netdev, stable, Security Officers

Hi Yiming Qian,

On 06/10/2026 13:42, Yiming Qian wrote:
> fill_local_addresses_vec_c_flag() clears the endpoint ID bit in
> id_avail_bitmap before checking whether the endpoint's address family
> matches the remote ADD_ADDR.  If it does not match, the loop skips the
> endpoint without incrementing local_addr_used, leaving id_avail_bitmap
> and local_addr_used inconsistent.
> 
> Removing that SUBFLOW endpoint later calls
> __mark_subflow_endp_available(), which sees the cleared bit, expects
> local_addr_used to be non-zero, and triggers a WARN and, with
> panic_on_warn=1, a panic.
> 
> Keep the endpoints that are skipped during this specific C-flag
> iteration available by tracking them in a local bitmap passed to
> select_local_address(), instead of clearing their IDs in the per-socket
> bitmap.  Only IDs of endpoints actually used for a subflow are now
> cleared and accounted for.

Thank you for the patch.

Please do not Cc security@k.o on patches sent to public ML. Here the
issue is "just" a WARN produced in some conditions when the user has the
rights to delete MPTCP endpoints.

(Please remove it from future replies.)

> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
> index 1a77508132354..a5952dd0ae125 100644
> --- a/net/mptcp/pm_kernel.c
> +++ b/net/mptcp/pm_kernel.c

(...)

> @@ -574,22 +578,28 @@ fill_local_addresses_vec_c_flag(struct mptcp_sock *msk,
>  	u8 endp_subflow_max = mptcp_pm_get_endp_subflow_max(msk);
>  	struct sock *sk = (struct sock *)msk;
>  	struct mptcp_pm_local *local;
> +	DECLARE_BITMAP(skip, MPTCP_PM_MAX_ADDR_ID + 1);
>  	int i = 0;
>  
> +	bitmap_zero(skip, MPTCP_PM_MAX_ADDR_ID + 1);
> +
>  	while (msk->pm.local_addr_used < endp_subflow_max) {
>  		local = &locals[i];
>  
> -		if (!select_local_address(pernet, msk, local))
> +		if (!select_local_address(pernet, msk, skip, local))
>  			break;
>  
> -		__clear_bit(local->addr.id, msk->pm.id_avail_bitmap);
> -
> -		if (!mptcp_pm_addr_families_match(sk, &local->addr, remote))
> +		if (!mptcp_pm_addr_families_match(sk, &local->addr, remote)) {
> +			__set_bit(local->addr.id, skip);
>  			continue;
> +		}
>  
> -		if (local->addr.id == msk->mpc_endpoint_id)
> +		if (local->addr.id == msk->mpc_endpoint_id) {
> +			__set_bit(local->addr.id, skip);
>  			continue;
> +		}
>  
> +		__clear_bit(local->addr.id, msk->pm.id_avail_bitmap);

Why is it not enough to move __clear_bit() to here? Why do you need to
modify select_local_address(), etc.?

Also, can you also provide a regression test as well, please?
Adding a subtest in tools/testing/selftests/net/mptcp/mptcp_join.sh,
e.g. in deny_join_id0_tests(), adding a new subcase using
"addr_nr_ns2=-1 speed=slow run_tests (...)" to remove the subflow during
the connection.

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


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

end of thread, other threads:[~2026-10-06 11:59 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-06 11:42 [PATCH net] mptcp: pm: in-kernel: fix C-flag endpoint accounting on family mismatch Yiming Qian
2026-10-06 11:49 ` netdev-bot+sinfo
2026-10-06 11:59 ` Matthieu Baerts

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