From 7d28108f4691e0e7df199fd89f386cafeb3466c7 Mon Sep 17 00:00:00 2001 From: Andre Noll Date: Thu, 16 Jun 2016 20:17:21 +0200 Subject: [PATCH] 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. --- daemon.c | 39 +++++++++++++++++++++++---------------- daemon.h | 2 +- server.c | 13 +++++++++---- 3 files changed, 33 insertions(+), 21 deletions(-) diff --git a/daemon.c b/daemon.c index 392d2b4e..20cdc6ae 100644 --- 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 c7153605..989678df 100644 --- 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 088cc8b1..ae8848b6 100644 --- 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"); } -- 2.30.2