Re: [PATCH mptcp-net 1/2] mptcp: pm: in-kernel: always set as unavail when removing addr

[Matthieu Baerts <matttbe@kernel.org>]

Hi Mat,

Thank you for the review!

On 15/01/2026 05:56, Mat Martineau wrote:
> On Mon, 15 Dec 2025, Matthieu Baerts (NGI0) wrote:
> 
>> Syzkaller managed to find a combination of actions that was generating
>> this warning:

(...)

>> diff --git a/net/mptcp/pm_kernel.c b/net/mptcp/pm_kernel.c
>> index f59d21e7579c..51bcfcec882d 100644
>> --- a/net/mptcp/pm_kernel.c
>> +++ b/net/mptcp/pm_kernel.c
>> @@ -1057,10 +1057,8 @@ static bool mptcp_pm_remove_anno_addr(struct
>> mptcp_sock *msk,
>>     ret = mptcp_remove_anno_list_by_saddr(msk, addr);
>>     if (ret || force) {
>>         spin_lock_bh(&msk->pm.lock);
>> -        if (ret) {
>> -            __set_bit(addr->id, msk->pm.id_avail_bitmap);
>> +        if (ret)
>>             msk->pm.add_addr_signaled--;
>> -        }
>>         mptcp_pm_remove_addr(msk, &list);
>>         spin_unlock_bh(&msk->pm.lock);
>>     }
>> @@ -1098,17 +1096,14 @@ static int
>> mptcp_nl_remove_subflow_and_signal_addr(struct net *net,
>>                       !(entry->flags & MPTCP_PM_ADDR_FLAG_IMPLICIT));
>>
>>         list.ids[0] = mptcp_endp_get_local_id(msk, addr);
>> -        if (remove_subflow) {
>> -            spin_lock_bh(&msk->pm.lock);
>> -            mptcp_pm_rm_subflow(msk, &list);
>> -            spin_unlock_bh(&msk->pm.lock);
>> -        }
>>
>> -        if (entry->flags & MPTCP_PM_ADDR_FLAG_SUBFLOW) {
>> -            spin_lock_bh(&msk->pm.lock);
>> +        spin_lock_bh(&msk->pm.lock);
>> +        if (remove_subflow)
>> +            mptcp_pm_rm_subflow(msk, &list);
>> +        if (entry->flags & MPTCP_PM_ADDR_FLAG_SUBFLOW)
>>             __mark_subflow_endp_available(msk, list.ids[0]);
>> -            spin_unlock_bh(&msk->pm.lock);
>> -        }
>> +        __set_bit(addr->id, msk->pm.id_avail_bitmap);
> 
> There's not any harm in setting this bit a second time if it was also
> set in __mark_subflow_endp_available().
> 
> However, __mark_subflow_endp_available() has some logic around ID 0 and
> mpc_endpoint_id. Is that relevant in this code path or is the new
> __set_bit() doing the correct thing by always clearing based on addr->id?

Good point. Even if there is no harm, no need to set the bit for ID 0. I
will look at that!

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