daemon: Fix race condition in daemonize().
authorAndre Noll <maan@tuebingen.mpg.de>
Thu, 16 Jun 2016 18:17:21 +0000 (20:17 +0200)
committerAndre Noll <maan@tuebingen.mpg.de>
Tue, 4 Oct 2016 10:19:48 +0000 (12:19 +0200)
If parent_waits is true, the parent process waits for a signal from
the child before it exits. However, this signal can arive before the
parent has set up its signal handler.

This patch closes the race window by switching from signals to
pipes. We now create a pipe before the new process is forked, and
let the parent block on read(2) until the child exits or indicates
success by writing a byte to one end of the pipe. The child process
receives the file descriptor of the writing end of the pipe as the
return value of daemonize().

The only user of the parent_waits feature is para_server, which is
changed accordingly.

daemon.c
daemon.h
server.c

index 392d2b4..20cdc6a 100644 (file)
--- a/daemon.c
+++ b/daemon.c
@@ -155,10 +155,6 @@ static bool daemon_test_flag(unsigned flag)
        return me->flags & flag;
 }
 
-static void dummy_sighandler(__a_unused int s)
-{
-}
-
 /**
  * Do the usual stuff to become a daemon.
  *
@@ -166,31 +162,42 @@ static void dummy_sighandler(__a_unused int s)
  *
  * Fork, become session leader, cd to /, and dup fd 0, 1, 2 to /dev/null. If \a
  * parent_waits is false, the parent process terminates immediately.
- * Otherwise, it calls pause() to sleep until it receives \p SIGTERM or \p
- * SIGCHLD and exits successfully thereafter. This behaviour is useful if the
- * daemon process should not detach from the console until the child process
- * has completed its setup.
+ * Otherwise, a pipe is created prior to the fork() and the parent tries to
+ * read a single byte from the reading end of the pipe. The child is supposed
+ * to write to the writing end of the pipe after it completed its setup
+ * procedure successfully. This behaviour is useful to let the parent process
+ * die with an error if the child process aborts early, since in this case the
+ * read() will return non-positive.
+ *
+ * \return This function either succeeds or calls exit(3). If parent_waits is
+ * true, the return value is the file descriptor of the writing end of the
+ * pipe. Otherwise the function returns zero.
  *
  * \sa fork(2), setsid(2), dup(2), pause(2).
  */
-void daemonize(bool parent_waits)
+int daemonize(bool parent_waits)
 {
        pid_t pid;
-       int null;
+       int null, pipe_fd[2];
 
+       if (parent_waits && pipe(pipe_fd) < 0)
+               goto err;
        PARA_INFO_LOG("subsequent log messages go to %s\n", me->logfile_name?
                 me->logfile_name : "/dev/null");
        pid = fork();
        if (pid < 0)
                goto err;
-       if (pid) {
+       if (pid) { /* parent exits */
                if (parent_waits) {
-                       signal(SIGTERM, dummy_sighandler);
-                       signal(SIGCHLD, dummy_sighandler);
-                       pause();
+                       char c;
+                       close(pipe_fd[1]);
+                       exit(read(pipe_fd[0], &c, 1) <= 0?
+                               EXIT_FAILURE : EXIT_SUCCESS);
                }
-               exit(EXIT_SUCCESS); /* parent exits */
+               exit(EXIT_SUCCESS);
        }
+       if (parent_waits)
+               close(pipe_fd[0]);
        /* become session leader */
        if (setsid() < 0)
                goto err;
@@ -206,7 +213,7 @@ void daemonize(bool parent_waits)
        if (dup2(null, STDERR_FILENO) < 0)
                goto err;
        close(null);
-       return;
+       return parent_waits? pipe_fd[1] : 0;
 err:
        PARA_EMERG_LOG("fatal: %s\n", strerror(errno));
        exit(EXIT_FAILURE);
index c715360..989678d 100644 (file)
--- a/daemon.h
+++ b/daemon.h
@@ -1,7 +1,7 @@
 /** \file daemon.h exported symbols from daemon.c */
 
 
-void daemonize(bool parent_waits);
+int daemonize(bool parent_waits);
 void daemon_open_log_or_die(void);
 void daemon_close_log(void);
 void daemon_log_welcome(const char *whoami);
index 088cc8b..ae8848b 100644 (file)
--- a/server.c
+++ b/server.c
@@ -457,7 +457,7 @@ static void server_init(int argc, char **argv)
                .check_ambiguity = 0,
                .print_errors = 1
        };
-       int afs_socket;
+       int afs_socket, daemon_pipe = -1;
 
        valid_fd_012();
        init_random_seed_or_die();
@@ -477,7 +477,7 @@ static void server_init(int argc, char **argv)
        init_user_list(user_list_file);
        /* become daemon */
        if (conf.daemon_given)
-               daemonize(true /* parent waits for SIGTERM */);
+               daemon_pipe = daemonize(true /* parent waits for us */);
        PARA_NOTICE_LOG("initializing audio format handlers\n");
        afh_init();
 
@@ -503,8 +503,13 @@ static void server_init(int argc, char **argv)
        PARA_NOTICE_LOG("initializing virtual streaming system\n");
        init_vss_task(afs_socket, &sched);
        init_server_command_task(argc, argv);
-       if (conf.daemon_given)
-               kill(getppid(), SIGTERM);
+       if (daemon_pipe >= 0) {
+               if (write(daemon_pipe, "\0", 1) < 0) {
+                       PARA_EMERG_LOG("daemon_pipe: %s", strerror(errno));
+                       exit(EXIT_FAILURE);
+               }
+               close(daemon_pipe);
+       }
        PARA_NOTICE_LOG("server init complete\n");
 }