[PATCH net] mptcp: upgrade network refcount before socket lock

[PATCH net] mptcp: upgrade network refcount before socket lock

[Runyu Xiao]

sk_net_refcnt_upgrade() calls get_net_track() with GFP_KERNEL and can enter
direct reclaim. Calling it while holding the newly created subflow socket
lock can create a reclaim-to-socket-lock dependency cycle.

Upgrade the network reference before taking the socket lock. The socket is
newly created and has not been exposed to other code at this point, so the
fields changed by sk_net_refcnt_upgrade() are not accessed concurrently.
The error path still releases the socket normally after the upgrade.

The PatchProof static-analysis tool detected a GFP_KERNEL allocation while
the socket lock is held. Manual source review of v7.1.5 and current
mainline confirmed the lock and allocation ordering.

A source-level check found `sk_net_refcnt_upgrade()` after
`lock_sock_nested()` in the original function and before it after this
change. A POSIX-thread lock-order model made the reclaim lock unavailable
while the socket lock was held, observed `EBUSY` for the reclaim lock, and
then completed with the reclaim-first order. The model checks the ordering
invariant only; it does not execute the kernel MPTCP path. No live lockdep
MPTCP test or reclaim fault injection was run.

Fixes: 1d2f3d3c6268 ("mptcp: adjust to use netns refcount tracker")
Cc: stable@vger.kernel.org
Signed-off-by: Runyu Xiao <runyu.xiao@seu.edu.cn>
---
 net/mptcp/subflow.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/net/mptcp/subflow.c b/net/mptcp/subflow.c
index e1f20ff8fdb4..a9f951cc6a0e 100644
--- a/net/mptcp/subflow.c
+++ b/net/mptcp/subflow.c
@@ -1786,6 +1786,12 @@ int mptcp_subflow_create_socket(struct sock *sk, unsigned short family,
 	if (err)
 		return err;
 
+	/* kernel sockets do not by default acquire net ref, but TCP timer
+	 * needs it.
+	 * Update ns_tracker to current stack trace and refcounted tracker.
+	 */
+	sk_net_refcnt_upgrade(sf->sk);
+
 	lock_sock_nested(sf->sk, SINGLE_DEPTH_NESTING);
 
 	err = security_mptcp_add_subflow(sk, sf->sk);
@@ -1795,11 +1801,6 @@ int mptcp_subflow_create_socket(struct sock *sk, unsigned short family,
 	/* the newly created socket has to be in the same cgroup as its parent */
 	mptcp_attach_cgroup(sk, sf->sk);
 
-	/* kernel sockets do not by default acquire net ref, but TCP timer
-	 * needs it.
-	 * Update ns_tracker to current stack trace and refcounted tracker.
-	 */
-	sk_net_refcnt_upgrade(sf->sk);
 	err = tcp_set_ulp(sf->sk, "mptcp");
 	if (err)
 		goto err_free;
-- 
2.34.1

Re: [PATCH net] mptcp: upgrade network refcount before socket lock

[MPTCP CI]

Hi Runyu,

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/31306413386

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


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)