From fbbcf6cd7f4e83fd29136932eef4302a302baa70 Mon Sep 17 00:00:00 2001 From: dv Date: Tue, 1 Mar 2022 21:46:19 +0000 Subject: [PATCH] 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@. --- usr.sbin/vmd/vm.c | 21 ++++++++++++++------- usr.sbin/vmd/vmd.c | 6 +++--- usr.sbin/vmd/vmm.c | 27 +++++++++++++++++---------- 3 files changed, 34 insertions(+), 20 deletions(-) diff --git a/usr.sbin/vmd/vm.c b/usr.sbin/vmd/vm.c index 4c6c99f1133..55d938ed1d1 100644 --- 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 @@ -26,6 +26,7 @@ #include #include #include +#include #include #include @@ -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 a60291c17f1..4d7e7b5e613 100644 --- 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 @@ -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 eb75b4c5878..10b0d0ffef0 100644 --- 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 @@ -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; -- 2.20.1