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 <errno.h>.

OK mlarkin@

diff --git a/usr.sbin/vmd/config.c b/usr.sbin/vmd/config.c
index 83c6c1e..6efdead 100644 (file)
--- 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 <reyk@openbsd.org>
@@ -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 1138148..423a699 100644 (file)
--- 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 <reyk@openbsd.org>
@@ -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: