[PATCH net v1] mptcp: pm: fix use-after-free in userspace_pm_get_local_id()

[xuanqiang.luo@linux.dev]

From: Xuanqiang Luo <luoxuanqiang@kylinos.cn>

mptcp_userspace_pm_get_local_id() looks up an address entry with
pm.lock held, but drops the lock before reading its ID. A concurrent
subflow destroy command can remove and free the entry in between,
resulting in a use-after-free while processing an MP_JOIN SYN.

Read the ID while holding pm.lock, then use the copied value after
unlocking.

Fixes: f012d796a6de ("mptcp: check addrs list in userspace_pm_get_local_id")
Cc: stable@vger.kernel.org
Signed-off-by: Xuanqiang Luo <luoxuanqiang@kylinos.cn>
---
The race window is narrow. It was reproduced only with a locally
constructed stress test that repeatedly overlaps an MP_JOIN SYN with a
MPTCP_PM_CMD_SUBFLOW_DESTROY request.

However, the KASAN report below confirms that the race is reachable:

[  666.319362] ==================================================================
[  666.319376] BUG: KASAN: slab-use-after-free in mptcp_userspace_pm_get_local_id+0x1dc/0x1f0
[  666.319386] Read of size 1 at addr ffff888124845610 by task swapper/0/0
...
[  666.319401] Call Trace:
[  666.319405]  <IRQ>
[  666.319408]  dump_stack_lvl+0x53/0x70
[  666.319412]  print_address_description.constprop.0+0x2c/0x3b0
[  666.319418]  print_report+0xbe/0x2b0
[  666.319421]  ? mptcp_userspace_pm_get_local_id+0x1dc/0x1f0
[  666.319423]  kasan_report+0xce/0x100
[  666.319426]  ? mptcp_userspace_pm_get_local_id+0x1dc/0x1f0
[  666.319429]  mptcp_userspace_pm_get_local_id+0x1dc/0x1f0
[  666.319433]  mptcp_pm_get_local_id+0x371/0x440
...
[  666.319821] Allocated by task 45539:
[  666.319844]  kasan_save_stack+0x33/0x60
[  666.319855]  kasan_save_track+0x14/0x30
[  666.319858]  __kasan_kmalloc+0x8f/0xa0
[  666.319863]  __kmalloc_noprof+0x1e7/0x520
[  666.319867]  sock_kmalloc+0xdf/0x130
[  666.319885]  sock_kmemdup+0x1b/0x40
[  666.319888]  mptcp_userspace_pm_append_new_local_addr+0x261/0x500
[  666.319910]  mptcp_pm_nl_announce_doit+0x16a/0x610
...
[  666.319967] Freed by task 45560:
[  666.319988]  kasan_save_stack+0x33/0x60
[  666.319991]  kasan_save_track+0x14/0x30
[  666.319994]  kasan_save_free_info+0x3b/0x60
[  666.319998]  __kasan_slab_free+0x43/0x70
[  666.320000]  kfree+0x166/0x440
[  666.320003]  sock_kfree_s+0x1d/0x50
[  666.320007]  mptcp_userspace_pm_delete_local_addr.isra.0+0x157/0x200
[  666.320011]  mptcp_pm_nl_subflow_destroy_doit+0x51d/0xea0

 net/mptcp/pm_userspace.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index d100867e9202f..27fa8dc757b4f 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -132,12 +132,15 @@ int mptcp_userspace_pm_get_local_id(struct mptcp_sock *msk,
 	__be16 msk_sport =  ((struct inet_sock *)
 			     inet_sk((struct sock *)msk))->inet_sport;
 	struct mptcp_pm_addr_entry *entry;
+	int id = -1;
 
 	spin_lock_bh(&msk->pm.lock);
 	entry = mptcp_userspace_pm_lookup_addr(msk, &skc->addr);
-	spin_unlock_bh(&msk->pm.lock);
 	if (entry)
-		return entry->addr.id;
+		id = entry->addr.id;
+	spin_unlock_bh(&msk->pm.lock);
+	if (id >= 0)
+		return id;
 
 	if (skc->addr.port == msk_sport)
 		skc->addr.port = 0;
-- 
2.43.0