Skip to content

Commit

Permalink
vsock: Fix transport_* TOCTOU
Browse files Browse the repository at this point in the history
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: c0cfa2d ("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>
  • Loading branch information
Michal Luczaj authored and Jakub Kicinski committed Jul 8, 2025
1 parent 209fd72 commit 687aa0c
Showing 1 changed file with 23 additions and 5 deletions.
28 changes: 23 additions & 5 deletions net/vmw_vsock/af_vsock.c
Original file line number Diff line number Diff line change
Expand Up @@ -407,6 +407,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;

Expand Down Expand Up @@ -464,6 +466,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;
Expand All @@ -479,12 +483,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_connect(), where we
Expand All @@ -508,8 +515,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);

if (sk->sk_type == SOCK_SEQPACKET) {
if (!new_transport->seqpacket_allow ||
Expand All @@ -528,6 +543,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);

Expand Down

0 comments on commit 687aa0c

Please sign in to comment.