fix signal race, when child dies before the monitor

sets it's signal handlers. ok hshoexer@ ho@

2 files changed, 56 insertions, 29 deletions

diff --git a/sbin/isakmpd/isakmpd.c b/sbin/isakmpd/isakmpd.c
index b26aac9154d..40d96097e68 100644
--- a/sbin/isakmpd/isakmpd.c
+++ b/sbin/isakmpd/isakmpd.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: isakmpd.c,v 1.85 2005/04/10 14:17:49 jmc Exp $	 */
+/* $OpenBSD: isakmpd.c,v 1.86 2005/05/03 13:09:45 moritz Exp $	 */
 /* $EOM: isakmpd.c,v 1.54 2000/10/05 09:28:22 niklas Exp $	 */
 
 /*
@@ -103,6 +103,7 @@ volatile sig_atomic_t sigusr2ed = 0;
  */
 volatile sig_atomic_t sigtermed = 0;
 void            daemon_shutdown_now(int);
+void		set_slave_signals(void);
 
 /* The default path of the PID file.  */
 static char    *pid_file = "/var/run/isakmpd.pid";
@@ -301,6 +302,32 @@ phase1_sa_check(struct sa *sa, void *arg)
 	return sa->phase == 1;
 }
 
+void
+set_slave_signals(void)
+{
+	int n;
+
+	for (n = 1; n < _NSIG; n++)
+		signal(n, SIG_DFL);
+
+	/*
+	 * Do a clean daemon shutdown on TERM/INT. These signals must be
+	 * initialized before monitor_init(). INT is only used with '-d'.
+         */
+	signal(SIGTERM, daemon_shutdown_now);
+	if (debug == 1)		/* i.e '-dd' will skip this.  */
+		signal(SIGINT, daemon_shutdown_now);
+
+	/* Reinitialize on HUP reception.  */
+	signal(SIGHUP, sighup);
+
+	/* Report state on USR1 reception.  */
+	signal(SIGUSR1, sigusr1);
+
+	/* Rehash soft expiration timers on USR2 reception.  */
+	signal(SIGUSR2, sigusr2);
+}
+
 static void
 daemon_shutdown(void)
 {
@@ -381,9 +408,6 @@ main(int argc, char *argv[])
 		if (fcntl(n, F_GETFL, 0) == -1 && errno == EBADF)
 			(void) open("/dev/null", n ? O_WRONLY : O_RDONLY, 0);
 
-	for (n = 1; n < _NSIG; n++)
-		signal(n, SIG_DFL);
-
 	/* Log cmd line parsing and initialization errors to stderr.  */
 	log_to(stderr);
 	parse_args(argc, argv);
@@ -393,14 +417,7 @@ main(int argc, char *argv[])
 	setprotoent(1);
 	setservent(1);
 
-	/*
-	 * Do a clean daemon shutdown on TERM/INT. These signals must be
-	 * initialized before monitor_init(). INT is only used with '-d'.
-         */
-	signal(SIGTERM, daemon_shutdown_now);
-	if (debug == 1)		/* i.e '-dd' will skip this.  */
-		signal(SIGINT, daemon_shutdown_now);
-
+	set_slave_signals();
 	/* Daemonize before forking unpriv'ed child */
 	if (!debug)
 		if (daemon(0, 0))
@@ -420,15 +437,6 @@ main(int argc, char *argv[])
 
 	write_pid_file();
 
-	/* Reinitialize on HUP reception.  */
-	signal(SIGHUP, sighup);
-
-	/* Report state on USR1 reception.  */
-	signal(SIGUSR1, sigusr1);
-
-	/* Rehash soft expiration timers on USR2 reception.  */
-	signal(SIGUSR2, sigusr2);
-
 	/* If we wanted IKE packet capture to file, initialize it now.  */
 	if (pcap_file != 0)
 		log_packet_init(pcap_file);
diff --git a/sbin/isakmpd/monitor.c b/sbin/isakmpd/monitor.c
index 72700238312..007dc86a53b 100644
--- a/sbin/isakmpd/monitor.c
+++ b/sbin/isakmpd/monitor.c
@@ -1,4 +1,4 @@
-/* $OpenBSD: monitor.c,v 1.40 2005/04/19 15:46:49 hshoexer Exp $	 */
+/* $OpenBSD: monitor.c,v 1.41 2005/05/03 13:09:45 moritz Exp $	 */
 
 /*
  * Copyright (c) 2003 Håkan Olsson.  All rights reserved.
@@ -62,6 +62,8 @@ volatile sig_atomic_t sigchlded = 0;
 extern volatile sig_atomic_t sigtermed;
 static volatile sig_atomic_t cur_state = STATE_INIT;
 
+extern void	set_slave_signals(void);
+
 /* Private functions.  */
 int             m_write_int32(int, int32_t);
 int             m_write_raw(int, const char *, size_t);
@@ -82,6 +84,10 @@ static void     m_priv_test_state(int);
 static void	m_priv_ui_init(int);
 static void	m_priv_pfkey_open(int);
 
+static void	set_monitor_signals(void);
+static void	monitor_got_sigchld(int);
+static void	sig_pass_to_chld(int);
+
 /*
  * Public functions, unprivileged.
  */
@@ -104,6 +110,7 @@ monitor_init(int debug)
 		    ISAKMPD_PRIVSEP_USER);
 	endpwent();
 
+	set_monitor_signals();
 	m_state.pid = fork();
 	m_state.s = p[m_state.pid ? 1 : 0];
 	strlcpy(m_state.root, pw->pw_dir, sizeof m_state.root);
@@ -113,6 +120,7 @@ monitor_init(int debug)
 
 	/* The child process should drop privileges now.  */
 	if (!m_state.pid) {
+		set_slave_signals();
 		if (chroot(pw->pw_dir) != 0 || chdir("/") != 0)
 			log_fatal("monitor_init: chroot failed");
 
@@ -501,6 +509,24 @@ monitor_init_done(void)
  * Start of code running with privileges (the monitor process).
  */
 
+static void
+set_monitor_signals(void)
+{
+	int n;
+
+	for (n = 0; n < _NSIG; n++)
+		signal(n, SIG_DFL);
+
+	/* If the child dies, we should shutdown also.  */
+	signal(SIGCHLD, monitor_got_sigchld);
+
+	/* Forward some signals to the child. */
+	signal(SIGTERM, sig_pass_to_chld);
+	signal(SIGHUP, sig_pass_to_chld);
+	signal(SIGUSR1, sig_pass_to_chld);
+	signal(SIGUSR2, sig_pass_to_chld);
+}
+
 /* Help functions for monitor_loop().  */
 /* ARGSUSED */
 static void
@@ -542,13 +568,6 @@ monitor_loop(int debug)
 		    (unsigned long)fdsn);
 		return;
 	}
-	/* If the child dies, we should shutdown also.  */
-	signal(SIGCHLD, monitor_got_sigchld);
-
-	/* SIGHUP, SIGUSR1 and SIGUSR2 will be forwarded to child. */
-	signal(SIGHUP, sig_pass_to_chld);
-	signal(SIGUSR1, sig_pass_to_chld);
-	signal(SIGUSR2, sig_pass_to_chld);
 
 	while (cur_state < STATE_QUIT) {
 		/*