Index | Thread | Search

From:
Martijn van Duren <openbsd+tech@list.imperialat.at>
Subject:
vmd: remove PROC_CONTROL bits from proc.[ch]
To:
tech@openbsd.org
Cc:
Dave Voutila <dv@sisu.io>
Date:
Fri, 14 Nov 2025 10:07:46 +0100

Download raw body.

Thread
Hello all,

A long time ago I removed the control bits from snmpd's proc.c, because
snmpd lost any use for snmpctl. Revisiting proc.c made me realize that
control setup functionality doesn't need to be in proc.c, and only
forces us into creating the process, regardless of being needed.

Since I have the impression that vmd is currently the most actively
maintained proc.c consumer I started here, but it should probably go
just as well for the other consumers, which I'll visit if people agree
with me here.

Diff below removes all the control bits from proc.[ch] and places them
in their equivalent places inside control.c and struct vmd. Since vmd
doesn't allow for multiple control sockets I've only took over the
main, and dropped the TAILQ bits.

Thoughts? OK?

martijn@

diff /usr/src
path + /usr/src
commit - 0377483fd63d89703211981e944e9e01c48a851d
blob - bf58b437b86d280d317198e9c0ad7bf3715d70e3
file + usr.sbin/vmd/control.c
--- usr.sbin/vmd/control.c
+++ usr.sbin/vmd/control.c
@@ -59,12 +59,18 @@ static struct privsep_proc procs[] = {
 void
 control(struct privsep *ps, struct privsep_proc *p)
 {
+	extern struct vmd *env;
+
+	if (control_init(ps, &env->vmd_csock) == -1)
+		fatalx("%s: control_init", __func__);
 	proc_run(ps, p, procs, nitems(procs), control_run, NULL);
 }
 
 void
 control_run(struct privsep *ps, struct privsep_proc *p, void *arg)
 {
+	extern struct vmd *env;
+
 	/*
 	 * pledge in the control process:
 	 * stdio - for malloc and basic I/O including events.
@@ -75,6 +81,8 @@ control_run(struct privsep *ps, struct privsep_proc *p
 	if (pledge("stdio unix recvfd sendfd", NULL) == -1)
 		fatal("pledge");
 
+	if (control_listen(&env->vmd_csock) == -1)
+		fatalx("%s: control_listen", __func__);
 	/* Signal to the parent that we're done initializing. */
 	proc_compose(ps, PROC_PARENT, IMSG_VMDOP_DONE, NULL, 0);
 }
commit - 0377483fd63d89703211981e944e9e01c48a851d
blob - 967583ea5fa26da4f30fe4e6b6c7e9377782621a
file + usr.sbin/vmd/parse.y
--- usr.sbin/vmd/parse.y
+++ usr.sbin/vmd/parse.y
@@ -212,8 +212,8 @@ main		: LOCAL INET6 {
 			free($3);
 		}
 		| SOCKET OWNER owner_id {
-			env->vmd_ps.ps_csock.cs_uid = $3.uid;
-			env->vmd_ps.ps_csock.cs_gid = $3.gid == -1 ? 0 : $3.gid;
+			env->vmd_csock.cs_uid = $3.uid;
+			env->vmd_csock.cs_gid = $3.gid == -1 ? 0 : $3.gid;
 		}
 		| AGENTX {
 			env->vmd_cfg.cfg_agentx.ax_enabled = 1;
commit - 0377483fd63d89703211981e944e9e01c48a851d
blob - 6bd8d973890fe4bb0d0bf805173538df98ef4515
file + usr.sbin/vmd/proc.c
--- usr.sbin/vmd/proc.c
+++ usr.sbin/vmd/proc.c
@@ -362,18 +362,9 @@ proc_run(struct privsep *ps, struct privsep_proc *p,
 {
 	struct passwd		*pw;
 	const char		*root;
-	struct control_sock	*rcs;
 
 	log_procinit("%s", p->p_title);
 
-	if (p->p_id == PROC_CONTROL) {
-		if (control_init(ps, &ps->ps_csock) == -1)
-			fatalx("%s: control_init", __func__);
-		TAILQ_FOREACH(rcs, &ps->ps_rcsocks, cs_entry)
-			if (control_init(ps, rcs) == -1)
-				fatalx("%s: control_init", __func__);
-	}
-
 	/* Use non-standard user */
 	if (p->p_pw != NULL)
 		pw = p->p_pw;
@@ -417,13 +408,6 @@ proc_run(struct privsep *ps, struct privsep_proc *p,
 
 	proc_setup(ps, procs, nproc);
 	proc_accept(ps, PROC_PARENT_SOCK_FILENO, PROC_PARENT);
-	if (p->p_id == PROC_CONTROL) {
-		if (control_listen(&ps->ps_csock) == -1)
-			fatalx("%s: control_listen", __func__);
-		TAILQ_FOREACH(rcs, &ps->ps_rcsocks, cs_entry)
-			if (control_listen(rcs) == -1)
-				fatalx("%s: control_listen", __func__);
-	}
 
 	DPRINTF("%s: %s, pid %d", __func__, p->p_title, getpid());
 
commit - 0377483fd63d89703211981e944e9e01c48a851d
blob - 581739abf59d3d1bf0d9e026817c955fc17e6fe8
file + usr.sbin/vmd/proc.h
--- usr.sbin/vmd/proc.h
+++ usr.sbin/vmd/proc.h
@@ -43,28 +43,6 @@ struct imsgev {
 	short			 events;
 };
 
-/* control socket */
-struct control_sock {
-	const char	*cs_name;
-	struct event	 cs_ev;
-	struct event	 cs_evt;
-	int		 cs_fd;
-	int		 cs_restricted;
-	void		*cs_env;
-	uid_t		 cs_uid;
-	gid_t		 cs_gid;
-
-	TAILQ_ENTRY(control_sock) cs_entry;
-};
-TAILQ_HEAD(control_socks, control_sock);
-
-struct ctl_conn {
-	TAILQ_ENTRY(ctl_conn)	 entry;
-	struct imsgev		 iev;
-	struct sockpeercred	 peercred;
-};
-TAILQ_HEAD(ctl_connlist, ctl_conn);
-
 /* privsep */
 enum privsep_procid {
 	PROC_PARENT	= 0,
@@ -91,9 +69,6 @@ struct privsep {
 	struct passwd			*ps_pw;
 	int				 ps_noaction;
 
-	struct control_sock		 ps_csock;
-	struct control_socks		 ps_rcsocks;
-
 	/* Event and signal handlers */
 	struct event			 ps_evsigint;
 	struct event			 ps_evsigterm;
@@ -158,12 +133,6 @@ enum privsep_procid
 	 proc_getid(struct privsep_proc *, unsigned int, const char *);
 int	 proc_flush_imsg(struct privsep *, enum privsep_procid);
 
-/* control.c */
-void	 control(struct privsep *, struct privsep_proc *);
-int	 control_init(struct privsep *, struct control_sock *);
-int	 control_reset(struct control_sock *);
-int	 control_listen(struct control_sock *);
-
 /* log.c */
 void	log_init(int, int);
 void	log_procinit(const char *, ...);
commit - 0377483fd63d89703211981e944e9e01c48a851d
blob - 6132f15735b54204e3743df3bd16827a62bc74b6
file + usr.sbin/vmd/vmd.c
--- usr.sbin/vmd/vmd.c
+++ usr.sbin/vmd/vmd.c
@@ -99,7 +99,6 @@ vmd_dispatch_control(int fd, struct privsep_proc *p, s
 	struct vmd_vm			*vm = NULL;
 	char				*str = NULL;
 	uint32_t			 peer_id, type, vm_id = 0;
-	struct control_sock		*rcs;
 
 	peer_id = imsg_get_id(imsg);
 	type = imsg_get_type(imsg);
@@ -248,9 +247,7 @@ vmd_dispatch_control(int fd, struct privsep_proc *p, s
 		    &vid, sizeof(vid));
 		break;
 	case IMSG_VMDOP_DONE:
-		control_reset(&ps->ps_csock);
-		TAILQ_FOREACH(rcs, &ps->ps_rcsocks, cs_entry)
-			control_reset(rcs);
+		control_reset(&env->vmd_csock);
 		cmd = 0;
 		break;
 	default:
@@ -689,8 +686,7 @@ main(int argc, char **argv)
 	}
 
 	/* Configure the control socket */
-	ps->ps_csock.cs_name = SOCKET_NAME;
-	TAILQ_INIT(&ps->ps_rcsocks);
+	env->vmd_csock.cs_name = SOCKET_NAME;
 
 	/* Configuration will be parsed after forking the children */
 	env->vmd_conffile = conffile;
commit - 0377483fd63d89703211981e944e9e01c48a851d
blob - c1a336bb580b1c254b0faa5d4d069c90109fd313
file + usr.sbin/vmd/vmd.h
--- usr.sbin/vmd/vmd.h
+++ usr.sbin/vmd/vmd.h
@@ -353,6 +353,24 @@ struct vmd_config {
 	struct vmd_agentx	 cfg_agentx;
 };
 
+struct control_sock {
+	const char	*cs_name;
+	struct event	 cs_ev;
+	struct event	 cs_evt;
+	int		 cs_fd;
+	int		 cs_restricted;
+	void		*cs_env;
+	uid_t		 cs_uid;
+	gid_t		 cs_gid;
+};
+
+struct ctl_conn {
+	TAILQ_ENTRY(ctl_conn)	 entry;
+	struct imsgev		 iev;
+	struct sockpeercred	 peercred;
+};
+TAILQ_HEAD(ctl_connlist, ctl_conn);
+
 struct vmd {
 	struct privsep		 vmd_ps;
 	const char		*vmd_conffile;
@@ -361,6 +379,8 @@ struct vmd {
 	/* global configuration that is sent to the children */
 	struct vmd_config	 vmd_cfg;
 
+	struct control_sock	 vmd_csock;
+
 	int			 vmd_debug;
 	int			 vmd_verbose;
 	int			 vmd_noaction;
@@ -422,6 +442,12 @@ struct packet_ctx {
 	struct sockaddr_storage	 pc_dst;
 };
 
+/* control.c */
+void	 control(struct privsep *, struct privsep_proc *);
+int	 control_init(struct privsep *, struct control_sock *);
+int	 control_reset(struct control_sock *);
+int	 control_listen(struct control_sock *);
+
 /* packet.c */
 ssize_t	 assemble_hw_header(unsigned char *, size_t, size_t,
 	    struct packet_ctx *, unsigned int);