All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH mptcp-net v3] mptcp: pm: fix userspace PM address ID overflow when all IDs are exhausted
@ 2026-08-05  7:09 luoqing
  2026-08-05  8:30 ` MPTCP CI
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: luoqing @ 2026-08-05  7:09 UTC (permalink / raw)
  To: mptcp

From: Qing Luo <luoqing@kylinos.cn>

When all MPTCP address IDs (1-255) are exhausted in the userspace PM,
find_next_zero_bit() returns MPTCP_PM_MAX_ADDR_ID + 1 (256). This value
overflows when stored in the u8 field e->addr.id, resulting in ID 0
being stored and the entry being incorrectly added to the list.

ID 0 is reserved for the initial connection in MPTCP, so this overflow
can cause address conflicts.

Note: the in-kernel PM already has an 'endpoints == MPTCP_PM_MAX_ADDR_ID'
check in mptcp_pm_nl_append_new_local_addr() that returns -ERANGE before
reaching find_next_zero_bit(), preventing this overflow. So this fix only
addresses the userspace PM path.

Store the find_next_zero_bit() result in a temporary unsigned int, check
against MPTCP_PM_MAX_ADDR_ID, and return -ENOSPC if all IDs are truly
exhausted. Properly free the allocated entry with sock_kfree_s() on error.

Fixes: 4638de5aefe5 ("mptcp: handle local addrs announced by userspace PMs")
Assisted-by: LLM
Signed-off-by: Qing Luo <luoqing@kylinos.cn>
---
 net/mptcp/pm_userspace.c | 15 +++++++++++----
 1 file changed, 11 insertions(+), 4 deletions(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 945aa5afc2dd..7d0e343c35ed 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -74,10 +74,17 @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
 			goto append_err;
 		}
 
-		if (!e->addr.id && needs_id)
-			e->addr.id = find_next_zero_bit(id_bitmap,
-							MPTCP_PM_MAX_ADDR_ID + 1,
-							1);
+		if (!e->addr.id && needs_id) {
+			unsigned int id = find_next_zero_bit(id_bitmap,
+							     MPTCP_PM_MAX_ADDR_ID + 1,
+							     1);
+			if (id > MPTCP_PM_MAX_ADDR_ID) {
+				sock_kfree_s(sk, e, sizeof(*e));
+				ret = -ENOSPC;
+				goto append_err;
+			}
+			e->addr.id = id;
+		}
 		list_add_tail_rcu(&e->list, &msk->pm.userspace_pm_local_addr_list);
 		msk->pm.local_addr_used++;
 		ret = e->addr.id;
-- 
2.25.1

v3: 
Hi Matthieu,

Thank you for your detailed review and for pointing out the issues with my patch. 

Based on your feedback, I will withdraw this patch submission. The core fix for the ID overflow is valid, but my patch became muddled with an unnecessary and potentially harmful behavioral change.

If you believe the core fix (checking find_next_zero_bit's return value against MPTCP_PM_MAX_ADDR_ID) is still worth submitting on its own, I can prepare a clean v3 that only contains that fix and removes the needs_id logic change. Otherwise, I am happy to let this go.

Please let me know your preference.

Best regards,
luoqing

v2: https://lore.kernel.org/all/63f14fa5-f2d3-46e4-bb3a-f02430701cc8@kernel.org/

v1: https://lore.kernel.org/all/20260714080356.805839-1-l1138897701@163.com/


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH mptcp-net v3] mptcp: pm: fix userspace PM address ID overflow when all IDs are exhausted
  2026-08-05  7:09 [PATCH mptcp-net v3] mptcp: pm: fix userspace PM address ID overflow when all IDs are exhausted luoqing
@ 2026-08-05  8:30 ` MPTCP CI
  2026-08-05 10:59 ` Matthieu Baerts (NGI0)
  2026-08-07  7:41 ` [PATCH mptcp-net v4] " luoqing
  2 siblings, 0 replies; 7+ messages in thread
From: MPTCP CI @ 2026-08-05  8:30 UTC (permalink / raw)
  To: Qing Luo; +Cc: mptcp

Hi Qing,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/30985448641

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/04610d1b68b9
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1140580


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH mptcp-net v3] mptcp: pm: fix userspace PM address ID overflow when all IDs are exhausted
  2026-08-05  7:09 [PATCH mptcp-net v3] mptcp: pm: fix userspace PM address ID overflow when all IDs are exhausted luoqing
  2026-08-05  8:30 ` MPTCP CI
@ 2026-08-05 10:59 ` Matthieu Baerts (NGI0)
  2026-08-07  7:41 ` [PATCH mptcp-net v4] " luoqing
  2 siblings, 0 replies; 7+ messages in thread
From: Matthieu Baerts (NGI0) @ 2026-08-05 10:59 UTC (permalink / raw)
  To: luoqing; +Cc: mptcp

Hi luoqing,

> When all MPTCP address IDs (1-255) are exhausted in the userspace PM,
> find_next_zero_bit() returns MPTCP_PM_MAX_ADDR_ID + 1 (256). This value
> overflows when stored in the u8 field e->addr.id, resulting in ID 0
> being stored and the entry being incorrectly added to the list.
>
> ID 0 is reserved for the initial connection in MPTCP, so this overflow
> can cause address conflicts.
>
> Note: the in-kernel PM already has an 'endpoints == MPTCP_PM_MAX_ADDR_ID'
> check in mptcp_pm_nl_append_new_local_addr() that returns -ERANGE before
> reaching find_next_zero_bit(), preventing this overflow. So this fix only
> addresses the userspace PM path.
>
> Store the find_next_zero_bit() result in a temporary unsigned int, check
> against MPTCP_PM_MAX_ADDR_ID, and return -ENOSPC if all IDs are truly
> exhausted. Properly free the allocated entry with sock_kfree_s() on error.

Thank you for the new version.

> Fixes: 4638de5aefe5 ("mptcp: handle local addrs announced by userspace PMs")
> Assisted-by: LLM
> Signed-off-by: Qing Luo <luoqing@kylinos.cn>

(...)

> Based on your feedback, I will withdraw this patch submission. The core fix
> for the ID overflow is valid, but my patch became muddled with an unnecessary
> and potentially harmful behavioral change.
>
> If you believe the core fix (checking find_next_zero_bit's return value
> against MPTCP_PM_MAX_ADDR_ID) is still worth submitting on its own, I can
> prepare a clean v3 that only contains that fix and removes the needs_id logic
> change. Otherwise, I am happy to let this go.

I'm sorry, this is confusing: this is the v3 with only the (valid) fix,
no?

Do you have issues to send replies to my previous emails? Because
sending your replies here at the end of a patch is unusual, and these
comments are stripped when replying to an existing email.

>
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index 945aa5afc2dd..7d0e343c35ed 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -74,10 +74,17 @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
>  			goto append_err;
>  		}
>
> -		if (!e->addr.id && needs_id)
> -			e->addr.id = find_next_zero_bit(id_bitmap,
> -							MPTCP_PM_MAX_ADDR_ID + 1,
> -							1);
> +		if (!e->addr.id && needs_id) {
> +			unsigned int id = find_next_zero_bit(id_bitmap,
> +							     MPTCP_PM_MAX_ADDR_ID + 1,
> +							     1);
> +			if (id > MPTCP_PM_MAX_ADDR_ID) {
> +				sock_kfree_s(sk, e, sizeof(*e));
> +				ret = -ENOSPC;
> +				goto append_err;
> +			}
> +			e->addr.id = id;
> +		}

The fix looks good to me, but I wonder if it wouldn't make more sense to
check this before allocating 'e', no?

e.g.

	if (!addr_match && !id_match) {
		struct mptcp_pm_addr_entry *e;
		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;
		}

		e = sock_kmemdup(sk, entry, sizeof(*entry), GFP_ATOMIC);
		if (!e) {
			ret = -ENOMEM;
			goto append_err;
		}

		e->addr.id = id;
		list_add_tail_rcu(&e->list, &msk->pm.userspace_pm_local_addr_list);
		msk->pm.local_addr_used++;
		ret = e->addr.id;

WDYT?

A bit more code, but we avoid having to handle the free.

-- 
Matthieu Baerts (NGI0) <matttbe@kernel.org>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH mptcp-net v4] mptcp: pm: fix userspace PM address ID overflow when all IDs are exhausted
  2026-08-05  7:09 [PATCH mptcp-net v3] mptcp: pm: fix userspace PM address ID overflow when all IDs are exhausted luoqing
  2026-08-05  8:30 ` MPTCP CI
  2026-08-05 10:59 ` Matthieu Baerts (NGI0)
@ 2026-08-07  7:41 ` luoqing
  2026-08-07  8:47   ` MPTCP CI
                     ` (2 more replies)
  2 siblings, 3 replies; 7+ messages in thread
From: luoqing @ 2026-08-07  7:41 UTC (permalink / raw)
  To: l1138897701; +Cc: mptcp

From: Qing Luo <luoqing@kylinos.cn>

When all MPTCP address IDs (1-255) are exhausted in the userspace PM,
find_next_zero_bit() returns MPTCP_PM_MAX_ADDR_ID + 1 (256). This value
overflows when stored in the u8 field e->addr.id, resulting in ID 0
being stored and the entry being incorrectly added to the list.

ID 0 is reserved for the initial connection in MPTCP, so this overflow
can cause address conflicts.

Note: the in-kernel PM already has an 'endpoints == MPTCP_PM_MAX_ADDR_ID'
check in mptcp_pm_nl_append_new_local_addr() that returns -ERANGE before
reaching find_next_zero_bit(), preventing this overflow. So this fix only
addresses the userspace PM path.

Check the find_next_zero_bit() result against MPTCP_PM_MAX_ADDR_ID and
return -ENOSPC if all IDs are truly exhausted. Move the ID allocation
check before the memory allocation so that the error path does not need
to free the allocated entry.

Fixes: 4638de5aefe5 ("mptcp: handle local addrs announced by userspace PMs")
Assisted-by: LLM
Signed-off-by: Qing Luo <luoqing@kylinos.cn>
---
v4: Move the ID allocation check before the memory allocation so that the error path 
    does not need to free the allocated entry

v3: Submit separately to only verify the return value of find_next_zero_bit

v2: https://lore.kernel.org/all/63f14fa5-f2d3-46e4-bb3a-f02430701cc8@kernel.org/

v1: https://lore.kernel.org/all/20260714080356.805839-1-l1138897701@163.com/
---
 net/mptcp/pm_userspace.c | 18 ++++++++++++++----
 1 file changed, 14 insertions(+), 4 deletions(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 2203cc2d2748..57c13293a5f8 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) {
+		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;
+		}
+
 		/* Memory for the entry is allocated from the
 		 * sock option buffer.
 		 */
@@ -78,10 +91,7 @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
 			goto append_err;
 		}
 
-		if (!e->addr.id && needs_id)
-			e->addr.id = find_next_zero_bit(id_bitmap,
-							MPTCP_PM_MAX_ADDR_ID + 1,
-							1);
+		e->addr.id = id;
 		list_add_tail_rcu(&e->list, &msk->pm.userspace_pm_local_addr_list);
 		msk->pm.local_addr_used++;
 		ret = e->addr.id;
-- 
2.25.1

> Do you have issues to send replies to my previous emails? Because
> sending your replies here at the end of a patch is unusual, and these
> comments are stripped when replying to an existing email.
Hi,Matthieu

Thanks a lot for your detailed review and pointing out these problems.

I have to admit I was a bit confused about the correct mailing‑list workflow, especially whether I should start a brand‑new thread or simply reply when sending new patch versions.

I will spend more time studying the community patch submission workflow to avoid similar mistakes next time.
I plan to release the v4 version soon regarding the reply on this old thread. Thank you for your guidance.

Thanks,
luoqing


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH mptcp-net v4] mptcp: pm: fix userspace PM address ID overflow when all IDs are exhausted
  2026-08-07  7:41 ` [PATCH mptcp-net v4] " luoqing
@ 2026-08-07  8:47   ` MPTCP CI
  2026-08-07  8:50   ` Matthieu Baerts
  2026-08-07 10:25   ` Matthieu Baerts
  2 siblings, 0 replies; 7+ messages in thread
From: MPTCP CI @ 2026-08-07  8:47 UTC (permalink / raw)
  To: Qing Luo; +Cc: mptcp

Hi Qing,

Thank you for your modifications, that's great!

Our CI did some validations and here is its report:

- KVM Validation: normal (except selftest_mptcp_join): Success! ✅
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Success! ✅
- KVM Validation: btf-normal (only bpftest_all): Success! ✅
- KVM Validation: btf-debug (only bpftest_all): Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/31160130229

Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/4f69b893564e
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1141997


If there are some issues, you can reproduce them using the same environment as
the one used by the CI thanks to a docker image, e.g.:

    $ cd [kernel source code]
    $ docker run -v "${PWD}:${PWD}:rw" -w "${PWD}" --privileged --rm -it \
        --pull always mptcp/mptcp-upstream-virtme-docker:latest \
        auto-normal

For more details:

    https://github.com/multipath-tcp/mptcp-upstream-virtme-docker


Please note that despite all the efforts that have been already done to have a
stable tests suite when executed on a public CI like here, it is possible some
reported issues are not due to your modifications. Still, do not hesitate to
help us improve that ;-)

Cheers,
MPTCP GH Action bot
Bot operated by Matthieu Baerts (NGI0 Core)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH mptcp-net v4] mptcp: pm: fix userspace PM address ID overflow when all IDs are exhausted
  2026-08-07  7:41 ` [PATCH mptcp-net v4] " luoqing
  2026-08-07  8:47   ` MPTCP CI
@ 2026-08-07  8:50   ` Matthieu Baerts
  2026-08-07 10:25   ` Matthieu Baerts
  2 siblings, 0 replies; 7+ messages in thread
From: Matthieu Baerts @ 2026-08-07  8:50 UTC (permalink / raw)
  To: luoqing; +Cc: mptcp

Hi luoqing,

On 07/08/2026 09:41, luoqing wrote:
> I have to admit I was a bit confused about the correct mailing‑list
> workflow, especially whether I should start a brand‑new thread or
> simply reply when sending new patch versions.

In short:

- Never use "git send-email --in-reply-to=(...)" (except when requested)
  => Tip: use "b4 prep/send" to prepare and send patches
     https://www.mptcp.dev/contributing.html#workflow

- Reply to questions with your email client, never with "git send-email"
  => Tip: use Evolution, Thunderbird, Mutt, etc.
     https://docs.kernel.org/process/email-clients.html

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH mptcp-net v4] mptcp: pm: fix userspace PM address ID overflow when all IDs are exhausted
  2026-08-07  7:41 ` [PATCH mptcp-net v4] " luoqing
  2026-08-07  8:47   ` MPTCP CI
  2026-08-07  8:50   ` Matthieu Baerts
@ 2026-08-07 10:25   ` Matthieu Baerts
  2 siblings, 0 replies; 7+ messages in thread
From: Matthieu Baerts @ 2026-08-07 10:25 UTC (permalink / raw)
  To: luoqing; +Cc: mptcp

Hi luoqing,

On 07/08/2026 09:41, luoqing wrote:
> From: Qing Luo <luoqing@kylinos.cn>
> 
> When all MPTCP address IDs (1-255) are exhausted in the userspace PM,
> find_next_zero_bit() returns MPTCP_PM_MAX_ADDR_ID + 1 (256). This value
> overflows when stored in the u8 field e->addr.id, resulting in ID 0
> being stored and the entry being incorrectly added to the list.
> 
> ID 0 is reserved for the initial connection in MPTCP, so this overflow
> can cause address conflicts.
> 
> Note: the in-kernel PM already has an 'endpoints == MPTCP_PM_MAX_ADDR_ID'
> check in mptcp_pm_nl_append_new_local_addr() that returns -ERANGE before
> reaching find_next_zero_bit(), preventing this overflow. So this fix only
> addresses the userspace PM path.
> 
> Check the find_next_zero_bit() result against MPTCP_PM_MAX_ADDR_ID and
> return -ENOSPC if all IDs are truly exhausted. Move the ID allocation
> check before the memory allocation so that the error path does not need
> to free the allocated entry.
> 
> Fixes: 4638de5aefe5 ("mptcp: handle local addrs announced by userspace PMs")
> Assisted-by: LLM
> Signed-off-by: Qing Luo <luoqing@kylinos.cn>
> ---
> v4: Move the ID allocation check before the memory allocation so that the error path 
>     does not need to free the allocated entry
Thanks, now in our tree:

New patches for t/upstream-net and t/upstream:
- b57c0c6fddb3: mptcp: pm: fix userspace PM address ID overflow when all
IDs are exhausted
- Results: 4439056e6ccd..d0562342cb80 (export-net)
- Results: e8891858656d..f393de6a43c3 (export)

Tests are now in progress:

- export-net:
https://github.com/multipath-tcp/mptcp_net-next/commit/1f6dc25f039e11595558280af68c916da4e86f71/checks
- export:
https://github.com/multipath-tcp/mptcp_net-next/commit/3601198557ef7a1d08e411ac4ca4aa5e636d4e3e/checks

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


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2026-08-07 10:25 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-05  7:09 [PATCH mptcp-net v3] mptcp: pm: fix userspace PM address ID overflow when all IDs are exhausted luoqing
2026-08-05  8:30 ` MPTCP CI
2026-08-05 10:59 ` Matthieu Baerts (NGI0)
2026-08-07  7:41 ` [PATCH mptcp-net v4] " luoqing
2026-08-07  8:47   ` MPTCP CI
2026-08-07  8:50   ` Matthieu Baerts
2026-08-07 10:25   ` Matthieu Baerts

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.