Skip to content

Commit 209fd72

Browse files
mmhalkuba-moo
authored andcommitted
vsock: Fix transport_{g2h,h2g} TOCTOU
vsock_find_cid() and vsock_dev_do_ioctl() may race with module unload. transport_{g2h,h2g} may become NULL after the NULL check. Introduce vsock_transport_local_cid() to protect from a potential null-ptr-deref. KASAN: null-ptr-deref in range [0x0000000000000118-0x000000000000011f] RIP: 0010:vsock_find_cid+0x47/0x90 Call Trace: __vsock_bind+0x4b2/0x720 vsock_bind+0x90/0xe0 __sys_bind+0x14d/0x1e0 __x64_sys_bind+0x6e/0xc0 do_syscall_64+0x92/0x1c0 entry_SYSCALL_64_after_hwframe+0x4b/0x53 KASAN: null-ptr-deref in range [0x0000000000000118-0x000000000000011f] RIP: 0010:vsock_dev_do_ioctl.isra.0+0x58/0xf0 Call Trace: __x64_sys_ioctl+0x12d/0x190 do_syscall_64+0x92/0x1c0 entry_SYSCALL_64_after_hwframe+0x4b/0x53 Fixes: c0cfa2d ("vsock: add multi-transports support") Suggested-by: Stefano Garzarella <[email protected]> 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]>
1 parent 95a234f commit 209fd72

File tree

1 file changed

+21
-6
lines changed

1 file changed

+21
-6
lines changed

net/vmw_vsock/af_vsock.c

Lines changed: 21 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -531,9 +531,25 @@ int vsock_assign_transport(struct vsock_sock *vsk, struct vsock_sock *psk)
531531
}
532532
EXPORT_SYMBOL_GPL(vsock_assign_transport);
533533

534+
/*
535+
* Provide safe access to static transport_{h2g,g2h,dgram,local} callbacks.
536+
* Otherwise we may race with module removal. Do not use on `vsk->transport`.
537+
*/
538+
static u32 vsock_registered_transport_cid(const struct vsock_transport **transport)
539+
{
540+
u32 cid = VMADDR_CID_ANY;
541+
542+
mutex_lock(&vsock_register_mutex);
543+
if (*transport)
544+
cid = (*transport)->get_local_cid();
545+
mutex_unlock(&vsock_register_mutex);
546+
547+
return cid;
548+
}
549+
534550
bool vsock_find_cid(unsigned int cid)
535551
{
536-
if (transport_g2h && cid == transport_g2h->get_local_cid())
552+
if (cid == vsock_registered_transport_cid(&transport_g2h))
537553
return true;
538554

539555
if (transport_h2g && cid == VMADDR_CID_HOST)
@@ -2536,18 +2552,17 @@ static long vsock_dev_do_ioctl(struct file *filp,
25362552
unsigned int cmd, void __user *ptr)
25372553
{
25382554
u32 __user *p = ptr;
2539-
u32 cid = VMADDR_CID_ANY;
25402555
int retval = 0;
2556+
u32 cid;
25412557

25422558
switch (cmd) {
25432559
case IOCTL_VM_SOCKETS_GET_LOCAL_CID:
25442560
/* To be compatible with the VMCI behavior, we prioritize the
25452561
* guest CID instead of well-know host CID (VMADDR_CID_HOST).
25462562
*/
2547-
if (transport_g2h)
2548-
cid = transport_g2h->get_local_cid();
2549-
else if (transport_h2g)
2550-
cid = transport_h2g->get_local_cid();
2563+
cid = vsock_registered_transport_cid(&transport_g2h);
2564+
if (cid == VMADDR_CID_ANY)
2565+
cid = vsock_registered_transport_cid(&transport_h2g);
25512566

25522567
if (put_user(cid, p) != 0)
25532568
retval = -EFAULT;

0 commit comments

Comments
 (0)