From: Martijn van Duren Subject: vmd: remove PROC_CONTROL bits from proc.[ch] To: tech@openbsd.org Cc: Dave Voutila Date: Fri, 14 Nov 2025 10:07:46 +0100 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);