aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorRoy Marples <roy@marples.name>2008-02-02 00:17:35 +0000
committerRoy Marples <roy@marples.name>2008-02-02 00:17:35 +0000
commitad045176238549c3267fff3e8c0014dd5e9ffbdf (patch)
tree8332746190f8e9fc47007090a3beac0a64d133c6
parentfef5d0af591c6c8a91f69bba5e62c99df1a732c9 (diff)
Block signals to avoid fork /signal races.
-rw-r--r--TODO2
-rw-r--r--src/includes/rc-misc.h3
-rw-r--r--src/librc/librc.c33
-rw-r--r--src/rc/rc-misc.c7
-rw-r--r--src/rc/rc-plugin.c158
-rw-r--r--src/rc/rc.c16
-rw-r--r--src/rc/runscript.c2
7 files changed, 136 insertions, 85 deletions
diff --git a/TODO b/TODO
index a3cdefff..d04785b9 100644
--- a/TODO
+++ b/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
diff --git a/src/includes/rc-misc.h b/src/includes/rc-misc.h
index 946754ee..1ffe48de 100644
--- a/src/includes/rc-misc.h
+++ b/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. */
diff --git a/src/librc/librc.c b/src/librc/librc.c
index dbbdbb5e..1cd3eec5 100644
--- a/src/librc/librc.c
+++ b/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);
diff --git a/src/rc/rc-misc.c b/src/rc/rc-misc.c
index 50101b43..59a52615 100644
--- a/src/rc/rc-misc.c
+++ b/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));
}
diff --git a/src/rc/rc-plugin.c b/src/rc/rc-plugin.c
index 29bd43c2..e4b4e0eb 100644
--- a/src/rc/rc-plugin.c
+++ b/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;
-
- /* 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;
- }
+ int i;
+ int flags;
+ int pfd[2];
+ pid_t pid;
+ char *buffer;
+ char *token;
+ char *p;
+ ssize_t nr;
+
+ 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 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;
- }
+ /* 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 (pid == 0) {
- int retval;
-
- 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;
-
- /* 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;
-
- 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++;
- }
- }
- }
+ /* 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));
+
+ sigprocmask (SIG_SETMASK, &full, &old);
+
+ /* 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;
+ }
- free (buffer);
- close (pfd[0]);
+ if (pid == 0) {
+ int retval;
+
+ /* 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;
+
+ /* Just in case the plugin sets this to false */
+ rc_in_plugin = true;
+ exit (retval);
+ }
- rc_waitpid (pid);
+ 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);
plugin = plugin->next;
}
}
diff --git a/src/rc/rc.c b/src/rc/rc.c
index f093b874..f87ddc95 100644
--- a/src/rc/rc.c
+++ b/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) {
diff --git a/src/rc/runscript.c b/src/rc/runscript.c
index dfabb86b..83f20868 100644
--- a/src/rc/runscript.c
+++ b/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);