Introduce new IMSG_CTL_PROCREADY which is used to signal that all pipes

are set up by child processes. The parent sends a ping to all children
and only starts once it has received an acknowledgement from all of them.
This fixes a race condition on process startup when the parent starts
running before all children are ready.

From markus@

4 files changed, 74 insertions, 11 deletions

diff --git a/sbin/iked/iked.c b/sbin/iked/iked.c
index 7e9f6addac0..00bd3f6e2d6 100644
--- a/sbin/iked/iked.c
+++ b/sbin/iked/iked.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: iked.c,v 1.69 2024/02/15 19:04:12 tobhe Exp $	*/
+/*	$OpenBSD: iked.c,v 1.70 2024/02/15 20:10:45 tobhe Exp $	*/
 
 /*
  * Copyright (c) 2019 Tobias Heider <tobias.heider@stusta.de>
@@ -45,6 +45,7 @@ void	 parent_sig_handler(int, short, void *);
 int	 parent_dispatch_ca(int, struct privsep_proc *, struct imsg *);
 int	 parent_dispatch_control(int, struct privsep_proc *, struct imsg *);
 int	 parent_dispatch_ikev2(int, struct privsep_proc *, struct imsg *);
+void	 parent_connected(struct privsep *);
 int	 parent_configure(struct iked *);
 
 struct iked	*iked_env;
@@ -219,12 +220,9 @@ main(int argc, char *argv[])
 	signal_add(&ps->ps_evsigpipe, NULL);
 	signal_add(&ps->ps_evsigusr1, NULL);
 
-	proc_connect(ps);
-
 	vroute_init(env);
 
-	if (parent_configure(env) == -1)
-		fatalx("configuration failed");
+	proc_connect(ps, parent_connected);
 
 	event_dispatch();
 
@@ -234,6 +232,15 @@ main(int argc, char *argv[])
 	return (0);
 }
 
+void
+parent_connected(struct privsep *ps)
+{
+	struct iked	*env = ps->ps_env;
+
+	if (parent_configure(env) == -1)
+		fatalx("configuration failed");
+}
+
 int
 parent_configure(struct iked *env)
 {
diff --git a/sbin/iked/iked.h b/sbin/iked/iked.h
index f13e6a08d43..5b65a16bc95 100644
--- a/sbin/iked/iked.h
+++ b/sbin/iked/iked.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: iked.h,v 1.228 2024/02/15 19:11:00 tobhe Exp $	*/
+/*	$OpenBSD: iked.h,v 1.229 2024/02/15 20:10:45 tobhe Exp $	*/
 
 /*
  * Copyright (c) 2019 Tobias Heider <tobias.heider@stusta.de>
@@ -730,6 +730,8 @@ struct privsep {
 	struct event			 ps_evsigusr1;
 
 	struct iked			*ps_env;
+	unsigned int			 ps_connecting;
+	void				(*ps_connected)(struct privsep *);
 };
 
 struct privsep_proc {
@@ -1192,7 +1194,7 @@ void	 timer_del(struct iked *, struct iked_timer *);
 void	 proc_init(struct privsep *, struct privsep_proc *, unsigned int, int,
 	    int, char **, enum privsep_procid);
 void	 proc_kill(struct privsep *);
-void	 proc_connect(struct privsep *);
+void	 proc_connect(struct privsep *, void (*)(struct privsep *));
 void	 proc_dispatch(int, short event, void *);
 void	 proc_run(struct privsep *, struct privsep_proc *,
 	    struct privsep_proc *, unsigned int,
diff --git a/sbin/iked/proc.c b/sbin/iked/proc.c
index 69078840e5d..534427b2b65 100644
--- a/sbin/iked/proc.c
+++ b/sbin/iked/proc.c
@@ -1,4 +1,4 @@
-/*	$OpenBSD: proc.c,v 1.41 2024/02/15 19:04:12 tobhe Exp $	*/
+/*	$OpenBSD: proc.c,v 1.42 2024/02/15 20:10:45 tobhe Exp $	*/
 
 /*
  * Copyright (c) 2010 - 2016 Reyk Floeter <reyk@openbsd.org>
@@ -155,14 +155,19 @@ proc_exec(struct privsep *ps, struct privsep_proc *procs, unsigned int nproc,
 }
 
 void
-proc_connect(struct privsep *ps)
+proc_connect(struct privsep *ps, void (*connected)(struct privsep *))
 {
 	struct imsgev		*iev;
 	unsigned int		 src, dst, inst;
 
 	/* Don't distribute any sockets if we are not really going to run. */
-	if (ps->ps_noaction)
+	if (ps->ps_noaction) {
+		if (connected == NULL)
+			fatalx("%s: missing callback", __func__);
+		connected(ps);
 		return;
+	}
+	ps->ps_connected = connected;
 
 	for (dst = 0; dst < PROC_MAX; dst++) {
 		/* We don't communicate with ourselves. */
@@ -187,6 +192,27 @@ proc_connect(struct privsep *ps)
 
 			proc_open(ps, src, dst);
 		}
+
+	/*
+	 * Finally, send a ready message to everyone:
+	 * When this message is processed by the receiver, it has
+	 * already processed all IMSG_CTL_PROCFD messages and all
+	 * pipes are ready.
+	 */
+	for (dst = 0; dst < PROC_MAX; dst++) {
+		if (dst == PROC_PARENT)
+			continue;
+		for (inst = 0; inst < ps->ps_instances[dst]; inst++) {
+			if (proc_compose_imsg(ps, dst, inst, IMSG_CTL_PROCREADY,
+			    -1, -1, NULL, 0) == -1)
+				fatal("%s: proc_compose_imsg", __func__);
+			ps->ps_connecting++;
+#if DEBUG
+			log_debug("%s: #%d %s %d", __func__,
+			    ps->ps_connecting, ps->ps_title[dst], inst + 1);
+#endif
+		}
+	}
 }
 
 void
@@ -663,6 +689,33 @@ proc_dispatch(int fd, short event, void *arg)
 			proc_accept(ps, imsg_get_fd(&imsg), pf.pf_procid,
 			    pf.pf_instance);
 			break;
+		case IMSG_CTL_PROCREADY:
+#if DEBUG
+			log_debug("%s: ready-%s: #%d %s %d -> %s %d", __func__,
+			    p->p_id == PROC_PARENT ? "req" : "ack",
+			    ps->ps_connecting, p->p_title, imsg.hdr.pid,
+			    title, ps->ps_instance + 1);
+#endif
+			if (p->p_id == PROC_PARENT) {
+				/* ack that we are ready */
+				if (proc_compose_imsg(ps, PROC_PARENT, 0,
+				    IMSG_CTL_PROCREADY, -1, -1, NULL, 0) == -1)
+					fatal("%s: proc_compose_imsg", __func__);
+			} else {
+				/* parent received ack */
+				if (ps->ps_connecting == 0)
+					fatalx("%s: wrong acks", __func__);
+				if (ps->ps_instance != 0)
+					fatalx("%s: wrong instance %d",
+					    __func__, ps->ps_instance);
+				if (ps->ps_connected == NULL)
+					fatalx("%s: missing callback", __func__);
+				if (--ps->ps_connecting == 0) {
+					log_debug("%s: all connected", __func__);
+					ps->ps_connected(ps);
+				}
+			}
+			break;
 		default:
 			fatalx("%s: %s %d got invalid imsg %d peerid %d "
 			    "from %s %d",
diff --git a/sbin/iked/types.h b/sbin/iked/types.h
index fd8add52a23..6690a4ab528 100644
--- a/sbin/iked/types.h
+++ b/sbin/iked/types.h
@@ -1,4 +1,4 @@
-/*	$OpenBSD: types.h,v 1.53 2024/01/15 15:29:00 tobhe Exp $	*/
+/*	$OpenBSD: types.h,v 1.54 2024/02/15 20:10:45 tobhe Exp $	*/
 
 /*
  * Copyright (c) 2019 Tobias Heider <tobias.heider@stusta.de>
@@ -132,6 +132,7 @@ enum imsg_type {
 	IMSG_CTL_SHOW_CERTSTORE,
 	IMSG_CTL_SHOW_STATS,
 	IMSG_CTL_PROCFD,
+	IMSG_CTL_PROCREADY,
 };
 
 enum privsep_procid {