Sync proc.c file vmd(8) and switchd(8).

New changes:
 * Fix msgbuf_write() usage idiom;
 * Add context (function name) that fatal()ed;
 * Use less fds on startup;

ok mlarkin@, reyk@

1 files changed, 90 insertions, 78 deletions

diff --git a/usr.sbin/switchd/proc.c b/usr.sbin/switchd/proc.c
index 37e4667f330..7b5e31f18f5 100644
--- a/usr.sbin/switchd/proc.c
+++ b/usr.sbin/switchd/proc.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: proc.c,v 1.7 2016/10/05 17:31:28 rzalamena Exp $	*/
+/*	$OpenBSD: proc.c,v 1.8 2016/10/11 07:45:26 rzalamena Exp $	*/
 
 /*
  * Copyright (c) 2010 - 2016 Reyk Floeter <reyk@openbsd.org>
@@ -37,8 +37,6 @@
 
 void	 proc_exec(struct privsep *, struct privsep_proc *, unsigned int,
 	    int, char **);
-void	 proc_connectpeer(struct privsep *, enum privsep_procid, int,
-	    struct privsep_pipes *);
 void	 proc_setup(struct privsep *, struct privsep_proc *, unsigned int);
 void	 proc_open(struct privsep *, int, int);
 void	 proc_accept(struct privsep *, int, enum privsep_procid,
@@ -157,72 +155,38 @@ proc_exec(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
 }
 
 void
-proc_connectpeer(struct privsep *ps, enum privsep_procid id, int inst,
-    struct privsep_pipes *pp)
-{
-	unsigned int		 i, j;
-	struct privsep_fd	 pf;
-
-	for (i = 0; i < PROC_MAX; i++) {
-		/* Parent is already connected with everyone. */
-		if (i == PROC_PARENT)
-			continue;
-
-		for (j = 0; j < ps->ps_instances[i]; j++) {
-			/* Don't send socket to child itself. */
-			if (i == (unsigned int)id &&
-			    j == (unsigned int)inst)
-				continue;
-			if (pp->pp_pipes[i][j] == -1)
-				continue;
-
-			pf.pf_procid = i;
-			pf.pf_instance = j;
-			proc_compose_imsg(ps, id, inst, IMSG_CTL_PROCFD,
-			    -1, pp->pp_pipes[i][j], &pf, sizeof(pf));
-			pp->pp_pipes[i][j] = -1;
-		}
-	}
-}
-
-/* Inter-connect all process except with ourself. */
-void
 proc_connect(struct privsep *ps)
 {
-	unsigned int		 src, i, j;
-	struct privsep_pipes	*pp;
 	struct imsgev		*iev;
+	unsigned int		 src, dst, inst;
 
-	/* Listen on appropriate pipes. */
-	src = privsep_process;
-	pp = &ps->ps_pipes[src][ps->ps_instance];
+	/* Don't distribute any sockets if we are not really going to run. */
+	if (ps->ps_noaction)
+		return;
 
-	for (i = 0; i < PROC_MAX; i++) {
-		/* Don't listen to ourself. */
-		if (i == src)
+	for (dst = 0; dst < PROC_MAX; dst++) {
+		/* We don't communicate with ourselves. */
+		if (dst == PROC_PARENT)
 			continue;
 
-		for (j = 0; j < ps->ps_instances[i]; j++) {
-			if (pp->pp_pipes[i][j] == -1)
-				continue;
-
-			iev = &ps->ps_ievs[i][j];
-			imsg_init(&iev->ibuf, pp->pp_pipes[i][j]);
+		for (inst = 0; inst < ps->ps_instances[dst]; inst++) {
+			iev = &ps->ps_ievs[dst][inst];
+			imsg_init(&iev->ibuf, ps->ps_pp->pp_pipes[dst][inst]);
 			event_set(&iev->ev, iev->ibuf.fd, iev->events,
 			    iev->handler, iev->data);
 			event_add(&iev->ev, NULL);
 		}
 	}
 
-	/* Exchange pipes between process. */
-	for (i = 0; i < PROC_MAX; i++) {
-		/* Parent is already connected with everyone. */
-		if (i == PROC_PARENT)
-			continue;
+	/* Distribute the socketpair()s for everyone. */
+	for (src = 0; src < PROC_MAX; src++)
+		for (dst = src; dst < PROC_MAX; dst++) {
+			/* Parent already distributed its fds. */
+			if (src == PROC_PARENT || dst == PROC_PARENT)
+				continue;
 
-		for (j = 0; j < ps->ps_instances[i]; j++)
-			proc_connectpeer(ps, i, j, &ps->ps_pipes[i][j]);
-	}
+			proc_open(ps, src, dst);
+		}
 }
 
 void
@@ -230,17 +194,41 @@ proc_init(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
     int argc, char **argv, enum privsep_procid proc_id)
 {
 	struct privsep_proc	*p = NULL;
+	struct privsep_pipes	*pa, *pb;
 	unsigned int		 proc;
-	unsigned int		 src, dst;
+	unsigned int		 dst;
+	int			 fds[2];
+
+	/* Don't initiate anything if we are not really going to run. */
+	if (ps->ps_noaction)
+		return;
 
 	if (proc_id == PROC_PARENT) {
 		privsep_process = PROC_PARENT;
 		proc_setup(ps, procs, nproc);
 
-		/* Open socketpair()s for everyone. */
-		for (src = 0; src < PROC_MAX; src++)
-			for (dst = 0; dst < PROC_MAX; dst++)
-				proc_open(ps, src, dst);
+		/*
+		 * Create the children sockets so we can use them 
+		 * to distribute the rest of the socketpair()s using
+		 * proc_connect() later.
+		 */
+		for (dst = 0; dst < PROC_MAX; dst++) {
+			/* Don't create socket for ourselves. */
+			if (dst == PROC_PARENT)
+				continue;
+
+			for (proc = 0; proc < ps->ps_instances[dst]; proc++) {
+				pa = &ps->ps_pipes[PROC_PARENT][0];
+				pb = &ps->ps_pipes[dst][proc];
+				if (socketpair(AF_UNIX,
+				    SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC,
+				    PF_UNSPEC, fds) == -1)
+					fatal("%s: socketpair", __func__);
+
+				pa->pp_pipes[dst][proc] = fds[0];
+				pb->pp_pipes[PROC_PARENT][0] = fds[1];
+			}
+		}
 
 		/* Engage! */
 		proc_exec(ps, procs, nproc, argc, argv);
@@ -317,7 +305,7 @@ proc_setup(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc)
 		ps->ps_title[id] = procs[src].p_title;
 		if ((ps->ps_ievs[id] = calloc(ps->ps_instances[id],
 		    sizeof(struct imsgev))) == NULL)
-			fatal(__func__);
+			fatal("%s: calloc", __func__);
 
 		/* With this set up, we are ready to call imsg_init(). */
 		for (i = 0; i < ps->ps_instances[id]; i++) {
@@ -409,9 +397,11 @@ void
 proc_open(struct privsep *ps, int src, int dst)
 {
 	struct privsep_pipes	*pa, *pb;
+	struct privsep_fd	 pf;
 	int			 fds[2];
 	unsigned int		 i, j;
 
+	/* Exchange pipes between process. */
 	for (i = 0; i < ps->ps_instances[src]; i++) {
 		for (j = 0; j < ps->ps_instances[dst]; j++) {
 			/* Don't create sockets for ourself. */
@@ -420,16 +410,36 @@ proc_open(struct privsep *ps, int src, int dst)
 
 			pa = &ps->ps_pipes[src][i];
 			pb = &ps->ps_pipes[dst][j];
-			if (pb->pp_pipes[dst][j] != -1)
-				continue;
-
 			if (socketpair(AF_UNIX,
 			    SOCK_STREAM | SOCK_NONBLOCK | SOCK_CLOEXEC,
 			    PF_UNSPEC, fds) == -1)
-				fatal(__func__);
+				fatal("%s: socketpair", __func__);
 
 			pa->pp_pipes[dst][j] = fds[0];
 			pb->pp_pipes[src][i] = fds[1];
+
+			pf.pf_procid = src;
+			pf.pf_instance = i;
+			if (proc_compose_imsg(ps, dst, j, IMSG_CTL_PROCFD,
+			    -1, pb->pp_pipes[src][i], &pf, sizeof(pf)) == -1)
+				fatal("%s: proc_compose_imsg", __func__);
+
+			pf.pf_procid = dst;
+			pf.pf_instance = j;
+			if (proc_compose_imsg(ps, src, i, IMSG_CTL_PROCFD,
+			    -1, pa->pp_pipes[dst][j], &pf, sizeof(pf)) == -1)
+				fatal("%s: proc_compose_imsg", __func__);
+
+			/*
+			 * We have to flush to send the descriptors and close
+			 * them to avoid the fd ramp on startup.
+			 */
+			if (imsg_flush(&ps->ps_ievs[src][i].ibuf) == -1 ||
+			    imsg_flush(&ps->ps_ievs[dst][j].ibuf) == -1)
+				fatal("%s: imsg_flush", __func__);
+
+			imsg_event_add(&ps->ps_ievs[src][i]);
+			imsg_event_add(&ps->ps_ievs[dst][j]);
 		}
 	}
 }
@@ -512,9 +522,6 @@ proc_run(struct privsep *ps, struct privsep_proc *p,
 	const char		*root;
 	struct control_sock	*rcs;
 
-	if (ps->ps_noaction)
-		exit(0);
-
 	log_procinit(p->p_title);
 
 	/* Set the process group of the current process */
@@ -522,10 +529,10 @@ proc_run(struct privsep *ps, struct privsep_proc *p,
 
 	if (p->p_id == PROC_CONTROL && ps->ps_instance == 0) {
 		if (control_init(ps, &ps->ps_csock) == -1)
-			fatalx(__func__);
+			fatalx("%s: control_init", __func__);
 		TAILQ_FOREACH(rcs, &ps->ps_rcsocks, cs_entry)
 			if (control_init(ps, rcs) == -1)
-				fatalx(__func__);
+				fatalx("%s: control_init", __func__);
 	}
 
 	/* Use non-standard user */
@@ -575,10 +582,10 @@ proc_run(struct privsep *ps, struct privsep_proc *p,
 	if (p->p_id == PROC_CONTROL && ps->ps_instance == 0) {
 		TAILQ_INIT(&ctl_conns);
 		if (control_listen(&ps->ps_csock) == -1)
-			fatalx(__func__);
+			fatalx("%s: control_listen", __func__);
 		TAILQ_FOREACH(rcs, &ps->ps_rcsocks, cs_entry)
 			if (control_listen(rcs) == -1)
-				fatalx(__func__);
+				fatalx("%s: control_listen", __func__);
 	}
 
 	DPRINTF("%s: %s %d/%d, pid %d", __func__, p->p_title,
@@ -610,7 +617,7 @@ proc_dispatch(int fd, short event, void *arg)
 
 	if (event & EV_READ) {
 		if ((n = imsg_read(ibuf)) == -1 && errno != EAGAIN)
-			fatal(__func__);
+			fatal("%s: imsg_read", __func__);
 		if (n == 0) {
 			/* this pipe is dead, so remove the event handler */
 			event_del(&iev->ev);
@@ -620,13 +627,19 @@ proc_dispatch(int fd, short event, void *arg)
 	}
 
 	if (event & EV_WRITE) {
-		if (msgbuf_write(&ibuf->w) <= 0 && errno != EAGAIN)
-			fatal(__func__);
+		if ((n = msgbuf_write(&ibuf->w)) == -1 && errno != EAGAIN)
+			fatal("%s: msgbuf_write", __func__);
+		if (n == 0) {
+			/* this pipe is dead, so remove the event handler */
+			event_del(&iev->ev);
+			event_loopexit(NULL);
+			return;
+		}
 	}
 
 	for (;;) {
 		if ((n = imsg_get(ibuf, &imsg)) == -1)
-			fatal(__func__);
+			fatal("%s: imsg_get", __func__);
 		if (n == 0)
 			break;
 
@@ -661,12 +674,11 @@ proc_dispatch(int fd, short event, void *arg)
 			    pf.pf_instance);
 			break;
 		default:
-			log_warnx("%s: %s %d got invalid imsg %d peerid %d "
+			fatalx("%s: %s %d got invalid imsg %d peerid %d "
 			    "from %s %d",
 			    __func__, title, ps->ps_instance + 1,
 			    imsg.hdr.type, imsg.hdr.peerid,
 			    p->p_title, imsg.hdr.pid);
-			fatalx(__func__);
 		}
 		imsg_free(&imsg);
 	}