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.
Also modify the CSF (subflow create) handler to pass !entry.addr.id as
the needs_id parameter. When no local ID is provided by user-space
(entry.addr.id == 0), this triggers auto-allocation instead of silently
using the reserved ID 0.
Fixes: 4638de5aefe5 ("mptcp: handle local addrs announced by userspace PMs")
Assisted-by: LLM # review
Signed-off-by: Qing Luo <luoqing@kylinos.cn>
---
net/mptcp/pm_userspace.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index 945aa5afc2dd..ada7d6cef625 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;
@@ -400,7 +407,8 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
goto create_err;
}
- err = mptcp_userspace_pm_append_new_local_addr(msk, &entry, false);
+ err = mptcp_userspace_pm_append_new_local_addr(msk, &entry,
+ !entry.addr.id);
if (err < 0) {
NL_SET_ERR_MSG_ATTR(info->extack, laddr,
"did not match address and id");
--
2.25.1
Hi luoqing,
(-cc David/Paolo)
Thank you for the patch.
Please only Cc the MPTCP mailing list.
Also, please send new versions in a new email thread, not as a reply to
a previous one.
One last thing: here, you can send the fix and the selftest as part of
the same series, not as separated patches, so the CI will validate the
new selftest with the fix. It is fine to have patches for -net and -next
in the same series. Also, please use 'mptcp-ne(x)t', no capital letters.
On 04/08/2026 04:23, 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.
>
> 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.
>
> Also modify the CSF (subflow create) handler to pass !entry.addr.id as
> the needs_id parameter. When no local ID is provided by user-space
> (entry.addr.id == 0), this triggers auto-allocation instead of silently
> using the reserved ID 0.
>
> Fixes: 4638de5aefe5 ("mptcp: handle local addrs announced by userspace PMs")
> Assisted-by: LLM # review
> Signed-off-by: Qing Luo <luoqing@kylinos.cn>
> ---
> net/mptcp/pm_userspace.c | 18 +++++++++++++-----
> 1 file changed, 13 insertions(+), 5 deletions(-)
>
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index 945aa5afc2dd..ada7d6cef625 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;
> @@ -400,7 +407,8 @@ int mptcp_pm_nl_subflow_create_doit(struct sk_buff *skb, struct genl_info *info)
> goto create_err;
> }
>
> - err = mptcp_userspace_pm_append_new_local_addr(msk, &entry, false);
> + err = mptcp_userspace_pm_append_new_local_addr(msk, &entry,
> + !entry.addr.id);
I don't think you can do that: it is valid to give an ID set to 0. But
it is different to give no ID.
I don't think this modification here should be part of this patch: it is
different from the overflow case you are trying to fix. It could be in
another patch, but that's changing the behaviour, and that's not a fix I
think.
Also, see my series:
https://lore.kernel.org/20260727-mptcp-pm-userspace-id0-case-v1-0-9877f02a9bae@kernel.org
*Maybe*, for -next, we could check if the ID was not set via Netlink
(and not set to 0), and pick an ID for it. *But*, it might not be a good
idea: the userspace daemon will not know the ID that has been picked,
and will need to get it via another request. That doesn't sound like a
good idea, no?
> if (err < 0) {
> NL_SET_ERR_MSG_ATTR(info->extack, laddr,
> "did not match address and id");
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
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): Unstable: 1 failed test(s): selftest_mptcp_join ⚠️
- KVM Validation: debug (except selftest_mptcp_join): Success! ✅
- KVM Validation: debug (only selftest_mptcp_join): Unstable: 1 failed test(s): selftest_mptcp_join ⚠️
- 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/30872730143
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/e6cb13c9a93b
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1139752
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)
From: Qing Luo <luoqing@kylinos.cn>
Add a test that verifies the userspace PM correctly returns an error
when all address IDs (1-255) are exhausted, instead of overflowing.
The test first announces 255 unique addresses (IDs 1-255) to fill the
ID bitmap. It then attempts to create a subflow (CSF) without specifying
a local ID, which triggers auto-allocation via
mptcp_userspace_pm_get_local_id(). With all IDs in use, the allocation
should fail with -ENOSPC.
Also modify pm_nl_ctl to make the 'lid' parameter optional for the CSF
command. When omitted, the kernel auto-allocates a local ID.
MPTCP_PM_MAX_ADDR_ID has been 255 since the userspace PM was introduced,
so no version-dependent limit adjustment is needed (unlike pm_netlink.sh).
Assisted-by: LLM # code
Signed-off-by: Qing Luo <luoqing@kylinos.cn>
---
tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 8 +--
.../selftests/net/mptcp/userspace_pm.sh | 63 +++++++++++++++++++
2 files changed, 67 insertions(+), 4 deletions(-)
diff --git a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
index 78180da1efcc..a9dd650805a9 100644
--- a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
+++ b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
@@ -32,7 +32,7 @@ static void syntax(char *argv[])
fprintf(stderr, "\tadd [flags signal|subflow|backup|fullmesh] [id <nr>] [dev <name>] <ip>\n");
fprintf(stderr, "\tann <local-ip> id <local-id> token <token> [port <local-port>] [dev <name>]\n");
fprintf(stderr, "\trem id <local-id> token <token>\n");
- fprintf(stderr, "\tcsf lip <local-ip> lid <local-id> rip <remote-ip> rport <remote-port> token <token>\n");
+ fprintf(stderr, "\tcsf lip <local-ip> [lid <local-id>] rip <remote-ip> rport <remote-port> token <token>\n");
fprintf(stderr, "\tdsf lip <local-ip> lport <local-port> rip <remote-ip> rport <remote-port> token <token>\n");
fprintf(stderr, "\tdel <id> [<ip>]\n");
fprintf(stderr, "\tget <id>\n");
@@ -481,7 +481,7 @@ int csf(int fd, int pm_family, int argc, char *argv[])
off = init_genl_req(data, pm_family, MPTCP_PM_CMD_SUBFLOW_CREATE,
MPTCP_PM_VER);
- if (argc < 12)
+ if (argc < 10)
syntax(argv);
/* Params recorded in this order:
@@ -557,9 +557,9 @@ int csf(int fd, int pm_family, int argc, char *argv[])
off += NLMSG_ALIGN(rta->rta_len);
}
- if (arg == 0) {
+ if (arg == 0 && params[1]) {
/* id */
- id = atoi(params[arg + 1]);
+ id = atoi(params[1]);
rta = (void *)(data + off);
rta->rta_type = MPTCP_PM_ADDR_ATTR_ID;
rta->rta_len = RTA_LENGTH(1);
diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
index 30a809752d1b..45b040a8c0e5 100755
--- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
+++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
@@ -847,6 +847,68 @@ test_subflows_v4_v6_mix()
sleep 0.5
}
+test_addr_id_overflow()
+{
+ print_title "Address ID overflow tests"
+
+ local i announced=0
+
+ :>"$server_evts"
+
+ # Clear leftover addresses from previous tests
+ for i in $(seq 0 255); do
+ ip netns exec "$ns2" ./pm_nl_ctl rem token "$client4_token" id "$i" > /dev/null 2>&1
+ done
+
+ # Announce 255 addresses (IDs 1-255) to exhaust all available IDs
+ for i in $(seq 1 255); do
+ if ip netns exec "$ns2" ./pm_nl_ctl ann 10.0.3."${i}" token "$client4_token" id \
+ "$i" dev ns2eth1 > /dev/null 2>&1; then
+ announced=$((announced + 1))
+ fi
+ done
+
+ print_test "ADD_ADDR with all IDs 1-255 exhausted"
+ sleep 1
+ if [ -s "$server_evts" ]; then
+ test_pass
+ else
+ test_fail "No events generated"
+ return
+ fi
+
+ # Start listener to ensure subflow creation doesn't fail on connectivity
+ ip netns exec "$ns1" ./pm_nl_ctl listen 10.0.1.1 "$app4_port" >/dev/null 2>&1 &
+ local listener_pid=$!
+ sleep 0.5
+
+ # Try to create a subflow without specifying a local ID.
+ # With all IDs exhausted, this should fail with -ENOSPC.
+ print_test "CSF without local ID after all IDs exhausted - expect failure"
+ local out
+ if out=$(ip netns exec "$ns2" ./pm_nl_ctl csf lip 10.0.1.2 \
+ rip 10.0.1.1 rport "$app4_port" token "$client4_token" 2>&1); then
+ test_fail "Expected failure but CSF succeeded"
+ else
+ # pm_nl_ctl prints the kernel error as "netlink error -28 (No space
+ # left on device)" for -ENOSPC. Match either form.
+ if echo "$out" | grep -qE "netlink error -?28|No space left on device"; then
+ test_pass
+ else
+ test_fail "CSF failed, but not with the expected ENOSPC: ${out}"
+ fi
+ fi
+
+ # Delete the listener from the server ns, if one was created
+ mptcp_lib_kill_wait $listener_pid
+
+ # Cleanup: remove all announced addresses
+ for i in $(seq 1 255); do
+ ip netns exec "$ns2" ./pm_nl_ctl rem token "$client4_token" id "$i" > /dev/null 2>&1
+ done
+ sleep 1
+}
+
test_prio()
{
print_title "Prio tests"
@@ -940,6 +1002,7 @@ test_subflows
test_subflows_v4_v6_mix
test_prio
test_listener
+test_addr_id_overflow
mptcp_lib_result_print_all_tap
exit ${ret}
--
2.25.1
Hi luoqing,
On 04/08/2026 04:23, luoqing wrote:
> From: Qing Luo <luoqing@kylinos.cn>
>
> Add a test that verifies the userspace PM correctly returns an error
> when all address IDs (1-255) are exhausted, instead of overflowing.
>
> The test first announces 255 unique addresses (IDs 1-255) to fill the
> ID bitmap. It then attempts to create a subflow (CSF) without specifying
> a local ID, which triggers auto-allocation via
> mptcp_userspace_pm_get_local_id(). With all IDs in use, the allocation
> should fail with -ENOSPC.
>
> Also modify pm_nl_ctl to make the 'lid' parameter optional for the CSF
> command. When omitted, the kernel auto-allocates a local ID.
Same as for the other patch: should be in a different patch, but can
certainly be dropped.
> MPTCP_PM_MAX_ADDR_ID has been 255 since the userspace PM was introduced,
> so no version-dependent limit adjustment is needed (unlike pm_netlink.sh).
>
> Assisted-by: LLM # code
> Signed-off-by: Qing Luo <luoqing@kylinos.cn>
> ---
> tools/testing/selftests/net/mptcp/pm_nl_ctl.c | 8 +--
> .../selftests/net/mptcp/userspace_pm.sh | 63 +++++++++++++++++++
> 2 files changed, 67 insertions(+), 4 deletions(-)
>
> diff --git a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
> index 78180da1efcc..a9dd650805a9 100644
> --- a/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
> +++ b/tools/testing/selftests/net/mptcp/pm_nl_ctl.c
> @@ -32,7 +32,7 @@ static void syntax(char *argv[])
> fprintf(stderr, "\tadd [flags signal|subflow|backup|fullmesh] [id <nr>] [dev <name>] <ip>\n");
> fprintf(stderr, "\tann <local-ip> id <local-id> token <token> [port <local-port>] [dev <name>]\n");
> fprintf(stderr, "\trem id <local-id> token <token>\n");
> - fprintf(stderr, "\tcsf lip <local-ip> lid <local-id> rip <remote-ip> rport <remote-port> token <token>\n");
> + fprintf(stderr, "\tcsf lip <local-ip> [lid <local-id>] rip <remote-ip> rport <remote-port> token <token>\n");
> fprintf(stderr, "\tdsf lip <local-ip> lport <local-port> rip <remote-ip> rport <remote-port> token <token>\n");
> fprintf(stderr, "\tdel <id> [<ip>]\n");
> fprintf(stderr, "\tget <id>\n");
> @@ -481,7 +481,7 @@ int csf(int fd, int pm_family, int argc, char *argv[])
> off = init_genl_req(data, pm_family, MPTCP_PM_CMD_SUBFLOW_CREATE,
> MPTCP_PM_VER);
>
> - if (argc < 12)
> + if (argc < 10)
> syntax(argv);
>
> /* Params recorded in this order:
> @@ -557,9 +557,9 @@ int csf(int fd, int pm_family, int argc, char *argv[])
> off += NLMSG_ALIGN(rta->rta_len);
> }
>
> - if (arg == 0) {
> + if (arg == 0 && params[1]) {
> /* id */
> - id = atoi(params[arg + 1]);
> + id = atoi(params[1]);
> rta = (void *)(data + off);
> rta->rta_type = MPTCP_PM_ADDR_ATTR_ID;
> rta->rta_len = RTA_LENGTH(1);
> diff --git a/tools/testing/selftests/net/mptcp/userspace_pm.sh b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> index 30a809752d1b..45b040a8c0e5 100755
> --- a/tools/testing/selftests/net/mptcp/userspace_pm.sh
> +++ b/tools/testing/selftests/net/mptcp/userspace_pm.sh
> @@ -847,6 +847,68 @@ test_subflows_v4_v6_mix()
> sleep 0.5
> }
>
> +test_addr_id_overflow()
> +{
> + print_title "Address ID overflow tests"
> +
> + local i announced=0
> +
> + :>"$server_evts"
> +
> + # Clear leftover addresses from previous tests
> + for i in $(seq 0 255); do
> + ip netns exec "$ns2" ./pm_nl_ctl rem token "$client4_token" id "$i" > /dev/null 2>&1
> + done
Is this really needed?
> +
> + # Announce 255 addresses (IDs 1-255) to exhaust all available IDs
> + for i in $(seq 1 255); do
> + if ip netns exec "$ns2" ./pm_nl_ctl ann 10.0.3."${i}" token "$client4_token" id \
> + "$i" dev ns2eth1 > /dev/null 2>&1; then
Why hiding errors? (OK to hide if it is just to use all bits, then try
one extra to check the error)
> + announced=$((announced + 1))
Set but not used?
> + fi
> + done
Mmh, all of this is very slow: 19 seconds on the CI with a "normal"
kconfig, 52 seconds with a "debug" one...
That's a lot... Any ideas on how to reduce this time? (maybe not possible?)
> +
> + print_test "ADD_ADDR with all IDs 1-255 exhausted"
> + sleep 1
Why this "sleep 1"?
> + if [ -s "$server_evts" ]; then
> + test_pass
> + else
> + test_fail "No events generated"
> + return
> + fi
> +
> + # Start listener to ensure subflow creation doesn't fail on connectivity
Really needed?
> + ip netns exec "$ns1" ./pm_nl_ctl listen 10.0.1.1 "$app4_port" >/dev/null 2>&1 &
> + local listener_pid=$!
> + sleep 0.5
> +
> + # Try to create a subflow without specifying a local ID.
> + # With all IDs exhausted, this should fail with -ENOSPC.
> + print_test "CSF without local ID after all IDs exhausted - expect failure"
> + local out
> + if out=$(ip netns exec "$ns2" ./pm_nl_ctl csf lip 10.0.1.2 \
> + rip 10.0.1.1 rport "$app4_port" token "$client4_token" 2>&1); then
I guess you cannot have an overflow with csf, because an ID is required.
> + test_fail "Expected failure but CSF succeeded"
> + else
> + # pm_nl_ctl prints the kernel error as "netlink error -28 (No space
> + # left on device)" for -ENOSPC. Match either form.
Really needed? The pm_nl_ctl should fail.
> + if echo "$out" | grep -qE "netlink error -?28|No space left on device"; then
> + test_pass
> + else
> + test_fail "CSF failed, but not with the expected ENOSPC: ${out}"
> + fi
> + fi
> +
> + # Delete the listener from the server ns, if one was created
> + mptcp_lib_kill_wait $listener_pid
> +
> + # Cleanup: remove all announced addresses
> + for i in $(seq 1 255); do
> + ip netns exec "$ns2" ./pm_nl_ctl rem token "$client4_token" id "$i" > /dev/null 2>&1
> + done
Maybe not needed?
> + sleep 1
Clearly not needed.
> +}
> +
> test_prio()
> {
> print_title "Prio tests"
> @@ -940,6 +1002,7 @@ test_subflows
> test_subflows_v4_v6_mix
> test_prio
> test_listener
> +test_addr_id_overflow
>
> mptcp_lib_result_print_all_tap
> exit ${ret}
Cheers,
Matt
--
Sponsored by the NGI0 Core fund.
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): Unstable: 1 failed test(s): selftest_userspace_pm ⚠️
- KVM Validation: normal (only selftest_mptcp_join): Success! ✅
- KVM Validation: debug (except selftest_mptcp_join): Unstable: 1 failed test(s): selftest_userspace_pm ⚠️
- 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/30872689423
Initiator: Patchew Applier
Commits: https://github.com/multipath-tcp/mptcp_net-next/commits/dc56ec09ff01
Patchwork: https://patchwork.kernel.org/project/mptcp/list/?series=1139751
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)
© 2016 - 2026 Red Hat, Inc.