From: dv <dv@openbsd.org>
Date: Fri, 12 May 2023 14:42:30 +0000 (+0000)
Subject: vmd(8): fix console attach from vmctl(8).
X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=4ff80aa38f7f2bdfc34fd1c72ee66e90ce19d4ad;p=openbsd

vmd(8): fix console attach from vmctl(8).

Adding in the ability to override the boot kernel created an edge
case in the ipc message handling logic for the parent process (vmd)
when receiving a "start vm" request. Result was incorrectly responding
to the control process, and as a result the vmctl client, with a
bogus "start vm response" reply with an empty tty name.

This commit rewrites the logic of how vmd goes about processing the
"start vm" request with the aim of making it simpler to understand
while addressing the edge case.

Issue reported by kn@. OK mlarkin@.
---

diff --git a/usr.sbin/vmd/vmd.c b/usr.sbin/vmd/vmd.c
index c06fe974877..b2e8fecfc8b 100644
--- a/usr.sbin/vmd/vmd.c
+++ b/usr.sbin/vmd/vmd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vmd.c,v 1.146 2023/04/28 19:46:42 dv Exp $	*/
+/*	$OpenBSD: vmd.c,v 1.147 2023/05/12 14:42:30 dv Exp $	*/
 
 /*
  * Copyright (c) 2015 Reyk Floeter <reyk@openbsd.org>
@@ -113,25 +113,39 @@ vmd_dispatch_control(int fd, struct privsep_proc *p, struct imsg *imsg)
 		memcpy(&vmc, imsg->data, sizeof(vmc));
 		vmc.vmc_kernel = imsg->fd;
 
-		ret = vm_register(ps, &vmc, &vm, 0, vmc.vmc_owner.uid);
-		if (vmc.vmc_flags == 0 || vmc.vmc_flags == VMOP_CREATE_KERNEL) {
-			/* start an existing VM with pre-configured options */
-			if (!(ret == -1 && errno == EALREADY &&
-			    !(vm->vm_state & VM_STATE_RUNNING))) {
-				res = errno;
-				cmd = IMSG_VMDOP_START_VM_RESPONSE;
-			}
-		} else if (ret != 0) {
+		/* Try registering our VM in our list of known VMs. */
+		if (vm_register(ps, &vmc, &vm, 0, vmc.vmc_owner.uid)) {
 			res = errno;
-			cmd = IMSG_VMDOP_START_VM_RESPONSE;
-		}
 
-		if (res == 0) {
-			res = config_setvm(ps, vm, imsg->hdr.peerid,
-			    vm->vm_params.vmc_owner.uid);
-			if (res)
+			/* Did we have a failure during lookup of a parent? */
+			if (vm == NULL) {
 				cmd = IMSG_VMDOP_START_VM_RESPONSE;
+				break;
+			}
+
+			/* Does the VM already exist? */
+			if (res == EALREADY) {
+				/* Is it already running? */
+				if (vm->vm_state & VM_STATE_RUNNING) {
+					cmd = IMSG_VMDOP_START_VM_RESPONSE;
+					break;
+				}
+
+				/* If not running, are our flags ok? */
+				if (vmc.vmc_flags &&
+				    vmc.vmc_flags != VMOP_CREATE_KERNEL) {
+					cmd = IMSG_VMDOP_START_VM_RESPONSE;
+					break;
+				}
+			}
+			res = 0;
 		}
+
+		/* Try to start the launch of the VM. */
+		res = config_setvm(ps, vm, imsg->hdr.peerid,
+		    vm->vm_params.vmc_owner.uid);
+		if (res)
+			cmd = IMSG_VMDOP_START_VM_RESPONSE;
 		break;
 	case IMSG_VMDOP_WAIT_VM_REQUEST:
 	case IMSG_VMDOP_TERMINATE_VM_REQUEST: