vmd(8): gracefully handle hitting data limits when starting a vm

With recent changes to login.conf(5) to restrict daemon datasize
to a finite value, users can now hit resource limits when attempting
to start a vm.

This change fixes the error path when hitting the limit. vmd(8)
will no longer abort and memory error messages are relayed to the
user.

While here, address potential under-reads/writes using atomicio
when relaying data between the child vm process and vmd's vmm
process.

Original diff from tedu@. OK mlarkin@.

diff --git a/usr.sbin/vmd/vm.c b/usr.sbin/vmd/vm.c
index 4c6c99f..55d938e 100644 (file)
--- a/usr.sbin/vmd/vm.c
+++ b/usr.sbin/vmd/vm.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: vm.c,v 1.67 2021/12/30 08:12:23 claudio Exp $ */
+/*     $OpenBSD: vm.c,v 1.68 2022/03/01 21:46:19 dv Exp $      */
 
 /*
  * Copyright (c) 2015 Mike Larkin <mlarkin@openbsd.org>
@@ -26,6 +26,7 @@
 #include <sys/socket.h>
 #include <sys/time.h>
 #include <sys/mman.h>
+#include <sys/resource.h>
 
 #include <dev/ic/i8253reg.h>
 #include <dev/isa/isareg.h>
@@ -292,17 +293,24 @@ start_vm(struct vmd_vm *vm, int fd)
        ret = alloc_guest_mem(vcp);
 
        if (ret) {
+               struct rlimit lim;
+               char buf[FMT_SCALED_STRSIZE];
+               if (ret == ENOMEM && getrlimit(RLIMIT_DATA, &lim) == 0) {
+                       if (fmt_scaled(lim.rlim_cur, buf) == 0)
+                               fatalx("could not allocate guest memory (data "
+                                   "limit is %s)", buf);
+               }
                errno = ret;
-               fatal("could not allocate guest memory - exiting");
+               fatal("could not allocate guest memory");
        }
 
        ret = vmm_create_vm(vcp);
        current_vm = vm;
 
        /* send back the kernel-generated vm id (0 on error) */
-       if (write(fd, &vcp->vcp_id, sizeof(vcp->vcp_id)) !=
+       if (atomicio(vwrite, fd, &vcp->vcp_id, sizeof(vcp->vcp_id)) !=
            sizeof(vcp->vcp_id))
-               fatal("write vcp id");
+               fatal("failed to send created vm id to vmm process");
 
        if (ret) {
                errno = ret;
@@ -319,10 +327,9 @@ start_vm(struct vmd_vm *vm, int fd)
                fatal("pledge");
 
        if (vm->vm_state & VM_STATE_RECEIVED) {
-               ret = read(vm->vm_receive_fd, &vrp, sizeof(vrp));
-               if (ret != sizeof(vrp)) {
+               ret = atomicio(read, vm->vm_receive_fd, &vrp, sizeof(vrp));
+               if (ret != sizeof(vrp))
                        fatal("received incomplete vrp - exiting");
-               }
                vrs = vrp.vrwp_regs;
        } else {
                /*

diff --git a/usr.sbin/vmd/vmd.c b/usr.sbin/vmd/vmd.c
index a60291c..4d7e7b5 100644 (file)
--- a/usr.sbin/vmd/vmd.c
+++ b/usr.sbin/vmd/vmd.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: vmd.c,v 1.129 2022/01/04 15:18:44 claudio Exp $       */
+/*     $OpenBSD: vmd.c,v 1.130 2022/03/01 21:46:19 dv Exp $    */
 
 /*
  * Copyright (c) 2015 Reyk Floeter <reyk@openbsd.org>
@@ -399,9 +399,9 @@ vmd_dispatch_vmm(int fd, struct privsep_proc *p, struct imsg *imsg)
                }
 
                if (vmr.vmr_result) {
-                       errno = vmr.vmr_result;
-                       log_warn("%s: failed to start vm", vcp->vcp_name);
+                       log_warnx("%s: failed to start vm", vcp->vcp_name);
                        vm_remove(vm, __func__);
+                       errno = vmr.vmr_result;
                        break;
                }
 

diff --git a/usr.sbin/vmd/vmm.c b/usr.sbin/vmd/vmm.c
index eb75b4c..10b0d0f 100644 (file)
--- a/usr.sbin/vmd/vmm.c
+++ b/usr.sbin/vmd/vmm.c
@@ -1,4 +1,4 @@
-/*     $OpenBSD: vmm.c,v 1.103 2022/01/04 15:25:05 claudio Exp $       */
+/*     $OpenBSD: vmm.c,v 1.104 2022/03/01 21:46:19 dv Exp $    */
 
 /*
  * Copyright (c) 2015 Mike Larkin <mlarkin@openbsd.org>
@@ -51,6 +51,7 @@
 
 #include "vmd.h"
 #include "vmm.h"
+#include "atomicio.h"
 
 void   vmm_sighdlr(int, short, void *);
 int    vmm_start_vm(struct imsg *, uint32_t *, pid_t *);
@@ -145,7 +146,7 @@ vmm_dispatch_parent(int fd, struct privsep_proc *p, struct imsg *imsg)
        case IMSG_VMDOP_START_VM_END:
                res = vmm_start_vm(imsg, &id, &pid);
                /* Check if the ID can be mapped correctly */
-               if ((id = vm_id2vmid(id, NULL)) == 0)
+               if (res == 0 && (id = vm_id2vmid(id, NULL)) == 0)
                        res = ENOENT;
                cmd = IMSG_VMDOP_START_VM_RESPONSE;
                break;
@@ -615,7 +616,8 @@ vmm_start_vm(struct imsg *imsg, uint32_t *id, pid_t *pid)
        struct vmd_vm           *vm;
        int                      ret = EINVAL;
        int                      fds[2];
-       size_t                   i, j;
+       pid_t                    vm_pid;
+       size_t                   i, j, sz;
 
        if ((vm = vm_getbyvmid(imsg->hdr.peerid)) == NULL) {
                log_warnx("%s: can't find vm", __func__);
@@ -635,18 +637,18 @@ vmm_start_vm(struct imsg *imsg, uint32_t *id, pid_t *pid)
                fatal("socketpair");
 
        /* Start child vmd for this VM (fork, chroot, drop privs) */
-       ret = fork();
+       vm_pid = fork();
 
        /* Start child failed? - cleanup and leave */
-       if (ret == -1) {
+       if (vm_pid == -1) {
                log_warnx("%s: start child failed", __func__);
                ret = EIO;
                goto err;
        }
 
-       if (ret > 0) {
+       if (vm_pid > 0) {
                /* Parent */
-               vm->vm_pid = ret;
+               vm->vm_pid = vm_pid;
                close(fds[1]);
 
                for (i = 0 ; i < vcp->vcp_ndisks; i++) {
@@ -674,9 +676,14 @@ vmm_start_vm(struct imsg *imsg, uint32_t *id, pid_t *pid)
                }
 
                /* Read back the kernel-generated vm id from the child */
-               if (read(fds[0], &vcp->vcp_id, sizeof(vcp->vcp_id)) !=
-                   sizeof(vcp->vcp_id))
-                       fatal("read vcp id");
+               sz = atomicio(read, fds[0], &vcp->vcp_id, sizeof(vcp->vcp_id));
+               if (sz != sizeof(vcp->vcp_id)) {
+                       log_debug("%s: failed to receive vm id from vm %s",
+                           __func__, vcp->vcp_name);
+                       /* vmd could not allocate memory for the vm. */
+                       ret = ENOMEM;
+                       goto err;
+               }
 
                if (vcp->vcp_id == 0)
                        goto err;