Download raw body.
vmd: remove PROC_CONTROL bits from proc.[ch]
On Fri, Nov 14, 2025 at 10:07:46AM +0100, Martijn van Duren wrote:
> 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?
>
dv@ should decide here. not sure I grok this but at first read it seems like
shuffling deck chairs? what benefit does this gain?
> 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);
>
vmd: remove PROC_CONTROL bits from proc.[ch]