[PATCH net] mptcp: pm: in-kernel: fix C-flag endpoint accounting on family mismatch

Yiming Qian posted 1 patch 1 week, 2 days ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/multipath-tcp/mptcp_net-next tags/patchew/20261006114409.117190-1-yimingqian591@gmail.com
net/mptcp/pm_kernel.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
[PATCH net] mptcp: pm: in-kernel: fix C-flag endpoint accounting on family mismatch
Posted by Yiming Qian 1 week, 2 days ago
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
Re: [PATCH net] mptcp: pm: in-kernel: fix C-flag endpoint accounting on family mismatch
Posted by MPTCP CI 1 week, 2 days ago
Hi Yiming,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Perf: Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/37460664542

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/823f691bd2d3
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1180261


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)
Re: [PATCH net] mptcp: pm: in-kernel: fix C-flag endpoint accounting on family mismatch
Posted by Matthieu Baerts 1 week, 2 days ago
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.
Re: [PATCH net] mptcp: pm: in-kernel: fix C-flag endpoint accounting on family mismatch
Posted by Matthieu Baerts 3 days, 13 hours ago
Hi Yiming Qian,

On 06/10/2026 13:59, Matthieu Baerts wrote:
> 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.

Just to avoid a deadlock, do you plan to send a v2, or do you prefer if
someone else does that?

Cheers,
Matt
-- 
Sponsored by the NGI0 Core fund.
Re: [PATCH net] mptcp: pm: in-kernel: fix C-flag endpoint accounting on family mismatch
Posted by netdev-bot+sinfo@kernel.org 1 week, 2 days ago
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.