From 8e8f56e933c8ef7a1b9d491332de403d4264d421 Mon Sep 17 00:00:00 2001 From: tobhe Date: Thu, 15 Feb 2024 20:10:45 +0000 Subject: [PATCH] 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@ --- sbin/iked/iked.c | 17 ++++++++++---- sbin/iked/iked.h | 6 +++-- sbin/iked/proc.c | 59 ++++++++++++++++++++++++++++++++++++++++++++--- sbin/iked/types.h | 3 ++- 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 @@ -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 @@ -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 @@ -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 @@ -132,6 +132,7 @@ enum imsg_type { IMSG_CTL_SHOW_CERTSTORE, IMSG_CTL_SHOW_STATS, IMSG_CTL_PROCFD, + IMSG_CTL_PROCREADY, }; enum privsep_procid { -- 2.20.1