Re: [PATCH mptcp-next] mptcp: fix add_addr_accepted accounting on subflow close

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

Pre-existing issues:
- [High] Concurrent overwrite of msk->pm.rm_list_rx while iterating over it in mptcp_pm_rm_addr_or_subflow.
--

--- Patch [1]: [PATCH mptcp-next] mptcp: fix add_addr_accepted accounting on subflow close ---
commit 66a438e0ebd8ec5a7b6eb2a506d61f6a2a2d2cc5
Author: Akshit Patadiya <[email protected]>

mptcp: fix add_addr_accepted accounting on subflow close

This commit modifies how MPTCP tracks accepted remote addresses by using a
bitmap instead of relying on a live subflow to signal removal. It releases
accepted-address slots as soon as the subflow is closed.

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

> diff --git a/net/mptcp/pm.c b/net/mptcp/pm.c
> index ba7c6f80a183c..b6edb9df3216c 100644
> --- a/net/mptcp/pm.c
> +++ b/net/mptcp/pm.c

[ ... ]

> @@ -807,7 +808,6 @@ static void mptcp_pm_rm_addr_or_subflow(struct mptcp_sock *msk,
>  				 i, rm_id, id, remote_id, msk->mpc_endpoint_id);
>  			spin_unlock_bh(&msk->pm.lock);
>  			mptcp_subflow_shutdown(sk, ssk, how);
> -			removed |= subflow->request_join;
>  
>  			/* the following takes care of updating the subflows counter */
>  			mptcp_close_ssk(sk, ssk, subflow);

[Severity: High]
This isn't a bug introduced by this patch, but could this unlock/lock sequence
lead to a race condition with the outer loop?

In mptcp_pm_rm_addr_or_subflow(), the outer loop iterates over rm_list:

	for (i = 0; i < rm_list->nr; i++) {
		u8 rm_id = rm_list->ids[i];

If rm_list points to &msk->pm.rm_list_rx, and the PM lock is dropped here
in the inner loop, could a concurrent softirq processing a new RM_ADDR
overwrite the list while the lock is temporarily released?

This might corrupt the ongoing iteration of the outer loop once the lock is
reacquired and the next item in rm_list->nr and rm_list->ids is evaluated.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1