Commit 40c47232 authored by Jakub Kicinski's avatar Jakub Kicinski
Browse files

Merge branch 'vsock-fix-transport_-h2g-g2h-dgram-local-toctou-issues' 

Michal Luczaj says:

====================
vsock: Fix transport_{h2g,g2h,dgram,local} TOCTOU issues

transport_{h2g,g2h,dgram,local} may become NULL on vsock_core_unregister().
Make sure a poorly timed `rmmod transport` won't lead to a NULL/stale
pointer dereference.

Note that these oopses are pretty unlikely to happen in the wild. Splats
were collected after sprinkling kernel with mdelay()s.

v3: https://lore.kernel.org/20250702-vsock-transports-toctou-v3-0-0a7e2e692987@rbox.co
v2: https://lore.kernel.org/20250620-vsock-transports-toctou-v2-0-02ebd20b1d03@rbox.co
v1: https://lore.kernel.org/20250618-vsock-transports-toctou-v1-0-dd2d2ede9052@rbox.co
====================

Link: https://patch.msgid.link/20250703-vsock-transports-toctou-v4-0-98f0eb530747@rbox.co


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parents 95a234f6 1e7d9df3

net/vmw_vsock/af_vsock.c

+46 −11
Original line number Diff line number Diff line
@@ -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;
@@ -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;
@@ -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
@@ -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 ||
@@ -528,12 +543,31 @@ 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);



/*

 * Provide safe access to static transport_{h2g,g2h,dgram,local} callbacks.

 * Otherwise we may race with module removal. Do not use on `vsk->transport`.

 */

static u32 vsock_registered_transport_cid(const struct vsock_transport **transport)

{

	u32 cid = VMADDR_CID_ANY;



	mutex_lock(&vsock_register_mutex);

	if (*transport)

		cid = (*transport)->get_local_cid();

	mutex_unlock(&vsock_register_mutex);



	return cid;

}



bool vsock_find_cid(unsigned int cid)

{

	if (transport_g2h && cid == transport_g2h->get_local_cid())

	if (cid == vsock_registered_transport_cid(&transport_g2h))

		return true;



	if (transport_h2g && cid == VMADDR_CID_HOST)
@@ -2536,18 +2570,19 @@ static long vsock_dev_do_ioctl(struct file *filp, 
			       unsigned int cmd, void __user *ptr)

{

	u32 __user *p = ptr;

	u32 cid = VMADDR_CID_ANY;

	int retval = 0;

	u32 cid;



	switch (cmd) {

	case IOCTL_VM_SOCKETS_GET_LOCAL_CID:

		/* To be compatible with the VMCI behavior, we prioritize the

		 * guest CID instead of well-know host CID (VMADDR_CID_HOST).

		 */

		if (transport_g2h)

			cid = transport_g2h->get_local_cid();

		else if (transport_h2g)

			cid = transport_h2g->get_local_cid();

		cid = vsock_registered_transport_cid(&transport_g2h);

		if (cid == VMADDR_CID_ANY)

			cid = vsock_registered_transport_cid(&transport_h2g);

		if (cid == VMADDR_CID_ANY)

			cid = vsock_registered_transport_cid(&transport_local);



		if (put_user(cid, p) != 0)

			retval = -EFAULT;