Index | Thread | Search

From:
Dave Voutila <dv@sisu.io>
Subject:
vmd(8): fix race condition with vcpu pause barrier
To:
tech@openbsd.org
Date:
Thu, 27 Nov 2025 13:18:04 -0500

Download raw body.

Thread
The vm pause function currently allocates and frees a pthread barrier
object at pause time, probably because at one point the idea the # of
vcpus could be dynamic.

While we can save a few bytes doing it this way if the vm is never
paused, it's far simpler to initialize and destroy this barrier as part
of the greater vm lifecycle before vcpus are launched and after they are
destroyed.

If you want to trigger this race condition easily, insert a `sleep(1)`
or the like in pause_vm(). Boot a ramdisk kernel guest and at the shell
prompt run something cpu heavy like `while true; do date; done`. Then
use vmctl(8) to try pausing the vm. The vcpu thread will try to
pthread_barrier_wait on an uninitialized barrier object and fail. Your
vm is now borked. :D

ok?

-dv


diffstat refs/heads/master refs/heads/vmd-pause-barrier
 M  usr.sbin/vmd/vm.c  |  11+  16-

1 file changed, 11 insertions(+), 16 deletions(-)

diff refs/heads/master refs/heads/vmd-pause-barrier
commit - 076e7c3b0745cd2dca97eb8708c522b384d17250
commit + b2f4db7d6f1ac0311376b308e3583696da731802
blob - 0b4f7ae3d7e42e451453e9c910de7d8fbcf137ff
blob + 8fe9a134d16644cfe2a403c71cd76e7321ac7fef
--- usr.sbin/vmd/vm.c
+++ usr.sbin/vmd/vm.c
@@ -442,14 +442,6 @@ pause_vm(struct vmd_vm *vm)
 	current_vm->vm_state |= VM_STATE_PAUSED;
 	mutex_unlock(&vm_mtx);

-	ret = pthread_barrier_init(&vm_pause_barrier, NULL,
-	    vm->vm_params.vmc_params.vcp_ncpus + 1);
-	if (ret) {
-		log_warnx("%s: cannot initialize pause barrier (%d)",
-		    __progname, ret);
-		return;
-	}
-
 	for (n = 0; n < vm->vm_params.vmc_params.vcp_ncpus; n++) {
 		ret = pthread_cond_broadcast(&vcpu_run_cond[n]);
 		if (ret) {
@@ -465,13 +457,6 @@ pause_vm(struct vmd_vm *vm)
 		return;
 	}

-	ret = pthread_barrier_destroy(&vm_pause_barrier);
-	if (ret) {
-		log_warnx("%s: could not destroy pause barrier (%d)",
-		    __progname, ret);
-		return;
-	}
-
 	pause_vm_md(vm);
 }

@@ -623,6 +608,12 @@ run_vm(struct vmop_create_params *vmc, struct vcpu_reg
 		return (ENOMEM);
 	}

+	ret = pthread_barrier_init(&vm_pause_barrier, NULL, vcp->vcp_ncpus + 1);
+	if (ret) {
+		log_warnx("cannot initialize pause barrier (%d)", ret);
+		return (ret);
+	}
+
 	log_debug("%s: starting %zu vcpu thread(s) for vm %s", __func__,
 	    vcp->vcp_ncpus, vcp->vcp_name);

@@ -781,11 +772,15 @@ run_vm(struct vmop_create_params *vmc, struct vcpu_reg
 		}
 		mutex_unlock(&vm_mtx);
 		if (i == vcp->vcp_ncpus)
-			return (ret);
+			break;

 		/* Some more threads to wait for, start over */
 	}

+	ret = pthread_barrier_destroy(&vm_pause_barrier);
+	if (ret)
+		log_warnx("could not destroy pause barrier (%d)", ret);
+
 	return (ret);
 }