]> git.ipfire.org Git - thirdparty/kernel/stable.git/commitdiff
vsock: Fix transport_* TOCTOU
authorMichal Luczaj <mhal@rbox.co>
Thu, 3 Jul 2025 15:18:19 +0000 (17:18 +0200)
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>
Thu, 17 Jul 2025 16:27:53 +0000 (18:27 +0200)
[ Upstream commit 687aa0c5581b8d4aa87fd92973e4ee576b550cdf ]

Transport assignment may race with module unload. Protect new_transport
from becoming a stale pointer.

This also takes care of an insecure call in vsock_use_local_transport();
add a lockdep assert.

BUG: unable to handle page fault for address: fffffbfff8056000
Oops: Oops: 0000 [#1] SMP KASAN
RIP: 0010:vsock_assign_transport+0x366/0x600
Call Trace:
 vsock_connect+0x59c/0xc40
 __sys_connect+0xe8/0x100
 __x64_sys_connect+0x6e/0xc0
 do_syscall_64+0x92/0x1c0
 entry_SYSCALL_64_after_hwframe+0x4b/0x53

Fixes: c0cfa2d8a788 ("vsock: add multi-transports support")
Reviewed-by: Stefano Garzarella <sgarzare@redhat.com>
Signed-off-by: Michal Luczaj <mhal@rbox.co>
Link: https://patch.msgid.link/20250703-vsock-transports-toctou-v4-2-98f0eb530747@rbox.co
Signed-off-by: Jakub Kicinski <kuba@kernel.org>
Signed-off-by: Sasha Levin <sashal@kernel.org>
net/vmw_vsock/af_vsock.c

index beacbe957594a96c9a2a1ded68cf501f09afea37..f65868d2e82c3402a6baf47e1f2f9115834896c3 100644 (file)
@@ -400,6 +400,8 @@ EXPORT_SYMBOL_GPL(vsock_enqueue_accept);
 
 static bool vsock_use_local_transport(unsigned int remote_cid)
 {
+       lockdep_assert_held(&vsock_register_mutex);
+
        if (!transport_local)
                return false;
 
@@ -457,6 +459,8 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
 
        remote_flags = vsk->remote_addr.svm_flags;
 
+       mutex_lock(&vsock_register_mutex);
+
        switch (sk->sk_type) {
        case SOCK_DGRAM:
                new_transport = transport_dgram;
@@ -471,12 +475,15 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
                        new_transport = transport_h2g;
                break;
        default:
-               return -ESOCKTNOSUPPORT;
+               ret = -ESOCKTNOSUPPORT;
+               goto err;
        }
 
        if (vsk->transport) {
-               if (vsk->transport == new_transport)
-                       return 0;
+               if (vsk->transport == new_transport) {
+                       ret = 0;
+                       goto err;
+               }
 
                /* transport->release() must be called with sock lock acquired.
                 * This path can only be taken during vsock_stream_connect(),
@@ -500,8 +507,16 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
        /* 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))
-               return -ENODEV;
+       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);
 
        ret = new_transport->init(vsk, psk);
        if (ret) {
@@ -512,6 +527,9 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
        vsk->transport = new_transport;
 
        return 0;
+err:
+       mutex_unlock(&vsock_register_mutex);
+       return ret;
 }
 EXPORT_SYMBOL_GPL(vsock_assign_transport);