daemon: Fix race condition in daemonize().

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.

diff --git a/daemon.c b/daemon.c
index 392d2b4e02f654126fe69249390a9caad429dbc5..20cdc6ae605a99f56f35bf41f0abeff7d508936d 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);

diff --git a/daemon.h b/daemon.h
index c715360583d350275641042b574708fd0cd2ec04..989678df40e6b29a44ae8fb5491db05841d25bca 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);

diff --git a/server.c b/server.c
index 088cc8b124448123ed6f4cb73f0226f584dffc25..ae8848b6856732ae7d1d7fb034953f64b682aad1 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");
 }