From: Geliang Tang <tanggeliang@kylinos.cn>

In mptcp_pm_nl_remove_doit(), sk_omem_alloc is decremented immediately
but the entry memory is freed later via kfree_rcu_mightsleep(). A
CAP_NET_ADMIN user can bypass the socket memory quota and exhaust kernel
memory by accumulating pending RCU callbacks.

Fix by deferring both the memory free and the sk_omem_alloc adjustment to
an RCU callback via sock_kfree_s().

A new mptcp_userspace_pm_release_entry() helper takes a sock reference
with sock_hold() before scheduling the callback, and the callback releases
it with sock_put() after freeing.

Apply the same helper to mptcp_userspace_pm_delete_local_addr() which has
the same issue. The synchronous freeing in free_local_addr_list() is left
unchanged: it only runs during msk destruction where no concurrent readers
exist.

While at it, reuse the lsk field of mptcp_pm_addr_entry via an anonymous
union for the sock backpointer - lsk is only used by the kernel PM, sk by
the userspace PM, so they are mutually exclusive.

Fixes: 13b4ece33cf9 ("mptcp: pm: Defer freeing of MPTCP userspace path manager entries")
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
v4:
 - address Mat & Matt's review comments on v3, thanks!
 - move sock_hold() and entry->sk assignment from allocation time to a new
   mptcp_userspace_pm_release_entry() helper at free time
 - keep free_local_addr_list() synchronous, no RCU needed there as it only
   runs during msk destruction
 - drop the SOCK_DEAD check in append_new_local_addr(), the leak chain is
   broken by the above sock_hold() relocation
 - use an anonymous union for lsk/sk instead of adding a new field

v3:
 - checking sock_flag(sk, SOCK_DEAD)) before holding the reference.
 - update the subject.
 - Link: https://patchwork.kernel.org/project/mptcp/patch/de8555fcb235d0e93c02c20bf47231846d30841d.1782886142.git.tanggeliang@kylinos.cn/

v2:
 - call mptcp_userspace_pm_free_entry in free_local_addr_list and
   delete_local_addr.
 - Link: https://patchwork.kernel.org/project/mptcp/patch/df199842d10185a73084c79aee9cdc91888adb6a.1782799160.git.tanggeliang@kylinos.cn/

v1:
 - Link: https://patchwork.kernel.org/project/mptcp/patch/9b443bafa57f40a51eb6a43f088ff37d71b39973.1782528088.git.tanggeliang@kylinos.cn/

This patch addresses the pre-existing issue Sashiko mentioned in
https://sashiko.dev/#/patchset/cover.1782457962.git.tanggeliang@kylinos.cn.
---
 net/mptcp/pm_userspace.c | 26 ++++++++++++++++++++------
 net/mptcp/protocol.h     |  6 +++++-
 2 files changed, 25 insertions(+), 7 deletions(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index f723a134356f..09c500c59eed 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -12,6 +12,24 @@
 	list_for_each_entry(__entry,						\
 			    &((__msk)->pm.userspace_pm_local_addr_list), list)
 
+static void mptcp_userspace_pm_free_entry(struct rcu_head *head)
+{
+	struct mptcp_pm_addr_entry *entry =
+		container_of(head, struct mptcp_pm_addr_entry, rcu);
+	struct sock *sk = entry->sk;
+
+	sock_kfree_s(sk, entry, sizeof(*entry));
+	sock_put(sk);
+}
+
+static void mptcp_userspace_pm_release_entry(struct mptcp_pm_addr_entry *entry,
+					     struct sock *sk)
+{
+	entry->sk = sk;
+	sock_hold(sk);
+	call_rcu(&entry->rcu, mptcp_userspace_pm_free_entry);
+}
+
 void mptcp_userspace_pm_free_local_addr_list(struct mptcp_sock *msk)
 {
 	struct mptcp_pm_addr_entry *entry, *tmp;
@@ -123,7 +141,7 @@ static int mptcp_userspace_pm_delete_local_addr(struct mptcp_sock *msk,
 	 * be used multiple times (e.g. fullmesh mode).
 	 */
 	list_del_rcu(&entry->list);
-	sock_kfree_s(sk, entry, sizeof(*entry));
+	mptcp_userspace_pm_release_entry(entry, sk);
 	msk->pm.local_addr_used--;
 	return 0;
 }
@@ -355,11 +373,7 @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
 
 	release_sock(sk);
 
-	kfree_rcu_mightsleep(match);
-	/* Adjust sk_omem_alloc like sock_kfree_s() does, to match
-	 * with allocation of this memory by sock_kmemdup()
-	 */
-	atomic_sub(sizeof(*match), &sk->sk_omem_alloc);
+	mptcp_userspace_pm_release_entry(match, sk);
 
 	err = 0;
 out:
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index 4bf04f9ecbd9..0724b90a4904 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -273,7 +273,11 @@ struct mptcp_pm_addr_entry {
 	struct mptcp_addr_info	addr;
 	u32			flags;
 	int			ifindex;
-	struct socket		*lsk;
+	union {
+		struct socket	*lsk;	/* kernel PM: listener socket */
+		struct sock	*sk;	/* userspace PM: owning msk */
+	};
+	struct rcu_head		rcu;
 };
 
 struct mptcp_data_frag {
-- 
2.53.0

Hi Geliang,

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! ✅
- Perf: Success! ✅
- Task: https://github.com/multipath-tcp/mptcp_net-next/actions/runs/35587560715

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


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)