Re: [PATCH mptcp-net v4 2/8] mptcp: pm: userspace: lookup: match port in priority

[netdev-bot+sashiko@kernel.org]

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

Critical: 0 · High: 0 · Medium: 1 · Low: 0

If you believe none of the issues are legitimate, please reply to each
issue of Medium severity or higher and explain why the AI is wrong.
If any of them are legitimate and you plan to respin, please let patchwork
know by including "pw-bot: cr" as a separate line at the end of your reply
(one such reply per series is enough).

- [Medium] Joins on a local address with a different port are now
  rejected.

--

mptcp: pm: userspace: lookup: match port in priority

This patch changes mptcp_userspace_pm_lookup_addr() so that, when the
address matches, an entry with the exact same port is returned first.
Otherwise it falls back to an entry where one of the two ports is 0. If
both ports are non-zero and different, the lookup now returns NULL. The
old lookup matched on the address alone.

> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index 45faee894ac3d..68a02b15a4c53 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -31,13 +31,19 @@ static struct mptcp_pm_addr_entry *
>  mptcp_userspace_pm_lookup_addr(struct mptcp_sock *msk,
>  			       const struct mptcp_addr_info *addr)
>  {
> -	struct mptcp_pm_addr_entry *entry;
> +	struct mptcp_pm_addr_entry *entry, *wildcard = NULL;
>
>  	mptcp_for_each_userspace_pm_addr(msk, entry) {
> -		if (mptcp_addresses_equal(&entry->addr, addr, false))
> -			return entry;
> +		if (mptcp_addresses_equal(&entry->addr, addr, false)) {
> +			if (addr->port == entry->addr.port)
> +				return entry;
> +
> +			if (!wildcard && (!addr->port || !entry->addr.port))
> +				wildcard = entry;
> +		}
>  	}
> -	return NULL;
> +
> +	return wildcard;
>  }

[Severity: Medium]
With this change, can an incoming MP_JOIN to a local address with a
different port be rejected?

Take a userspace PM connection where the daemon announced B:X (id 5,
X != 0) on a non-initial address B. The peer then sends MP_JOIN to B:P,
where P is the msk port (listener on a wildcard address). Before this
patch, the lookup matched B:X on the address alone and the join got
id 5. Now both ports are non-zero and different, so the lookup returns
NULL.

On a lookup miss, mptcp_pm_userspace_get_local_id() falls back to
appending a new entry with needs_id set. The skc id is 0, because
mptcp_pm_get_local_id() sets it that way:

    if (skc->addr.port == msk_sport)
        skc->addr.port = 0;

    return mptcp_userspace_pm_append_new_local_addr(msk, skc, true);

The parent commit added an ID0 entry to every userspace PM connection
in mptcp_pm_userspace_created():

    memset(entry, 0, sizeof(*entry));
    mptcp_local_address((struct sock_common *)ssk, &entry->addr);
    entry->addr.port = 0; /* msk port */

In mptcp_userspace_pm_append_new_local_addr(), the loop reaches that
ID0 entry with addr_match false (different address). Since the
requested id is 0, id_match is true:

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

Neither the "!addr_match && !id_match" branch nor the
"addr_match && id_match" branch applies after this, so the function
returns -EINVAL without allocating an ID.

subflow_token_join_request() then sees the negative id and drops the
join:

    local_id = mptcp_pm_get_local_id(msk, (struct sock_common *)req);
    if (local_id < 0) {
        SUBFLOW_REQ_INC_STATS(req, MPTCP_MIB_MPJOINNOIDFOUND);

Any subflow whose address only appears in entries with a different
non-zero port would take the same path. The underlying problem is that
the needs_id append matches id 0 against the ID0 entry. This patch is
what turns the former lookup hit into a miss.

No later patch in the series appears to change the append loop or the
lookup. Only the local_addr_used accounting around it changes. Could
the id 0 case be handled in the append loop, or could the lookup keep
an address-only fallback for this case?

-- 
Sashiko AI review · https://netdev-ai.bots.linux.dev/sashiko/#/patchset/20261029-mptcp-pm-userspace-id0-case-v4-0-555066fa4292%40kernel.org