[PATCH mptcp-net v3] mptcp: pm: fix userspace PM address ID overflow when all IDs are exhausted

From: Qing Luo <[email protected]>

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 <[email protected]>
---
 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/[email protected]/

v1: https://lore.kernel.org/all/[email protected]/