Re: [PATCH net v2 08/15] mptcp: pm: userspace: fix address ID overflow

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Incorrect ID collision check permanently breaks ID allocation if an entry with ID 0 exists.
--

commit 0ac6c6a2ecc7912387ac1b8203c8c6f77120a794
Author: Qing Luo <luoqing@kylinos.cn>
mptcp: pm: userspace: fix address ID overflow

This commit fixes an address ID overflow in the userspace PM when all
IDs (1-255) are exhausted. By returning -ENOSPC when the next available ID
exceeds the maximum, it prevents ID 0 from being incorrectly allocated.
The ID allocation check is also moved before memory allocation to simplify
the error path.

> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index b94fbb483bf94..fab16d953dbfe 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -69,6 +69,19 @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
>  	}
>  
>  	if (!addr_match && !id_match) {

[Severity: High]
This isn't a bug introduced by this patch, but does the loop preceding this
block permanently break ID allocation if an entry with ID 0 exists?

If needs_id is true and no address match is found, entry->addr.id remains 0.
The check in the loop:

    id_match = (e->addr.id == entry->addr.id);
    if (addr_match || id_match)
        break;

evaluates to true if an existing entry has ID 0. This breaks the loop early
and causes the function to return -EINVAL instead of allocating a new ID.

A privileged user (CAP_NET_ADMIN) can configure the userspace PM to create a
subflow using the initial connection address (omitting the ID or specifying
ID 0) via mptcp_pm_nl_subflow_create_doit(). This legitimately adds an entry
with ID 0 to the userspace_pm_local_addr_list. Later, when an event requires
a new implicit ID allocation (e.g., calling
mptcp_userspace_pm_get_local_id()), needs_id is true and entry->addr.id is
initially 0.

Could this incorrectly match the unassigned ID against the existing entry
with ID 0, permanently breaking MPTCP connection management for the socket?

> +		unsigned int id;
> +
> +		if (!entry->addr.id && needs_id) {
> +			id = find_next_zero_bit(id_bitmap,
> +						MPTCP_PM_MAX_ADDR_ID + 1, 1);
> +			if (id > MPTCP_PM_MAX_ADDR_ID) {
> +				ret = -ENOSPC;
> +				goto append_err;
> +			}
> +		} else {
> +			id = entry->addr.id;
> +		}
> +

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-net-mptcp-misc-fixes-7-3-rc1-v2-0-df1de70348b6@kernel.org?part=8