Re: [PATCH net v2 04/15] mptcp: pm: kernel: drop pending ADD_ADDR when removing ID0

[Matthieu Baerts <matttbe@kernel.org>]

Hello,

On 09/09/2026 16:49, netdev-bot+sashiko@kernel.org wrote:
> Thank you for your contribution! Sashiko AI review found 2 potential
> issue(s) to consider:
> 
> Critical: 0 · High: 1 · Medium: 1 · Low: 0
> 
> - [High] Incomplete fix: in mptcp_nl_remove_id_zero_address()
>   (net/mptcp/pm_kernel.c) the new cleanup forces `anno_addr.port = 0`…
> - [Medium] Newly introduced wrong-entry teardown in
>   mptcp_nl_remove_id_zero_address() (net/mptcp/pm_kernel.c:1152-1157):…
> 
> --
> 
> mptcp: pm: kernel: drop pending ADD_ADDR when removing ID0
> 
> This makes the in-kernel path manager's id 0 removal path symmetric with the
> non-zero id path by dropping a pending ADD_ADDR announcement and decrementing
> add_addr_signaled before queuing the RM_ADDR.  The lookup key is the msk local
> address with the port forced to 0.
> 
> Closes: https://github.com/multipath-tcp/mptcp_net-next/issues/620
> 
>> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
>> index 424f1a7f9248..1a7750813235 100644
>> --- a/net/mptcp/pm_kernel.c
>> +++ b/net/mptcp/pm_kernel.c
>> @@ -1137,6 +1137,8 @@ static int mptcp_nl_remove_id_zero_address(struct net *net,
>>  	while ((msk = mptcp_token_iter_next(net, &s_slot, &s_num)) != NULL) {
>>  		struct sock *sk = (struct sock *)msk;
>>  		struct mptcp_addr_info msk_local;
>> +		struct mptcp_addr_info anno_addr;
>> +		bool announced;
>>  
>>  		if (list_empty(&msk->conn_list) || mptcp_pm_is_userspace(msk))
>>  			goto next;
>> @@ -1146,7 +1148,13 @@ static int mptcp_nl_remove_id_zero_address(struct net *net,
>>  			goto next;
>>  
>>  		lock_sock(sk);
>> +		/* Drop a possibly pending ADD_ADDR for this address. */
>> +		anno_addr = msk_local;
>> +		anno_addr.port = 0;
>> +		announced = mptcp_pm_announced_remove(msk, &anno_addr);
> 
> [Severity: High]
> Does zeroing the port here make the lookup miss the announcements this
> patch is meant to remove?
> 
> The commit message says:
> 
>     Subtle detail: signal endpoints are stored in anno_list with port 0, but
>     msk_local carries the connection's local port. In other words, entries
>     linked to ID0 paths should have port == 0. A follow-up patch will ensure
>     that.
As mentioned, a follow-up patch to fix this non-directly related issue
is in preparation. The series was already 15 patches long, it has not
been included and will be sent later on.

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