From: dv Date: Sun, 18 Jul 2021 11:55:45 +0000 (+0000) Subject: vmd(8): remove invalid errno values from config_setvm X-Git-Url: http://artulab.com/gitweb/?a=commitdiff_plain;h=2d5457562462204003f7bd5dd1ba28edc377c3d7;p=openbsd vmd(8): remove invalid errno values from config_setvm Refactor config_setvm to directly return error code on failure instead of returning -1 and setting errno. It was setting unsupported values not defined in . OK mlarkin@ --- diff --git a/usr.sbin/vmd/config.c b/usr.sbin/vmd/config.c index 83c6c1e9b4d..6efdead6576 100644 --- a/usr.sbin/vmd/config.c +++ b/usr.sbin/vmd/config.c @@ -1,4 +1,4 @@ -/* $OpenBSD: config.c,v 1.62 2021/05/12 02:24:56 mlarkin Exp $ */ +/* $OpenBSD: config.c,v 1.63 2021/07/18 11:55:45 dv Exp $ */ /* * Copyright (c) 2015 Reyk Floeter @@ -208,6 +208,13 @@ config_getreset(struct vmd *env, struct imsg *imsg) return (0); } +/* + * config_setvm + * + * Configure a vm, opening any required file descriptors. + * + * Returns 0 on success, error code on failure. + */ int config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid) { @@ -216,34 +223,25 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid) struct vmop_create_params *vmc = &vm->vm_params; struct vm_create_params *vcp = &vmc->vmc_params; unsigned int i, j; - int fd = -1; - int kernfd = -1; + int fd = -1, cdromfd = -1, kernfd = -1; int *tapfds = NULL; - int cdromfd = -1; - int saved_errno = 0; - int n = 0, aflags, oflags; + int n = 0, aflags, oflags, ret = -1; char ifname[IF_NAMESIZE], *s; - char path[PATH_MAX]; - char base[PATH_MAX]; + char path[PATH_MAX], base[PATH_MAX]; unsigned int unit; struct timeval tv, rate, since_last; struct vmop_addr_req var; - errno = 0; - if (vm->vm_state & VM_STATE_RUNNING) { log_warnx("%s: vm is already running", __func__); - errno = EALREADY; - return (-1); + return (EALREADY); } /* increase the user reference counter and check user limits */ if (vm->vm_user != NULL && user_get(vm->vm_user->usr_id.uid) != NULL) { user_inc(vcp, vm->vm_user, 1); - if (user_checklimit(vm->vm_user, vcp) == -1) { - errno = EPERM; - goto fail; - } + if (user_checklimit(vm->vm_user, vcp) == -1) + return (EPERM); } /* @@ -273,8 +271,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid) if (vm->vm_start_limit >= VM_START_RATE_LIMIT) { log_warnx("%s: vm %u restarted too quickly", __func__, vcp->vcp_id); - errno = EPERM; - goto fail; + return (EPERM); } } vm->vm_start_tv = tv; @@ -285,8 +282,9 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid) tapfds = reallocarray(NULL, vcp->vcp_nnics, sizeof(*tapfds)); if (tapfds == NULL) { + ret = errno; log_warn("%s: can't allocate tap fds", __func__); - goto fail; + return (ret); } for (i = 0; i < vcp->vcp_nnics; i++) tapfds[i] = -1; @@ -294,10 +292,15 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid) vm->vm_peerid = peerid; vm->vm_uid = uid; + /* + * From here onward, all failures need cleanup and use goto fail + */ + if (!(vm->vm_state & VM_STATE_RECEIVED)) { if (strlen(vcp->vcp_kernel)) { /* Open external kernel for child */ if ((kernfd = open(vcp->vcp_kernel, O_RDONLY)) == -1) { + ret = errno; log_warn("%s: can't open kernel or BIOS " "boot image %s", __func__, vcp->vcp_kernel); goto fail; @@ -313,7 +316,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid) if (kernfd == -1 && (kernfd = open(VM_DEFAULT_BIOS, O_RDONLY)) == -1) { log_warn("can't open %s", VM_DEFAULT_BIOS); - errno = VMD_BIOS_MISSING; + ret = VMD_BIOS_MISSING; goto fail; } @@ -322,7 +325,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid) uid, R_OK) == -1) { log_warnx("vm \"%s\" no read access to kernel %s", vcp->vcp_name, vcp->vcp_kernel); - errno = EPERM; + ret = EPERM; goto fail; } } @@ -333,7 +336,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid) if ((cdromfd = open(vcp->vcp_cdrom, O_RDONLY)) == -1) { log_warn("can't open cdrom %s", vcp->vcp_cdrom); - errno = VMD_CDROM_MISSING; + ret = VMD_CDROM_MISSING; goto fail; } @@ -342,7 +345,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid) uid, R_OK) == -1) { log_warnx("vm \"%s\" no read access to cdrom %s", vcp->vcp_name, vcp->vcp_cdrom); - errno = EPERM; + ret = EPERM; goto fail; } } @@ -360,7 +363,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid) if ((diskfds[i][j] = open(path, oflags)) == -1) { log_warn("can't open disk %s", vcp->vcp_disks[i]); - errno = VMD_DISK_MISSING; + ret = VMD_DISK_MISSING; goto fail; } @@ -405,7 +408,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid) strcmp(ifname, "tap") != 0) { log_warnx("%s: invalid tap name %s", __func__, s); - errno = EINVAL; + ret = EINVAL; goto fail; } } else @@ -423,7 +426,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid) s = ifname; } if (tapfds[i] == -1) { - log_warn("%s: can't open tap %s", __func__, s); + log_warnx("%s: can't open tap %s", __func__, s); goto fail; } if ((vif->vif_name = strdup(s)) == NULL) { @@ -473,6 +476,7 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid) } /* Send VM information */ + /* XXX check proc_compose_imsg return values */ if (vm->vm_state & VM_STATE_RECEIVED) proc_compose_imsg(ps, PROC_VMM, -1, IMSG_VMDOP_RECEIVE_VM_REQUEST, vm->vm_vmid, fd, vmc, @@ -518,7 +522,6 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid) return (0); fail: - saved_errno = errno; log_warnx("failed to start vm %s", vcp->vcp_name); if (kernfd != -1) @@ -540,10 +543,8 @@ config_setvm(struct privsep *ps, struct vmd_vm *vm, uint32_t peerid, uid_t uid) } else { vm_remove(vm, __func__); } - errno = saved_errno; - if (errno == 0) - errno = EINVAL; - return (-1); + + return (ret); } int diff --git a/usr.sbin/vmd/vmd.c b/usr.sbin/vmd/vmd.c index 1138148e5c7..423a6991228 100644 --- a/usr.sbin/vmd/vmd.c +++ b/usr.sbin/vmd/vmd.c @@ -1,4 +1,4 @@ -/* $OpenBSD: vmd.c,v 1.125 2021/05/05 21:33:11 dv Exp $ */ +/* $OpenBSD: vmd.c,v 1.126 2021/07/18 11:55:45 dv Exp $ */ /* * Copyright (c) 2015 Reyk Floeter @@ -116,11 +116,11 @@ vmd_dispatch_control(int fd, struct privsep_proc *p, struct imsg *imsg) res = errno; cmd = IMSG_VMDOP_START_VM_RESPONSE; } - if (res == 0 && - config_setvm(ps, vm, - imsg->hdr.peerid, vm->vm_params.vmc_owner.uid) == -1) { - 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) + cmd = IMSG_VMDOP_START_VM_RESPONSE; } break; case IMSG_VMDOP_WAIT_VM_REQUEST: