[PATCH mptcp-net v2] mptcp: pm: kernel: c-flag: mark as used only when it is

[PATCH mptcp-net v2] mptcp: pm: kernel: c-flag: mark as used only when it is

[Matthieu Baerts (NGI0)]

The ID was marked as used before checking if it could actually be used
in all conditions: if the families match and the local endpoint is not
the one linked to the initial endpoint.

Marking endpoints as used while they are not can have an impact on the
local_addr_used counter when endpoints are removed -- CAP_NET_ADMIN is
required -- and active connections have such badly marked endpoints. In
this case, the local counter will be decremented while it shouldn't. If
the counter was already at 0, this warning will be emitted:

  WARNING: net/mptcp/pm_kernel.c:1080 at __mark_subflow_endp_available+0x46/0x50, CPU#0: a.out/149
  RIP: 0010:__mark_subflow_endp_available (net/mptcp/pm_kernel.c:1080 (discriminator 2) net/mptcp/pm_kernel.c:1076 (discriminator 2))
  Call Trace:
   mptcp_pm_nl_del_addr_doit (net/mptcp/pm_kernel.c:1112 net/mptcp/pm_kernel.c:1227)
   genl_family_rcv_msg_doit (net/netlink/genetlink.c:1114)

The fix is easy: increment local_addr_used similar to what is done in
mptcp_pm_create_subflow_or_signal_addr() for the non C-flag case.

Fixes: 4b1ff850e0c1 ("mptcp: pm: in-kernel: usable client side with C-flag")
Reported-by: Yiming Qian <yimingqian591@gmail.com>
Closes: https://lore.kernel.org/20261006114409.117190-1-yimingqian591@gmail.com
Signed-off-by: Matthieu Baerts (NGI0) <matttbe@kernel.org>
---
Changes in v2:
- Move the increment of local_addr_used before checking the family,
  similar to what is done for the non C-flag case.
- Link to v1: https://patch.msgid.link/20261022-mptcp-pm-cflag-clear-when-used-v1-1-516c09b4ee65@kernel.org
---
Note: this patch supersedes the patch mentioned above: the author
doesn't seem to be willing to send a v2. Because security@k.o was Cc'ed
on public ML, that's forcing us not to wait, even if this fix is just
about a counter that will be wrongly decremented when MPTCP endpoints
are removed (requiring CAP_NET_ADMIN). Put the original author as
"Reported-by" following the recommendations given to maintainers in such
cases.
---
 net/mptcp/pm_kernel.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
index d3014bf57bf3..fb611c4b3eb8 100644
--- a/net/mptcp/pm_kernel.c
+++ b/net/mptcp/pm_kernel.c
@@ -584,13 +584,15 @@ fill_local_addresses_vec_c_flag(struct mptcp_sock *msk,
 
 		__clear_bit(local->addr.id, msk->pm.id_avail_bitmap);
 
-		if (!mptcp_pm_addr_families_match(sk, &local->addr, remote))
-			continue;
-
 		if (local->addr.id == msk->mpc_endpoint_id)
 			continue;
 
+		/* Increment before checking families, similar to non C-flag */
 		msk->pm.local_addr_used++;
+
+		if (!mptcp_pm_addr_families_match(sk, &local->addr, remote))
+			continue;
+
 		msk->pm.extra_subflows++;
 		i++;
 

---
base-commit: 894c5749a6139dffd3687f165fb7119f9e572ed3
change-id: 20261006-mptcp-pm-cflag-clear-when-used-ee817b26a553

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

Re: [PATCH mptcp-net v2] mptcp: pm: kernel: c-flag: mark as used only when it is

[MPTCP CI]

Hi Matthieu,

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/39768741688

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


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 mptcp-net v2] mptcp: pm: kernel: c-flag: mark as used only when it is

[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

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] The subject and first paragraph of the commit message describe
  a different fix from the one in the code.

--

mptcp: pm: kernel: c-flag: mark as used only when it is

This patch changes fill_local_addresses_vec_c_flag() so that
local_addr_used is incremented before the address family check. The
counter then stays consistent when endpoints are removed. Without this,
removing endpoints could decrement local_addr_used below zero and
trigger a warning in __mark_subflow_endp_available().

Closes: https://lore.kernel.org/20261006114409.117190-1-yimingqian591@gmail.com

> mptcp: pm: kernel: c-flag: mark as used only when it is
>
> The ID was marked as used before checking if it could actually be used
> in all conditions: if the families match and the local endpoint is not
> the one linked to the initial endpoint.

[Severity: Medium]
Do the subject and this first paragraph match what the diff does?

In fill_local_addresses_vec_c_flag(), the bitmap update still happens
before both checks, and the patch does not move it:

    __clear_bit(local->addr.id, msk->pm.id_avail_bitmap);

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

    msk->pm.local_addr_used++;

    if (!mptcp_pm_addr_families_match(sk, &local->addr, remote))
        continue;

So an endpoint whose family does not match the remote address, and which
never gets a subflow, is now counted as used in both id_avail_bitmap and
local_addr_used. That is the opposite of "mark as used only when it is".

The code itself looks right. select_local_address() returns the first
subflow endpoint that still has its avail bit set, so the loop has to
clear the bit to make progress. If it skipped mismatched endpoints
without clearing their bit, the loop could spin forever on the same
endpoint.

The actual fix is keeping local_addr_used in step with id_avail_bitmap,
which is what __mark_subflow_endp_available() relies on. Only the final
paragraph ("increment local_addr_used similar to ...") says that, plus
the v2 changelog below the "---", and that changelog is dropped when the
patch is applied.

Could the subject and the opening paragraph be reworded to describe that
instead, something like "c-flag: keep local_addr_used in sync with
id_avail_bitmap"?

> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
> index d3014bf57bf3c..fb611c4b3eb8d 100644
> --- a/net/mptcp/pm_kernel.c
> +++ b/net/mptcp/pm_kernel.c
> @@ -584,13 +584,15 @@ fill_local_addresses_vec_c_flag(struct mptcp_sock *msk,
>  
>  		__clear_bit(local->addr.id, msk->pm.id_avail_bitmap);
>  
> -		if (!mptcp_pm_addr_families_match(sk, &local->addr, remote))
> -			continue;
> -
>  		if (local->addr.id == msk->mpc_endpoint_id)
>  			continue;
>  
> +		/* Increment before checking families, similar to non C-flag */
>  		msk->pm.local_addr_used++;
> +
> +		if (!mptcp_pm_addr_families_match(sk, &local->addr, remote))
> +			continue;
> +
>  		msk->pm.extra_subflows++;
>  		i++;
>  

[ ... ]

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261022-mptcp-pm-cflag-clear-when-used-v2-1-8c7fbe166b74%40kernel.org