Skip to content

Commit 8667e8d

Browse files
mmhalgregkh
authored andcommitted
vsock: Fix transport_* TOCTOU
[ 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: c0cfa2d ("vsock: add multi-transports support") Reviewed-by: Stefano Garzarella <[email protected]> Signed-off-by: Michal Luczaj <[email protected]> Link: https://patch.msgid.link/[email protected] Signed-off-by: Jakub Kicinski <[email protected]> Signed-off-by: Sasha Levin <[email protected]>
1 parent 833da12 commit 8667e8d

File tree

1 file changed

+23
-5
lines changed

1 file changed

+23
-5
lines changed

net/vmw_vsock/af_vsock.c

Lines changed: 23 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -400,6 +400,8 @@ EXPORT_SYMBOL_GPL(vsock_enqueue_accept);
400400

401401
static bool vsock_use_local_transport(unsigned int remote_cid)
402402
{
403+
lockdep_assert_held(&vsock_register_mutex);
404+
403405
if (!transport_local)
404406
return false;
405407

@@ -457,6 +459,8 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
457459

458460
remote_flags = vsk->remote_addr.svm_flags;
459461

462+
mutex_lock(&vsock_register_mutex);
463+
460464
switch (sk->sk_type) {
461465
case SOCK_DGRAM:
462466
new_transport = transport_dgram;
@@ -471,12 +475,15 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
471475
new_transport = transport_h2g;
472476
break;
473477
default:
474-
return -ESOCKTNOSUPPORT;
478+
ret = -ESOCKTNOSUPPORT;
479+
goto err;
475480
}
476481

477482
if (vsk->transport) {
478-
if (vsk->transport == new_transport)
479-
return 0;
483+
if (vsk->transport == new_transport) {
484+
ret = 0;
485+
goto err;
486+
}
480487

481488
/* transport->release() must be called with sock lock acquired.
482489
* 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)
500507
/* We increase the module refcnt to prevent the transport unloading
501508
* while there are open sockets assigned to it.
502509
*/
503-
if (!new_transport || !try_module_get(new_transport->module))
504-
return -ENODEV;
510+
if (!new_transport || !try_module_get(new_transport->module)) {
511+
ret = -ENODEV;
512+
goto err;
513+
}
514+
515+
/* It's safe to release the mutex after a successful try_module_get().
516+
* Whichever transport `new_transport` points at, it won't go away until
517+
* the last module_put() below or in vsock_deassign_transport().
518+
*/
519+
mutex_unlock(&vsock_register_mutex);
505520

506521
ret = new_transport->init(vsk, psk);
507522
if (ret) {
@@ -512,6 +527,9 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
512527
vsk->transport = new_transport;
513528

514529
return 0;
530+
err:
531+
mutex_unlock(&vsock_register_mutex);
532+
return ret;
515533
}
516534
EXPORT_SYMBOL_GPL(vsock_assign_transport);
517535

0 commit comments

Comments
 (0)