Re: [PATCH net-next 3/8] mptcp: pm: kernel: allow flushing more than 8 endpoints

[Matthieu Baerts <matttbe@kernel.org>]

Hello,

On 08/05/2026 17:40, Matthieu Baerts (NGI0) wrote:
> The mptcp_rm_list structure contains an array of IDs of 8 entries: to be
> able to send a RM_ADDR with 8 IDs. This limitation was OK so far because
> there could maximum 8 endpoints.
> 
> But this is going to change in the next commit. To cope with that, if
> one of the arrays is full, the iteration stops, the lists are processed,
> then the iteration continues where it previously stopped.
> 
> Note that if there are many endpoints to remove, and multiple RM_ADDR to
> send, it might be more likely that some of these RM_ADDRs are dropped or
> lost. This is a known limitation: RM_ADDR are not retransmitted in
> MPTCPv1.

(...)

> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
> index aabd73d15c15..ea3a7ea82013 100644
> --- a/net/mptcp/pm_kernel.c
> +++ b/net/mptcp/pm_kernel.c
> @@ -1223,19 +1223,30 @@ int mptcp_pm_nl_del_addr_doit(struct sk_buff *skb, struct genl_info *info)
>  }
>  
>  static void mptcp_pm_flush_addrs_and_subflows(struct mptcp_sock *msk,
> -					      struct list_head *rm_list)
> +					      struct list_head *rm_list,
> +					      struct mptcp_pm_addr_entry *entry)
>  {
> -	struct mptcp_rm_list alist = { .nr = 0 }, slist = { .nr = 0 };
> -	struct mptcp_pm_addr_entry *entry;
> +	struct mptcp_rm_list alist, slist;
> +	bool more;
>  
> -	list_for_each_entry(entry, rm_list, list) {
> -		if (slist.nr < MPTCP_RM_IDS_MAX &&
> -		    mptcp_lookup_subflow_by_saddr(&msk->conn_list, &entry->addr))
> +again:
> +	alist.nr = 0;
> +	slist.nr = 0;

FYI, Sashiko Gemini is saying:

> Are the ids arrays in alist and slist left uninitialized on the stack here?
> Later, in mptcp_pm_remove_addr(), a full struct assignment
> (msk->pm.rm_list_tx = *rm_list) copies the structure. Could this copy
> uninitialized stack memory into the persistent socket structure and
> trigger KMSAN use-of-uninitialized-value warnings?

It is not an issue, because if 'nr' are 0, nothing else is read from the
structures. AFAICS KMSAN will then not complain if the uninitialized
values are not used.

> +	more = false;

Sashiko Gemini is saying:

> If "more" is true and the function loops back to process another batch,
> mptcp_pm_remove_addr() will have already set the MPTCP_RM_ADDR_SIGNAL bit in
> msk->pm.addr_signal during the first iteration.
> Since mptcp_pm_flush_addrs_and_subflows() is called with lock_sock(sk) held,
> the MPTCP TX path cannot run to transmit the RM_ADDR and clear the signal bit
> between iterations.
> When the loop processes the second batch and calls mptcp_pm_remove_addr()
> again, msk->pm.addr_signal is still set.
> Will this cause mptcp_pm_remove_addr() to return -EINVAL and silently drop all
> batches after the first locally?

That's fine: RM_ADDR are notifications that can be lost anyway. What is
important is to remove the different addresses internally.

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