From 3bbc9b4e3ba89575448d218a699a85a2355d9812 Mon Sep 17 00:00:00 2001
From: dv <dv@openbsd.org>
Date: Sun, 16 Apr 2023 12:47:26 +0000
Subject: [PATCH] vmd(8): clean up fd closing in vmm process.

Some mild tidying of fd closing in the vmm process in prep for
landing parts of my fork+exec diff.

With input from guenther@ on the nuances of if/when EINTR may happen
in a call to close(2).

ok mlarkin@
---
 usr.sbin/vmd/vmd.c | 28 +++++++++++++++++++++++++++-
 usr.sbin/vmd/vmd.h |  3 ++-
 usr.sbin/vmd/vmm.c | 35 +++++++++++++----------------------
 3 files changed, 42 insertions(+), 24 deletions(-)

diff --git a/usr.sbin/vmd/vmd.c b/usr.sbin/vmd/vmd.c
index 75af37a29a6..38b8ad46bf5 100644
--- a/usr.sbin/vmd/vmd.c
+++ b/usr.sbin/vmd/vmd.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vmd.c,v 1.139 2023/04/02 02:04:10 dv Exp $	*/
+/*	$OpenBSD: vmd.c,v 1.140 2023/04/16 12:47:26 dv Exp $	*/
 
 /*
  * Copyright (c) 2015 Reyk Floeter <reyk@openbsd.org>
@@ -1956,3 +1956,29 @@ vm_terminate(struct vmd_vm *vm, const char *caller)
 		vm_remove(vm, caller);
 	}
 }
+
+/*
+ * Utility function for closing vm file descriptors. Assumes an fd of -1 was
+ * already closed or never opened.
+ *
+ * Returns 0 on success, otherwise -1 on failure.
+ */
+int
+close_fd(int fd)
+{
+	int	ret;
+
+	if (fd == -1)
+		return (0);
+
+#ifdef POSIX_CLOSE_RESTART
+	do { ret = close(fd); } while (ret == -1 && errno == EINTR);
+#else
+	ret = close(fd);
+#endif /* POSIX_CLOSE_RESTART */
+
+	if (ret == -1 && errno == EIO)
+		log_warn("%s(%d)", __func__, fd);
+
+	return (ret);
+}
diff --git a/usr.sbin/vmd/vmd.h b/usr.sbin/vmd/vmd.h
index 7bbbf62734b..153d4206257 100644
--- a/usr.sbin/vmd/vmd.h
+++ b/usr.sbin/vmd/vmd.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vmd.h,v 1.115 2023/04/02 02:04:10 dv Exp $	*/
+/*	$OpenBSD: vmd.h,v 1.116 2023/04/16 12:47:26 dv Exp $	*/
 
 /*
  * Copyright (c) 2015 Mike Larkin <mlarkin@openbsd.org>
@@ -443,6 +443,7 @@ char	*get_string(uint8_t *, size_t);
 uint32_t prefixlen2mask(uint8_t);
 void	 prefixlen2mask6(u_int8_t, struct in6_addr *);
 void	 getmonotime(struct timeval *);
+int	 close_fd(int);
 
 /* priv.c */
 void	 priv(struct privsep *, struct privsep_proc *);
diff --git a/usr.sbin/vmd/vmm.c b/usr.sbin/vmd/vmm.c
index d9eff3c8f70..36c909e94be 100644
--- a/usr.sbin/vmd/vmm.c
+++ b/usr.sbin/vmd/vmm.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: vmm.c,v 1.107 2023/01/14 20:55:55 dv Exp $	*/
+/*	$OpenBSD: vmm.c,v 1.108 2023/04/16 12:47:26 dv Exp $	*/
 
 /*
  * Copyright (c) 2015 Mike Larkin <mlarkin@openbsd.org>
@@ -641,12 +641,10 @@ vmm_start_vm(struct imsg *imsg, uint32_t *id, pid_t *pid)
 	if (socketpair(AF_UNIX, SOCK_STREAM, PF_UNSPEC, fds) == -1)
 		fatal("socketpair");
 
-	/* Start child vmd for this VM (fork, chroot, drop privs) */
+	/* Fork the vmm process to create the vm, inheriting open device fds. */
 	vm_pid = fork();
-
-	/* Start child failed? - cleanup and leave */
 	if (vm_pid == -1) {
-		log_warnx("%s: start child failed", __func__);
+		log_warn("%s: fork child failed", __func__);
 		ret = EIO;
 		goto err;
 	}
@@ -654,31 +652,24 @@ vmm_start_vm(struct imsg *imsg, uint32_t *id, pid_t *pid)
 	if (vm_pid > 0) {
 		/* Parent */
 		vm->vm_pid = vm_pid;
-		close(fds[1]);
+		close_fd(fds[1]);
 
 		for (i = 0 ; i < vcp->vcp_ndisks; i++) {
 			for (j = 0; j < VM_MAX_BASE_PER_DISK; j++) {
-				if (vm->vm_disks[i][j] != -1)
-					close(vm->vm_disks[i][j]);
-				vm->vm_disks[i][j] = -1;
+				if (close_fd(vm->vm_disks[i][j]) == 0)
+				    vm->vm_disks[i][j] = -1;
 			}
 		}
 		for (i = 0 ; i < vcp->vcp_nnics; i++) {
-			close(vm->vm_ifs[i].vif_fd);
-			vm->vm_ifs[i].vif_fd = -1;
+			if (close_fd(vm->vm_ifs[i].vif_fd) == 0)
+			    vm->vm_ifs[i].vif_fd = -1;
 		}
-		if (vm->vm_kernel != -1) {
-			close(vm->vm_kernel);
+		if (close_fd(vm->vm_kernel) == 0)
 			vm->vm_kernel = -1;
-		}
-		if (vm->vm_cdrom != -1) {
-			close(vm->vm_cdrom);
+		if (close_fd(vm->vm_cdrom) == 0)
 			vm->vm_cdrom = -1;
-		}
-		if (vm->vm_tty != -1) {
-			close(vm->vm_tty);
+		if (close_fd(vm->vm_tty) == 0)
 			vm->vm_tty = -1;
-		}
 
 		/* Read back the kernel-generated vm id from the child */
 		sz = atomicio(read, fds[0], &vcp->vcp_id, sizeof(vcp->vcp_id));
@@ -702,8 +693,8 @@ vmm_start_vm(struct imsg *imsg, uint32_t *id, pid_t *pid)
 		return (0);
 	} else {
 		/* Child */
-		close(fds[0]);
-		close(PROC_PARENT_SOCK_FILENO);
+		close_fd(fds[0]);
+		close_fd(PROC_PARENT_SOCK_FILENO);
 
 		ret = start_vm(vm, fds[1]);
 
-- 
2.20.1