Index | Thread | Search

From:
Claudio Jeker <cjeker@diehard.n-r-g.com>
Subject:
vmd: convert to imsg_get_fd()
To:
dv@openbsd.org, tech@openbsd.org
Date:
Thu, 18 Jan 2024 14:21:32 +0100

Download raw body.

Thread
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;
 		}