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@

diff --git a/usr.sbin/vmd/vmd.c b/usr.sbin/vmd/vmd.c
index 75af37a..38b8ad4 100644 (file)
--- 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 7bbbf62..153d420 100644 (file)
--- 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 d9eff3c..36c909e 100644 (file)
--- 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]);