netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH net] vsock: fix lock inversion in vsock_assign_transport()
@ 2025-10-21 12:17 Stefano Garzarella
  2025-10-23 14:10 ` patchwork-bot+netdevbpf
  0 siblings, 1 reply; 2+ messages in thread
From: Stefano Garzarella @ 2025-10-21 12:17 UTC (permalink / raw)
  To: netdev
  Cc: Simon Horman, Paolo Abeni, linux-kernel, Stefano Garzarella,
	Jakub Kicinski, virtualization, Michal Luczaj, Eric Dumazet,
	David S. Miller, syzbot+10e35716f8e4929681fa, stable

From: Stefano Garzarella <sgarzare@redhat.com>

Syzbot reported a potential lock inversion deadlock between
vsock_register_mutex and sk_lock-AF_VSOCK when vsock_linger() is called.

The issue was introduced by commit 687aa0c5581b ("vsock: Fix
transport_* TOCTOU") which added vsock_register_mutex locking in
vsock_assign_transport() around the transport->release() call, that can
call vsock_linger(). vsock_assign_transport() can be called with sk_lock
held. vsock_linger() calls sk_wait_event() that temporarily releases and
re-acquires sk_lock. During this window, if another thread hold
vsock_register_mutex while trying to acquire sk_lock, a circular
dependency is created.

Fix this by releasing vsock_register_mutex before calling
transport->release() and vsock_deassign_transport(). This is safe
because we don't need to hold vsock_register_mutex while releasing the
old transport, and we ensure the new transport won't disappear by
obtaining a module reference first via try_module_get().

Reported-by: syzbot+10e35716f8e4929681fa@syzkaller.appspotmail.com
Tested-by: syzbot+10e35716f8e4929681fa@syzkaller.appspotmail.com
Fixes: 687aa0c5581b ("vsock: Fix transport_* TOCTOU")
Cc: mhal@rbox.co
Cc: stable@vger.kernel.org
Signed-off-by: Stefano Garzarella <sgarzare@redhat.com>
---
 net/vmw_vsock/af_vsock.c | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

diff --git a/net/vmw_vsock/af_vsock.c b/net/vmw_vsock/af_vsock.c
index 4c2db6cca557..76763247a377 100644
--- a/net/vmw_vsock/af_vsock.c
+++ b/net/vmw_vsock/af_vsock.c
@@ -487,12 +487,26 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
 		goto err;
 	}
 
-	if (vsk->transport) {
-		if (vsk->transport == new_transport) {
-			ret = 0;
-			goto err;
-		}
+	if (vsk->transport && vsk->transport == new_transport) {
+		ret = 0;
+		goto err;
+	}
 
+	/* We increase the module refcnt to prevent the transport unloading
+	 * while there are open sockets assigned to it.
+	 */
+	if (!new_transport || !try_module_get(new_transport->module)) {
+		ret = -ENODEV;
+		goto err;
+	}
+
+	/* It's safe to release the mutex after a successful try_module_get().
+	 * Whichever transport `new_transport` points at, it won't go away until
+	 * the last module_put() below or in vsock_deassign_transport().
+	 */
+	mutex_unlock(&vsock_register_mutex);
+
+	if (vsk->transport) {
 		/* transport->release() must be called with sock lock acquired.
 		 * This path can only be taken during vsock_connect(), where we
 		 * have already held the sock lock. In the other cases, this
@@ -512,20 +526,6 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
 		vsk->peer_shutdown = 0;
 	}
 
-	/* We increase the module refcnt to prevent the transport unloading
-	 * while there are open sockets assigned to it.
-	 */
-	if (!new_transport || !try_module_get(new_transport->module)) {
-		ret = -ENODEV;
-		goto err;
-	}
-
-	/* It's safe to release the mutex after a successful try_module_get().
-	 * Whichever transport `new_transport` points at, it won't go away until
-	 * the last module_put() below or in vsock_deassign_transport().
-	 */
-	mutex_unlock(&vsock_register_mutex);
-
 	if (sk->sk_type == SOCK_SEQPACKET) {
 		if (!new_transport->seqpacket_allow ||
 		    !new_transport->seqpacket_allow(remote_cid)) {
-- 
2.51.0


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [PATCH net] vsock: fix lock inversion in vsock_assign_transport()
  2025-10-21 12:17 [PATCH net] vsock: fix lock inversion in vsock_assign_transport() Stefano Garzarella
@ 2025-10-23 14:10 ` patchwork-bot+netdevbpf
  0 siblings, 0 replies; 2+ messages in thread
From: patchwork-bot+netdevbpf @ 2025-10-23 14:10 UTC (permalink / raw)
  To: Stefano Garzarella
  Cc: netdev, horms, pabeni, linux-kernel, kuba, virtualization, mhal,
	edumazet, davem, syzbot+10e35716f8e4929681fa, stable

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Tue, 21 Oct 2025 14:17:18 +0200 you wrote:
> From: Stefano Garzarella <sgarzare@redhat.com>
> 
> Syzbot reported a potential lock inversion deadlock between
> vsock_register_mutex and sk_lock-AF_VSOCK when vsock_linger() is called.
> 
> The issue was introduced by commit 687aa0c5581b ("vsock: Fix
> transport_* TOCTOU") which added vsock_register_mutex locking in
> vsock_assign_transport() around the transport->release() call, that can
> call vsock_linger(). vsock_assign_transport() can be called with sk_lock
> held. vsock_linger() calls sk_wait_event() that temporarily releases and
> re-acquires sk_lock. During this window, if another thread hold
> vsock_register_mutex while trying to acquire sk_lock, a circular
> dependency is created.
> 
> [...]

Here is the summary with links:
  - [net] vsock: fix lock inversion in vsock_assign_transport()
    https://git.kernel.org/netdev/net/c/f7c877e75352

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html



^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2025-10-23 14:10 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-21 12:17 [PATCH net] vsock: fix lock inversion in vsock_assign_transport() Stefano Garzarella
2025-10-23 14:10 ` patchwork-bot+netdevbpf

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).