Block signals to avoid fork /signal races.

TODO

@@ -1,3 +1,3 @@
-- stop using signal() and convert everything to sigaction()
+- ensure all forks block, restore and unblock signals. needs review 
 
 - add support somehow for optional translations

src/includes/rc-misc.h

@@ -34,7 +34,6 @@
 
 #include <sys/stat.h>
 #include <errno.h>
-#include <signal.h>
 #include <stdbool.h>
 #include <string.h>
 
@@ -134,7 +133,7 @@ bool rc_conf_yesno (const char *var);
 char **env_filter (void);
 char **env_config (void);
 bool service_plugable (const char *service);
-void signal_setup (int sig, void (*handler)(int));
+int signal_setup (int sig, void (*handler)(int));
 
 /* basename_c never modifies the argument. As such, if there is a trailing
  * slash then an empty string is returned. */

src/librc/librc.c

@@ -32,6 +32,7 @@
 const char librc_copyright[] = "Copyright (c) 2007-2008 Roy Marples";
 
 #include "librc.h"
+#include <signal.h>
 
 #define SOFTLEVEL	RC_SVCDIR "/softlevel"
 
@@ -575,6 +576,10 @@ static pid_t _exec_service (const char *service, const char *arg)
 	char *file;
 	char *fifo;
 	pid_t pid = -1;
+	sigset_t empty;
+	sigset_t full;
+	sigset_t old;
+	struct sigaction sa;
 
 	file = rc_service_resolve (service);
 	if (! exists (file)) {
@@ -593,13 +598,37 @@ static pid_t _exec_service (const char *service, const char *arg)
 		return (-1);
 	}
 
-	if ((pid = vfork ()) == 0) {
+	/* We need to block signals until we have forked */
+	memset (&sa, 0, sizeof (sa));
+	sa.sa_handler = SIG_DFL;
+	sigemptyset (&sa.sa_mask);
+	sigemptyset (&empty);
+	sigfillset (&full);
+	sigprocmask (SIG_SETMASK, &full, &old);
+
+	if ((pid = fork ()) == 0) {
+		/* Restore default handlers */
+		sigaction (SIGCHLD, &sa, NULL);
+		sigaction (SIGHUP,  &sa, NULL);
+		sigaction (SIGINT,  &sa, NULL);
+		sigaction (SIGQUIT, &sa, NULL);
+		sigaction (SIGTERM, &sa, NULL);
+		sigaction (SIGUSR1, &sa, NULL);
+		sigaction (SIGWINCH, &sa, NULL);
+
+		/* Unmask signals */
+		sigprocmask (SIG_SETMASK, &empty, NULL);
+
+		/* Safe to run now */
 		execl (file, file, arg, (char *) NULL);
-		fprintf (stderr, "unable to exec `%s': %s\n", file, strerror (errno));
+		fprintf (stderr, "unable to exec `%s': %s\n",
+			 file, strerror (errno));
 		unlink (fifo);
 		_exit (EXIT_FAILURE);
 	}
 
+	sigprocmask (SIG_SETMASK, &old, NULL);
+
 	free (fifo);
 	free (file);
 

src/rc/rc-misc.c

@@ -432,13 +432,12 @@ bool service_plugable (const char *service)
 	return (allow);
 }
 
-void signal_setup (int sig, void (*handler)(int))
+int signal_setup (int sig, void (*handler)(int))
 {
 	struct sigaction sa;
 
 	memset (&sa, 0, sizeof (sa));
-	sa.sa_handler = handler;
 	sigemptyset (&sa.sa_mask);
-	if (sigaction (sig, &sa, NULL) == -1)
-		eerrorx ("sigaction: %s", strerror (errno));
+	sa.sa_handler = handler;
+	return (sigaction (sig, &sa, NULL));
 }

src/rc/rc-plugin.c

@@ -143,84 +143,110 @@ int rc_waitpid (pid_t pid)
 void rc_plugin_run (rc_hook_t hook, const char *value)
 {
 	plugin_t *plugin = plugins;
+	struct sigaction sa;
+	sigset_t empty;
+	sigset_t full;
+	sigset_t old;
 
 	/* Don't run plugins if we're in one */
 	if (rc_in_plugin)
 		return;
 
+	/* We need to block signals until we have forked */
+	memset (&sa, 0, sizeof (sa));
+	sa.sa_handler = SIG_DFL;
+	sigemptyset (&sa.sa_mask);
+	sigemptyset (&empty);
+	sigfillset (&full);
+
 	while (plugin) {
-		if (plugin->hook) {
-			int i;
-			int flags;
-			int pfd[2];
-			pid_t pid;
+		int i;
+		int flags;
+		int pfd[2];
+		pid_t pid;
+		char *buffer;
+		char *token;
+		char *p;
+		ssize_t nr;
 
-			/* We create a pipe so that plugins can affect our environment
-			 * vars, which in turn influence our scripts. */
-			if (pipe (pfd) == -1) {
-				eerror ("pipe: %s", strerror (errno));
-				return;
-			}
+		if (! plugin->hook) {
+			plugin = plugin->next;
+			continue;
+		}
 
-			/* Stop any scripts from inheriting us.
-			 * This is actually quite important as without this, the splash
-			 * plugin will probably hang when running in silent mode. */
-			for (i = 0; i < 2; i++)
-				if ((flags = fcntl (pfd[i], F_GETFD, 0)) < 0 ||
-				    fcntl (pfd[i], F_SETFD, flags | FD_CLOEXEC) < 0)
-					eerror ("fcntl: %s", strerror (errno));
+		/* We create a pipe so that plugins can affect our environment
+		 * vars, which in turn influence our scripts. */
+		if (pipe (pfd) == -1) {
+			eerror ("pipe: %s", strerror (errno));
+			return;
+		}
 
-			/* We run the plugin in a new process so we never crash
-			 * or otherwise affected by it */
-			if ((pid = fork ()) == -1) {
-				eerror ("fork: %s", strerror (errno));
-				return;
-			}
+		/* Stop any scripts from inheriting us.
+		 * This is actually quite important as without this, the splash
+		 * plugin will probably hang when running in silent mode. */
+		for (i = 0; i < 2; i++)
+			if ((flags = fcntl (pfd[i], F_GETFD, 0)) < 0 ||
+			    fcntl (pfd[i], F_SETFD, flags | FD_CLOEXEC) < 0)
+				eerror ("fcntl: %s", strerror (errno));
 
-			if (pid == 0) {
-				int retval;
+		sigprocmask (SIG_SETMASK, &full, &old);
 
-				rc_in_plugin = true;
-				close (pfd[0]);
-				rc_environ_fd = fdopen (pfd[1], "w");
-				retval = plugin->hook (hook, value);
-				fclose (rc_environ_fd);
-				rc_environ_fd = NULL;
+		/* We run the plugin in a new process so we never crash
+		 * or otherwise affected by it */
+		if ((pid = fork ()) == -1) {
+			eerror ("fork: %s", strerror (errno));
+			break;
+		}
 
-				/* Just in case the plugin sets this to false */
-				rc_in_plugin = true;
-				exit (retval);
-			} else {
-				char *buffer;
-				char *token;
-				char *p;
-				ssize_t nr;
+		if (pid == 0) {
+			int retval;
 
-				close (pfd[1]);
-				buffer = xmalloc (sizeof (char) * BUFSIZ);
-				memset (buffer, 0, BUFSIZ);
+			/* Restore default handlers */
+			sigaction (SIGCHLD, &sa, NULL);
+			sigaction (SIGHUP,  &sa, NULL);
+			sigaction (SIGINT,  &sa, NULL);
+			sigaction (SIGQUIT, &sa, NULL);
+			sigaction (SIGTERM, &sa, NULL);
+			sigaction (SIGUSR1, &sa, NULL);
+			sigaction (SIGWINCH, &sa, NULL);
+			sigprocmask (SIG_SETMASK, &old, NULL);
+			
+			rc_in_plugin = true;
+			close (pfd[0]);
+			rc_environ_fd = fdopen (pfd[1], "w");
+			retval = plugin->hook (hook, value);
+			fclose (rc_environ_fd);
+			rc_environ_fd = NULL;
 
-				while ((nr = read (pfd[0], buffer, BUFSIZ)) > 0) {
-					p = buffer;
-					while (*p && p - buffer < nr) {
-						token = strsep (&p, "=");
-						if (token) {
-							unsetenv (token);
-							if (*p) {
-								setenv (token, p, 1);
-								p += strlen (p) + 1;
-							} else
-								p++;
-						}
-					}
+			/* Just in case the plugin sets this to false */
+			rc_in_plugin = true;
+			exit (retval);
+		}
+
+		sigprocmask (SIG_SETMASK, &old, NULL);
+		close (pfd[1]);
+		buffer = xmalloc (sizeof (char) * BUFSIZ);
+		memset (buffer, 0, BUFSIZ);
+
+		while ((nr = read (pfd[0], buffer, BUFSIZ)) > 0) {
+			p = buffer;
+			while (*p && p - buffer < nr) {
+				token = strsep (&p, "=");
+				if (token) {
+					unsetenv (token);
+					if (*p) {
+						setenv (token, p, 1);
+						p += strlen (p) + 1;
+					} else
+						p++;
 				}
-
-				free (buffer);
-				close (pfd[0]);
-
-				rc_waitpid (pid);
 			}
 		}
+
+		free (buffer);
+		close (pfd[0]);
+
+		rc_waitpid (pid);
 		plugin = plugin->next;
 	}
 }

src/rc/rc.c

@@ -663,6 +663,7 @@ int main (int argc, char **argv)
 	int opt;
 	bool parallel;
 	int regen = 0;
+	pid_t pid;
 
 	applet = basename_c (argv[0]);
 	atexit (cleanup);
@@ -994,7 +995,7 @@ int main (int argc, char **argv)
 
 		/* We always stop the service when in these runlevels */
 		if (going_down) {
-			pid_t pid = rc_service_stop (service);
+			pid = rc_service_stop (service);
 			if (pid > 0 && ! parallel)
 				rc_waitpid (pid);
 			continue;
@@ -1060,11 +1061,10 @@ int main (int argc, char **argv)
 
 		/* After all that we can finally stop the blighter! */
 		if (! found) {
-			pid_t pid;
+			pid = rc_service_stop (service);
 
-			if ((pid = rc_service_stop (service)) > 0) {
+			if (pid > 0) {
 				add_pid (pid);
-
 				if (! parallel) {
 					rc_waitpid (pid);
 					remove_pid (pid);
@@ -1136,9 +1136,7 @@ int main (int argc, char **argv)
 #endif
 
 	STRLIST_FOREACH (start_services, service, i) {
-		if (rc_service_state (service) & RC_SERVICE_STOPPED)	{
-			pid_t pid;
-
+		if (rc_service_state (service) & RC_SERVICE_STOPPED) {
 			if (! interactive)
 				interactive = want_interactive ();
 
@@ -1160,8 +1158,10 @@ interactive_option:
 				}
 			}
 
+			pid = rc_service_start (service);
+			
 			/* Remember the pid if we're running in parallel */
-			if ((pid = rc_service_start (service)) > 0) {
+			if (pid > 0) {
 				add_pid (pid);
 
 				if (! parallel) {

src/rc/runscript.c

@@ -432,8 +432,6 @@ static bool svc_exec (const char *arg1, const char *arg2)
 		eerrorx ("%s: vfork: %s", service, strerror (errno));
 	if (service_pid == 0) {
 		if (slave_tty >= 0) {
-			/* Hmmm, this shouldn't work in a vfork, but it does which is
-			 * good for us */
 			close (master_tty);
 
 			dup2 (slave_tty, 1);