[PATCH mptcp-net v3] mptcp: pm: userspace: unify entry free path via RCU callback

[PATCH mptcp-net v3] mptcp: pm: userspace: unify entry free path via RCU callback

[Geliang Tang]

From: Geliang Tang <tanggeliang@kylinos.cn>

In mptcp_pm_nl_remove_doit(), sk_omem_alloc is decremented immediately
but the memory is freed later via kfree_rcu(). This allows a CAP_NET_ADMIN
user to bypass the socket memory quota and exhaust kernel memory by
accumulating RCU callbacks.

Fix by using call_rcu() with a custom callback that uses sock_kfree_s()
to free the entry and decrement sk_omem_alloc atomically. To ensure the
socket remains valid until the callback runs, take a reference with
sock_hold() when storing the socket pointer in the entry, and release it
with sock_put() in the callback.

Convert the synchronous freeing paths in free_local_addr_list() and
delete_local_addr() to use the same RCU callback, ensuring the socket
reference is properly released.

Additionally, mptcp_userspace_pm_append_new_local_addr() now checks
SOCK_DEAD under the spinlock before allocating. A SYN+JOIN handler
holding an msk reference from mptcp_token_get_sock() could otherwise
race with __mptcp_destroy_sock() - sock_orphan() sets SOCK_DEAD and
then mptcp_userspace_pm_release() clears the list, so a new entry
allocated after that point would never be freed and its sock_hold()
would leak the msk permanently.

Fixes: 13b4ece33cf9 ("mptcp: pm: Defer freeing of MPTCP userspace path manager entries")
Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
---
v3:
 - checking sock_flag(sk, SOCK_DEAD)) before holding the reference.
 - update the subject.

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 | 33 ++++++++++++++++++++++++---------
 net/mptcp/protocol.h     |  2 ++
 2 files changed, 26 insertions(+), 9 deletions(-)

diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
index ad6ba658e5a5..c024c5cd5da1 100644
--- a/net/mptcp/pm_userspace.c
+++ b/net/mptcp/pm_userspace.c
@@ -12,10 +12,19 @@
 	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);
+}
+
 void mptcp_userspace_pm_free_local_addr_list(struct mptcp_sock *msk)
 {
 	struct mptcp_pm_addr_entry *entry, *tmp;
-	struct sock *sk = (struct sock *)msk;
 	LIST_HEAD(free_list);
 
 	spin_lock_bh(&msk->pm.lock);
@@ -23,7 +32,7 @@ void mptcp_userspace_pm_free_local_addr_list(struct mptcp_sock *msk)
 	spin_unlock_bh(&msk->pm.lock);
 
 	list_for_each_entry_safe(entry, tmp, &free_list, list) {
-		sock_kfree_s(sk, entry, sizeof(*entry));
+		call_rcu(&entry->rcu, mptcp_userspace_pm_free_entry);
 	}
 }
 
@@ -54,6 +63,15 @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
 	bitmap_zero(id_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);
 
 	spin_lock_bh(&msk->pm.lock);
+	/* sock_orphan() has been called and mptcp_userspace_pm_release()
+	 * has cleared userspace_pm_local_addr_list. Any entry we allocate
+	 * here would never be freed via the list, leaking the sock_hold().
+	 */
+	if (sock_flag(sk, SOCK_DEAD)) {
+		ret = -EINVAL;
+		goto append_err;
+	}
+
 	mptcp_for_each_userspace_pm_addr(msk, e) {
 		addr_match = mptcp_addresses_equal(&e->addr, &entry->addr, true);
 		if (addr_match && entry->addr.id == 0 && needs_id)
@@ -73,6 +91,8 @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
 			ret = -ENOMEM;
 			goto append_err;
 		}
+		sock_hold(sk);
+		e->sk = sk;
 
 		if (!e->addr.id && needs_id)
 			e->addr.id = find_next_zero_bit(id_bitmap,
@@ -98,7 +118,6 @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
 static int mptcp_userspace_pm_delete_local_addr(struct mptcp_sock *msk,
 						struct mptcp_pm_addr_entry *addr)
 {
-	struct sock *sk = (struct sock *)msk;
 	struct mptcp_pm_addr_entry *entry;
 
 	entry = mptcp_userspace_pm_lookup_addr(msk, &addr->addr);
@@ -109,7 +128,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));
+	call_rcu(&entry->rcu, mptcp_userspace_pm_free_entry);
 	msk->pm.local_addr_used--;
 	return 0;
 }
@@ -337,11 +356,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);
+	call_rcu(&match->rcu, mptcp_userspace_pm_free_entry);
 
 	err = 0;
 out:
diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
index da40c6f3705f..250736eae0be 100644
--- a/net/mptcp/protocol.h
+++ b/net/mptcp/protocol.h
@@ -257,6 +257,8 @@ struct mptcp_pm_addr_entry {
 	u32			flags;
 	int			ifindex;
 	struct socket		*lsk;
+	struct sock		*sk;
+	struct rcu_head		rcu;
 };
 
 struct mptcp_data_frag {
-- 
2.53.0

Re: [PATCH mptcp-net v3] mptcp: pm: userspace: unify entry free path via RCU callback

[MPTCP CI]

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

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


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)

Re: [PATCH mptcp-net v3] mptcp: pm: userspace: unify entry free path via RCU callback

[Matthieu Baerts]

Hi Geliang, Mat,

On 01/07/2026 08:11, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> In mptcp_pm_nl_remove_doit(), sk_omem_alloc is decremented immediately
> but the memory is freed later via kfree_rcu(). This allows a CAP_NET_ADMIN
> user to bypass the socket memory quota and exhaust kernel memory by
> accumulating RCU callbacks.
> 
> Fix by using call_rcu() with a custom callback that uses sock_kfree_s()
> to free the entry and decrement sk_omem_alloc atomically. To ensure the
> socket remains valid until the callback runs, take a reference with
> sock_hold() when storing the socket pointer in the entry, and release it
> with sock_put() in the callback.
> 
> Convert the synchronous freeing paths in free_local_addr_list() and
> delete_local_addr() to use the same RCU callback, ensuring the socket
> reference is properly released.
> 
> Additionally, mptcp_userspace_pm_append_new_local_addr() now checks
> SOCK_DEAD under the spinlock before allocating. A SYN+JOIN handler
> holding an msk reference from mptcp_token_get_sock() could otherwise
> race with __mptcp_destroy_sock() - sock_orphan() sets SOCK_DEAD and
> then mptcp_userspace_pm_release() clears the list, so a new entry
> allocated after that point would never be freed and its sock_hold()
> would leak the msk permanently.

Thank you for this patch, and your patience!

It looks OK to me, but TBH, I'm not confident enough to judge if this is
the best way to address this issue, plus if this issue mentioned by
Sashiko [1] can really be observed. By chance, do you have any
reproducer for the original issue?

@Mat: WDYT?

Also, after "mptcp: pm: userspace: properly handle the ID0 case" [2], I
guess we will need a sock_hold() in mptcp_pm_userspace_created as well.

[1]
https://sashiko.dev/#/patchset/cover.1782457962.git.tanggeliang@kylinos.cn
[2]
https://lore.kernel.org/20260807-mptcp-pm-userspace-id0-case-v3-1-de9088549924@kernel.org

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

Re: [PATCH mptcp-net v3] mptcp: pm: userspace: unify entry free path via RCU callback

[Mat Martineau]

On Wed, 1 Jul 2026, Geliang Tang wrote:

> From: Geliang Tang <tanggeliang@kylinos.cn>
>
> In mptcp_pm_nl_remove_doit(), sk_omem_alloc is decremented immediately
> but the memory is freed later via kfree_rcu(). This allows a CAP_NET_ADMIN
> user to bypass the socket memory quota and exhaust kernel memory by
> accumulating RCU callbacks.
>
> Fix by using call_rcu() with a custom callback that uses sock_kfree_s()
> to free the entry and decrement sk_omem_alloc atomically. To ensure the
> socket remains valid until the callback runs, take a reference with
> sock_hold() when storing the socket pointer in the entry, and release it
> with sock_put() in the callback.
>
> Convert the synchronous freeing paths in free_local_addr_list() and
> delete_local_addr() to use the same RCU callback, ensuring the socket
> reference is properly released.
>
> Additionally, mptcp_userspace_pm_append_new_local_addr() now checks
> SOCK_DEAD under the spinlock before allocating. A SYN+JOIN handler
> holding an msk reference from mptcp_token_get_sock() could otherwise
> race with __mptcp_destroy_sock() - sock_orphan() sets SOCK_DEAD and
> then mptcp_userspace_pm_release() clears the list, so a new entry
> allocated after that point would never be freed and its sock_hold()
> would leak the msk permanently.
>
> Fixes: 13b4ece33cf9 ("mptcp: pm: Defer freeing of MPTCP userspace path manager entries")
> Signed-off-by: Geliang Tang <tanggeliang@kylinos.cn>
> ---
> v3:
> - checking sock_flag(sk, SOCK_DEAD)) before holding the reference.
> - update the subject.
>
> 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 | 33 ++++++++++++++++++++++++---------
> net/mptcp/protocol.h     |  2 ++
> 2 files changed, 26 insertions(+), 9 deletions(-)
>
> diff --git a/net/mptcp/pm_userspace.c b/net/mptcp/pm_userspace.c
> index ad6ba658e5a5..c024c5cd5da1 100644
> --- a/net/mptcp/pm_userspace.c
> +++ b/net/mptcp/pm_userspace.c
> @@ -12,10 +12,19 @@
> 	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);
> +}
> +
> void mptcp_userspace_pm_free_local_addr_list(struct mptcp_sock *msk)
> {
> 	struct mptcp_pm_addr_entry *entry, *tmp;
> -	struct sock *sk = (struct sock *)msk;
> 	LIST_HEAD(free_list);
>
> 	spin_lock_bh(&msk->pm.lock);
> @@ -23,7 +32,7 @@ void mptcp_userspace_pm_free_local_addr_list(struct mptcp_sock *msk)
> 	spin_unlock_bh(&msk->pm.lock);
>
> 	list_for_each_entry_safe(entry, tmp, &free_list, list) {
> -		sock_kfree_s(sk, entry, sizeof(*entry));
> +		call_rcu(&entry->rcu, mptcp_userspace_pm_free_entry);
> 	}
> }

Hi Geliang -

The only code path that leads here is when the msk is being destroyed. It 
makes more sense to keep the existing synchronous code here.

>
> @@ -54,6 +63,15 @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
> 	bitmap_zero(id_bitmap, MPTCP_PM_MAX_ADDR_ID + 1);
>
> 	spin_lock_bh(&msk->pm.lock);
> +	/* sock_orphan() has been called and mptcp_userspace_pm_release()
> +	 * has cleared userspace_pm_local_addr_list. Any entry we allocate
> +	 * here would never be freed via the list, leaking the sock_hold().
> +	 */
> +	if (sock_flag(sk, SOCK_DEAD)) {
> +		ret = -EINVAL;
> +		goto append_err;
> +	}
> +

This can be checked before the spinlock and bitmap_zero(), which allows a 
direct return instead of using the goto.

> 	mptcp_for_each_userspace_pm_addr(msk, e) {
> 		addr_match = mptcp_addresses_equal(&e->addr, &entry->addr, true);
> 		if (addr_match && entry->addr.id == 0 && needs_id)
> @@ -73,6 +91,8 @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
> 			ret = -ENOMEM;
> 			goto append_err;
> 		}
> +		sock_hold(sk);
> +		e->sk = sk;

Better to set these immediately before using call_rcu(), it's not obvious 
where the matching sock_put() is. If taking this approach, a helper 
function could set these and then invoke call_rcu().

>
> 		if (!e->addr.id && needs_id)
> 			e->addr.id = find_next_zero_bit(id_bitmap,
> @@ -98,7 +118,6 @@ static int mptcp_userspace_pm_append_new_local_addr(struct mptcp_sock *msk,
> static int mptcp_userspace_pm_delete_local_addr(struct mptcp_sock *msk,
> 						struct mptcp_pm_addr_entry *addr)
> {
> -	struct sock *sk = (struct sock *)msk;
> 	struct mptcp_pm_addr_entry *entry;
>
> 	entry = mptcp_userspace_pm_lookup_addr(msk, &addr->addr);
> @@ -109,7 +128,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));
> +	call_rcu(&entry->rcu, mptcp_userspace_pm_free_entry);
> 	msk->pm.local_addr_used--;
> 	return 0;
> }
> @@ -337,11 +356,7 @@ int mptcp_pm_nl_remove_doit(struct sk_buff *skb, struct genl_info *info)
>
> 	release_sock(sk);
>
> -	kfree_rcu_mightsleep(match);

The function name here reminded me that this is a sleepable context. 
Instead of adding all the new async code and increasing the size of 
mptcp_pm_addr_entry, another option is to insert a synchronize_rcu() here. 
However, that would delay completion of this netlink call and block other 
userspace PM operations for the RCU grace period, so maybe the async 
technique is worth it.

- Mat

> -	/* 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);
> +	call_rcu(&match->rcu, mptcp_userspace_pm_free_entry);
>
> 	err = 0;
> out:
> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index da40c6f3705f..250736eae0be 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -257,6 +257,8 @@ struct mptcp_pm_addr_entry {
> 	u32			flags;
> 	int			ifindex;
> 	struct socket		*lsk;
> +	struct sock		*sk;
> +	struct rcu_head		rcu;
> };
>
> struct mptcp_data_frag {
> -- 
> 2.53.0
>
>
>

Re: [PATCH mptcp-net v3] mptcp: pm: userspace: unify entry free path via RCU callback

[Matthieu Baerts]

Hi Geliang,

On 01/07/2026 08:11, Geliang Tang wrote:
> From: Geliang Tang <tanggeliang@kylinos.cn>
> 
> In mptcp_pm_nl_remove_doit(), sk_omem_alloc is decremented immediately
> but the memory is freed later via kfree_rcu(). This allows a CAP_NET_ADMIN
> user to bypass the socket memory quota and exhaust kernel memory by
> accumulating RCU callbacks.
> 
> Fix by using call_rcu() with a custom callback that uses sock_kfree_s()
> to free the entry and decrement sk_omem_alloc atomically. To ensure the
> socket remains valid until the callback runs, take a reference with
> sock_hold() when storing the socket pointer in the entry, and release it
> with sock_put() in the callback.
> 
> Convert the synchronous freeing paths in free_local_addr_list() and
> delete_local_addr() to use the same RCU callback, ensuring the socket
> reference is properly released.
> 
> Additionally, mptcp_userspace_pm_append_new_local_addr() now checks
> SOCK_DEAD under the spinlock before allocating. A SYN+JOIN handler
> holding an msk reference from mptcp_token_get_sock() could otherwise
> race with __mptcp_destroy_sock() - sock_orphan() sets SOCK_DEAD and
> then mptcp_userspace_pm_release() clears the list, so a new entry
> allocated after that point would never be freed and its sock_hold()
> would leak the msk permanently.

(...)

> diff --git a/net/mptcp/protocol.h b/net/mptcp/protocol.h
> index da40c6f3705f..250736eae0be 100644
> --- a/net/mptcp/protocol.h
> +++ b/net/mptcp/protocol.h
> @@ -257,6 +257,8 @@ struct mptcp_pm_addr_entry {
>  	u32			flags;
>  	int			ifindex;
>  	struct socket		*lsk;
> +	struct sock		*sk;

Also, I don't think 'lsk' is used with the userspace PM: can we re-use
it instead of adding a new one? (Or using an anonymous union?)

> +	struct rcu_head		rcu;
>  };
>  
>  struct mptcp_data_frag {

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