From: Claudio Jeker Subject: vmd: convert to imsg_get_fd() To: dv@openbsd.org, tech@openbsd.org Date: Thu, 18 Jan 2024 14:21:32 +0100 vmd is doing a lot of fd passing so this diff is far more complex then any other daemon (apart from smtpd). I did not manage to figure out if proc_forward_imsg() needs to forward fds or not. So I took the save route and forward with imsg_get_fd(). Works with my very basic use of vmd. -- :wq Claudio Index: config.c =================================================================== RCS file: /cvs/src/usr.sbin/vmd/config.c,v diff -u -p -r1.73 config.c --- config.c 3 Jan 2024 22:34:39 -0000 1.73 +++ config.c 16 Jan 2024 12:43:43 -0000 @@ -552,10 +552,12 @@ config_getvm(struct privsep *ps, struct { struct vmop_create_params vmc; struct vmd_vm *vm = NULL; + int fd; IMSG_SIZE_CHECK(imsg, &vmc); memcpy(&vmc, imsg->data, sizeof(vmc)); - vmc.vmc_kernel = imsg->fd; + fd = imsg_get_fd(imsg); + vmc.vmc_kernel = fd; errno = 0; if (vm_register(ps, &vmc, &vm, imsg->hdr.peerid, 0) == -1) @@ -563,14 +565,12 @@ config_getvm(struct privsep *ps, struct vm->vm_state |= VM_STATE_RUNNING; vm->vm_peerid = (uint32_t)-1; - vm->vm_kernel = imsg->fd; + vm->vm_kernel = fd; return (0); fail: - if (imsg->fd != -1) { - close(imsg->fd); - imsg->fd = -1; - } + if (fd != -1) + close(fd); vm_remove(vm, __func__); if (errno == 0) @@ -584,6 +584,7 @@ config_getdisk(struct privsep *ps, struc { struct vmd_vm *vm; unsigned int n, idx; + int fd; errno = 0; if ((vm = vm_getbyvmid(imsg->hdr.peerid)) == NULL) { @@ -593,8 +594,9 @@ config_getdisk(struct privsep *ps, struc IMSG_SIZE_CHECK(imsg, &n); memcpy(&n, imsg->data, sizeof(n)); + fd = imsg_get_fd(imsg); - if (n >= vm->vm_params.vmc_ndisks || imsg->fd == -1) { + if (n >= vm->vm_params.vmc_ndisks || fd == -1) { log_warnx("invalid disk id"); errno = EINVAL; return (-1); @@ -605,7 +607,7 @@ config_getdisk(struct privsep *ps, struc errno = EINVAL; return (-1); } - vm->vm_disks[n][idx] = imsg->fd; + vm->vm_disks[n][idx] = fd; return (0); } @@ -614,6 +616,7 @@ config_getif(struct privsep *ps, struct { struct vmd_vm *vm; unsigned int n; + int fd; errno = 0; if ((vm = vm_getbyvmid(imsg->hdr.peerid)) == NULL) { @@ -623,16 +626,18 @@ config_getif(struct privsep *ps, struct IMSG_SIZE_CHECK(imsg, &n); memcpy(&n, imsg->data, sizeof(n)); + fd = imsg_get_fd(imsg); + if (n >= vm->vm_params.vmc_nnics || - vm->vm_ifs[n].vif_fd != -1 || imsg->fd == -1) { + vm->vm_ifs[n].vif_fd != -1 || fd == -1) { log_warnx("invalid interface id"); goto fail; } - vm->vm_ifs[n].vif_fd = imsg->fd; + vm->vm_ifs[n].vif_fd = fd; return (0); fail: - if (imsg->fd != -1) - close(imsg->fd); + if (fd != -1) + close(fd); errno = EINVAL; return (-1); } @@ -641,6 +646,7 @@ int config_getcdrom(struct privsep *ps, struct imsg *imsg) { struct vmd_vm *vm; + int fd; errno = 0; if ((vm = vm_getbyvmid(imsg->hdr.peerid)) == NULL) { @@ -648,16 +654,15 @@ config_getcdrom(struct privsep *ps, stru return (-1); } - if (imsg->fd == -1) { + fd = imsg_get_fd(imsg); + if (fd == -1) { log_warnx("invalid cdrom id"); goto fail; } - vm->vm_cdrom = imsg->fd; + vm->vm_cdrom = fd; return (0); fail: - if (imsg->fd != -1) - close(imsg->fd); errno = EINVAL; return (-1); } Index: control.c =================================================================== RCS file: /cvs/src/usr.sbin/vmd/control.c,v diff -u -p -r1.41 control.c --- control.c 28 Apr 2023 19:46:42 -0000 1.41 +++ control.c 16 Jan 2024 11:40:02 -0000 @@ -440,7 +440,7 @@ control_dispatch_imsg(int fd, short even case IMSG_VMDOP_RELOAD: case IMSG_CTL_RESET: if (proc_compose_imsg(ps, PROC_PARENT, -1, - imsg.hdr.type, fd, imsg.fd, + imsg.hdr.type, fd, imsg_get_fd(&imsg), imsg.data, IMSG_DATA_SIZE(&imsg)) == -1) goto fail; break; @@ -453,7 +453,7 @@ control_dispatch_imsg(int fd, short even /* imsg.fd may contain kernel image fd. */ if (proc_compose_imsg(ps, PROC_PARENT, -1, - imsg.hdr.type, fd, imsg.fd, &vmc, + imsg.hdr.type, fd, imsg_get_fd(&imsg), &vmc, sizeof(vmc)) == -1) { control_close(fd, cs); return; @@ -508,7 +508,7 @@ control_dispatch_imsg(int fd, short even vid.vid_uid); if (proc_compose_imsg(ps, PROC_PARENT, -1, - imsg.hdr.type, fd, imsg.fd, + imsg.hdr.type, fd, imsg_get_fd(&imsg), &vid, sizeof(vid)) == -1) goto fail; break; Index: priv.c =================================================================== RCS file: /cvs/src/usr.sbin/vmd/priv.c,v diff -u -p -r1.23 priv.c --- priv.c 13 Jul 2023 18:31:59 -0000 1.23 +++ priv.c 16 Jan 2024 11:41:21 -0000 @@ -94,6 +94,7 @@ priv_dispatch_parent(int fd, struct priv struct vmop_addr_req vareq; struct vmop_addr_result varesult; char type[IF_NAMESIZE]; + int ifd; switch (imsg->hdr.type) { case IMSG_VMDOP_PRIV_IFDESCR: @@ -254,14 +255,15 @@ priv_dispatch_parent(int fd, struct priv varesult.var_vmid = vareq.var_vmid; varesult.var_nic_idx = vareq.var_nic_idx; + ifd = imsg_get_fd(imsg); /* resolve lladdr for the tap(4) and send back to parent */ - if (ioctl(imsg->fd, SIOCGIFADDR, &varesult.var_addr) != 0) + if (ioctl(ifd, SIOCGIFADDR, &varesult.var_addr) != 0) log_warn("SIOCGIFADDR"); else proc_compose_imsg(ps, PROC_PARENT, -1, IMSG_VMDOP_PRIV_GET_ADDR_RESPONSE, imsg->hdr.peerid, -1, &varesult, sizeof(varesult)); - close(imsg->fd); + close(ifd); break; case IMSG_VMDOP_CONFIG: config_getconfig(env, imsg); Index: proc.c =================================================================== RCS file: /cvs/src/usr.sbin/vmd/proc.c,v diff -u -p -r1.21 proc.c --- proc.c 26 Sep 2023 01:53:54 -0000 1.21 +++ proc.c 16 Jan 2024 11:53:27 -0000 @@ -661,7 +661,7 @@ proc_dispatch(int fd, short event, void case IMSG_CTL_PROCFD: IMSG_SIZE_CHECK(&imsg, &pf); memcpy(&pf, imsg.data, sizeof(pf)); - proc_accept(ps, imsg.fd, pf.pf_procid, + proc_accept(ps, imsg_get_fd(&imsg), pf.pf_procid, pf.pf_instance); break; default: @@ -792,7 +792,8 @@ proc_forward_imsg(struct privsep *ps, st enum privsep_procid id, int n) { return (proc_compose_imsg(ps, id, n, imsg->hdr.type, - imsg->hdr.peerid, imsg->fd, imsg->data, IMSG_DATA_SIZE(imsg))); + imsg->hdr.peerid, imsg_get_fd(imsg), imsg->data, + IMSG_DATA_SIZE(imsg))); } struct imsgbuf * Index: vm.c =================================================================== RCS file: /cvs/src/usr.sbin/vmd/vm.c,v diff -u -p -r1.95 vm.c --- vm.c 10 Jan 2024 04:13:59 -0000 1.95 +++ vm.c 16 Jan 2024 12:48:23 -0000 @@ -596,7 +596,7 @@ vm_dispatch_vmm(int fd, short event, voi break; case IMSG_VMDOP_SEND_VM_REQUEST: vmr.vmr_id = vm->vm_vmid; - vmr.vmr_result = send_vm(imsg.fd, vm); + vmr.vmr_result = send_vm(imsg_get_fd(&imsg), vm); imsg_compose_event(&vm->vm_iev, IMSG_VMDOP_SEND_VM_RESPONSE, imsg.hdr.peerid, imsg.hdr.pid, -1, &vmr, Index: vmd.c =================================================================== RCS file: /cvs/src/usr.sbin/vmd/vmd.c,v diff -u -p -r1.152 vmd.c --- vmd.c 26 Sep 2023 01:53:54 -0000 1.152 +++ vmd.c 18 Jan 2024 13:17:32 -0000 @@ -97,6 +97,7 @@ vmd_dispatch_control(int fd, struct priv { struct privsep *ps = p->p_ps; int res = 0, ret = 0, cmd = 0, verbose; + int ifd; unsigned int v = 0, flags; struct vmop_create_params vmc; struct vmop_id vid; @@ -111,7 +112,7 @@ vmd_dispatch_control(int fd, struct priv case IMSG_VMDOP_START_VM_REQUEST: IMSG_SIZE_CHECK(imsg, &vmc); memcpy(&vmc, imsg->data, sizeof(vmc)); - vmc.vmc_kernel = imsg->fd; + vmc.vmc_kernel = imsg_get_fd(imsg); /* Try registering our VM in our list of known VMs. */ if (vm_register(ps, &vmc, &vm, 0, vmc.vmc_owner.uid)) { @@ -257,11 +258,12 @@ vmd_dispatch_control(int fd, struct priv IMSG_SIZE_CHECK(imsg, &vid); memcpy(&vid, imsg->data, sizeof(vid)); id = vid.vid_id; + ifd = imsg_get_fd(imsg); if (vid.vid_id == 0) { if ((vm = vm_getbyname(vid.vid_name)) == NULL) { res = ENOENT; cmd = IMSG_VMDOP_SEND_VM_RESPONSE; - close(imsg->fd); + close(ifd); break; } else { vid.vid_id = vm->vm_vmid; @@ -269,43 +271,42 @@ vmd_dispatch_control(int fd, struct priv } else if ((vm = vm_getbyvmid(vid.vid_id)) == NULL) { res = ENOENT; cmd = IMSG_VMDOP_SEND_VM_RESPONSE; - close(imsg->fd); + close(ifd); break; } vmr.vmr_id = vid.vid_id; log_debug("%s: sending fd to vmm", __func__); proc_compose_imsg(ps, PROC_VMM, -1, imsg->hdr.type, - imsg->hdr.peerid, imsg->fd, &vid, sizeof(vid)); + imsg->hdr.peerid, ifd, &vid, sizeof(vid)); break; case IMSG_VMDOP_RECEIVE_VM_REQUEST: IMSG_SIZE_CHECK(imsg, &vid); memcpy(&vid, imsg->data, sizeof(vid)); - if (imsg->fd == -1) { + ifd = imsg_get_fd(imsg); + if (ifd == -1) { log_warnx("%s: invalid fd", __func__); return (-1); } - if (atomicio(read, imsg->fd, &vmh, sizeof(vmh)) != - sizeof(vmh)) { + if (atomicio(read, ifd, &vmh, sizeof(vmh)) != sizeof(vmh)) { log_warnx("%s: error reading vmh from received vm", __func__); res = EIO; - close(imsg->fd); + close(ifd); cmd = IMSG_VMDOP_START_VM_RESPONSE; break; } if (vmd_check_vmh(&vmh)) { res = ENOENT; - close(imsg->fd); + close(ifd); cmd = IMSG_VMDOP_START_VM_RESPONSE; break; } - if (atomicio(read, imsg->fd, &vmc, sizeof(vmc)) != - sizeof(vmc)) { + if (atomicio(read, ifd, &vmc, sizeof(vmc)) != sizeof(vmc)) { log_warnx("%s: error reading vmc from received vm", __func__); res = EIO; - close(imsg->fd); + close(ifd); cmd = IMSG_VMDOP_START_VM_RESPONSE; break; } @@ -317,14 +318,14 @@ vmd_dispatch_control(int fd, struct priv if (ret != 0) { res = errno; cmd = IMSG_VMDOP_START_VM_RESPONSE; - close(imsg->fd); + close(ifd); } else { vm->vm_state |= VM_STATE_RECEIVED; config_setvm(ps, vm, imsg->hdr.peerid, vmc.vmc_owner.uid); log_debug("%s: sending fd to vmm", __func__); proc_compose_imsg(ps, PROC_VMM, -1, - IMSG_VMDOP_RECEIVE_VM_END, vm->vm_vmid, imsg->fd, + IMSG_VMDOP_RECEIVE_VM_END, vm->vm_vmid, ifd, NULL, 0); } break; Index: vmm.c =================================================================== RCS file: /cvs/src/usr.sbin/vmd/vmm.c,v diff -u -p -r1.116 vmm.c --- vmm.c 3 Jan 2024 22:34:39 -0000 1.116 +++ vmm.c 16 Jan 2024 12:47:30 -0000 @@ -252,7 +252,7 @@ vmm_dispatch_parent(int fd, struct privs } imsg_compose_event(&vm->vm_iev, imsg->hdr.type, imsg->hdr.peerid, imsg->hdr.pid, - imsg->fd, &vid, sizeof(vid)); + imsg_get_fd(imsg), &vid, sizeof(vid)); break; case IMSG_VMDOP_UNPAUSE_VM: IMSG_SIZE_CHECK(imsg, &vid); @@ -265,7 +265,7 @@ vmm_dispatch_parent(int fd, struct privs } imsg_compose_event(&vm->vm_iev, imsg->hdr.type, imsg->hdr.peerid, imsg->hdr.pid, - imsg->fd, &vid, sizeof(vid)); + imsg_get_fd(imsg), &vid, sizeof(vid)); break; case IMSG_VMDOP_SEND_VM_REQUEST: IMSG_SIZE_CHECK(imsg, &vid); @@ -273,13 +273,13 @@ vmm_dispatch_parent(int fd, struct privs id = vid.vid_id; if ((vm = vm_getbyvmid(id)) == NULL) { res = ENOENT; - close(imsg->fd); + close(imsg_get_fd(imsg)); /* XXX */ cmd = IMSG_VMDOP_START_VM_RESPONSE; break; } imsg_compose_event(&vm->vm_iev, imsg->hdr.type, imsg->hdr.peerid, imsg->hdr.pid, - imsg->fd, &vid, sizeof(vid)); + imsg_get_fd(imsg), &vid, sizeof(vid)); break; case IMSG_VMDOP_RECEIVE_VM_REQUEST: IMSG_SIZE_CHECK(imsg, &vmc); @@ -290,18 +290,18 @@ vmm_dispatch_parent(int fd, struct privs cmd = IMSG_VMDOP_START_VM_RESPONSE; break; } - vm->vm_tty = imsg->fd; + vm->vm_tty = imsg_get_fd(imsg); vm->vm_state |= VM_STATE_RECEIVED; vm->vm_state |= VM_STATE_PAUSED; break; case IMSG_VMDOP_RECEIVE_VM_END: if ((vm = vm_getbyvmid(imsg->hdr.peerid)) == NULL) { res = ENOENT; - close(imsg->fd); + close(imsg_get_fd(imsg)); /* XXX */ cmd = IMSG_VMDOP_START_VM_RESPONSE; break; } - vm->vm_receive_fd = imsg->fd; + vm->vm_receive_fd = imsg_get_fd(imsg); res = vmm_start_vm(imsg, &id, &pid); /* Check if the ID can be mapped correctly */ if ((id = vm_id2vmid(id, NULL)) == 0) @@ -318,12 +318,12 @@ vmm_dispatch_parent(int fd, struct privs /* Forward hardware address details to the guest vm */ imsg_compose_event(&vm->vm_iev, imsg->hdr.type, imsg->hdr.peerid, imsg->hdr.pid, - imsg->fd, &var, sizeof(var)); + imsg_get_fd(imsg), &var, sizeof(var)); break; case IMSG_VMDOP_RECEIVE_VMM_FD: if (env->vmd_fd > -1) fatalx("already received vmm fd"); - env->vmd_fd = imsg->fd; + env->vmd_fd = imsg_get_fd(imsg); /* Get and terminate all running VMs */ get_info_vm(ps, NULL, 1); @@ -656,7 +656,7 @@ vmm_start_vm(struct imsg *imsg, uint32_t vcp = &vm->vm_params.vmc_params; if (!(vm->vm_state & VM_STATE_RECEIVED)) { - if ((vm->vm_tty = imsg->fd) == -1) { + if ((vm->vm_tty = imsg_get_fd(imsg)) == -1) { log_warnx("%s: can't get tty", __func__); goto err; }