Skip to content

Commit 39162f0

Browse files
mmhalNipaLocal
authored andcommitted
vsock: Fix transport_* TOCTOU
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") Signed-off-by: Michal Luczaj <[email protected]> Signed-off-by: NipaLocal <nipa@local>
1 parent 7f7bf53 commit 39162f0

File tree

1 file changed

+19
-5
lines changed

1 file changed

+19
-5
lines changed

net/vmw_vsock/af_vsock.c

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -407,6 +407,8 @@ EXPORT_SYMBOL_GPL(vsock_enqueue_accept);
407407

408408
static bool vsock_use_local_transport(unsigned int remote_cid)
409409
{
410+
lockdep_assert_held(&vsock_register_mutex);
411+
410412
if (!transport_local)
411413
return false;
412414

@@ -464,6 +466,8 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
464466

465467
remote_flags = vsk->remote_addr.svm_flags;
466468

469+
mutex_lock(&vsock_register_mutex);
470+
467471
switch (sk->sk_type) {
468472
case SOCK_DGRAM:
469473
new_transport = transport_dgram;
@@ -479,12 +483,15 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
479483
new_transport = transport_h2g;
480484
break;
481485
default:
482-
return -ESOCKTNOSUPPORT;
486+
ret = -ESOCKTNOSUPPORT;
487+
goto unlock;
483488
}
484489

485490
if (vsk->transport) {
486-
if (vsk->transport == new_transport)
487-
return 0;
491+
if (vsk->transport == new_transport) {
492+
ret = 0;
493+
goto unlock;
494+
}
488495

489496
/* transport->release() must be called with sock lock acquired.
490497
* This path can only be taken during vsock_connect(), where we
@@ -508,8 +515,12 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
508515
/* We increase the module refcnt to prevent the transport unloading
509516
* while there are open sockets assigned to it.
510517
*/
511-
if (!new_transport || !try_module_get(new_transport->module))
512-
return -ENODEV;
518+
if (!new_transport || !try_module_get(new_transport->module)) {
519+
ret = -ENODEV;
520+
goto unlock;
521+
}
522+
523+
mutex_unlock(&vsock_register_mutex);
513524

514525
if (sk->sk_type == SOCK_SEQPACKET) {
515526
if (!new_transport->seqpacket_allow ||
@@ -528,6 +539,9 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
528539
vsk->transport = new_transport;
529540

530541
return 0;
542+
unlock:
543+
mutex_unlock(&vsock_register_mutex);
544+
return ret;
531545
}
532546
EXPORT_SYMBOL_GPL(vsock_assign_transport);
533547

0 commit comments

Comments
 (0)