Fix race condition with zed pidfile creation

When the zed is started as a forking daemon (by default),
a race-condition exists where the parent process can terminate before
the pidfile has been created by the grandchild process.  When invoked
as a Type=forking systemd service, this can result in the following:

  systemd[1]: Starting ZFS Event Daemon (zed)...
  systemd[1]: PID file /var/run/zed.pid not readable (yet?) after start.

This commit adds a daemonize pipe to allow the grandchild process to
signal the parent process that initialization is complete (and the
pidfile has been created).  The parent process will wait for this
notification before exiting.

Signed-off-by: Chris Dunlap <[email protected]>
Signed-off-by: Brian Behlendorf <[email protected]>
Issue #2252

1 files changed, 73 insertions, 20 deletions

diff --git a/cmd/zed/zed.c b/cmd/zed/zed.c
index c54a59b0a..bf29c0486 100644
--- a/cmd/zed/zed.c
+++ b/cmd/zed/zed.c
@@ -93,6 +93,9 @@ _setup_sig_handlers(void)
  * Lock all current and future pages in the virtual memory address space.
  *   Access to locked pages will never be delayed by a page fault.
  * EAGAIN is tested up to max_tries in case this is a transient error.
+ * Note that memory locks are not inherited by a child created via fork()
+ *   and are automatically removed during an execve().  As such, this must
+ *   be called after the daemon fork()s (when running in the background).
  */
 static void
 _lock_memory(void)
@@ -117,25 +120,57 @@ _lock_memory(void)
 }
 
 /*
- * Transform the process into a daemon.
+ * Start daemonization of the process including the double fork().
+ * The parent process will block here until _finish_daemonize() is called
+ *   (in the grandchild process), at which point the parent process will exit.
+ *   This prevents the parent process from exiting until initialization is
+ *   complete.
  */
 static void
-_become_daemon(void)
+_start_daemonize(void)
 {
 	pid_t pid;
-	int fd;
+	struct sigaction sa;
+
+	/* Create pipe for communicating with child during daemonization. */
+	zed_log_pipe_open();
 
+	/* Background process and ensure child is not process group leader. */
 	pid = fork();
 	if (pid < 0) {
 		zed_log_die("Failed to create child process: %s",
 		    strerror(errno));
 	} else if (pid > 0) {
+
+		/* Close writes since parent will only read from pipe. */
+		zed_log_pipe_close_writes();
+
+		/* Wait for notification that daemonization is complete. */
+		zed_log_pipe_wait();
+
+		zed_log_pipe_close_reads();
 		_exit(EXIT_SUCCESS);
 	}
+
+	/* Close reads since child will only write to pipe. */
+	zed_log_pipe_close_reads();
+
+	/* Create independent session and detach from terminal. */
 	if (setsid() < 0)
 		zed_log_die("Failed to create new session: %s",
 		    strerror(errno));
 
+	/* Prevent child from terminating on HUP when session leader exits. */
+	if (sigemptyset(&sa.sa_mask) < 0)
+		zed_log_die("Failed to initialize sigset");
+
+	sa.sa_flags = 0;
+	sa.sa_handler = SIG_IGN;
+
+	if (sigaction(SIGHUP, &sa, NULL) < 0)
+		zed_log_die("Failed to ignore SIGHUP");
+
+	/* Ensure process cannot re-acquire terminal. */
 	pid = fork();
 	if (pid < 0) {
 		zed_log_die("Failed to create grandchild process: %s",
@@ -143,25 +178,40 @@ _become_daemon(void)
 	} else if (pid > 0) {
 		_exit(EXIT_SUCCESS);
 	}
-	fd = open("/dev/null", O_RDWR);
+}
+
+/*
+ * Finish daemonization of the process by closing stdin/stdout/stderr.
+ * This must be called at the end of initialization after all external
+ *   communication channels are established and accessible.
+ */
+static void
+_finish_daemonize(void)
+{
+	int devnull;
 
-	if (fd < 0)
+	/* Preserve fd 0/1/2, but discard data to/from stdin/stdout/stderr. */
+	devnull = open("/dev/null", O_RDWR);
+	if (devnull < 0)
 		zed_log_die("Failed to open /dev/null: %s", strerror(errno));
 
-	if (dup2(fd, STDIN_FILENO) < 0)
+	if (dup2(devnull, STDIN_FILENO) < 0)
 		zed_log_die("Failed to dup /dev/null onto stdin: %s",
 		    strerror(errno));
 
-	if (dup2(fd, STDOUT_FILENO) < 0)
+	if (dup2(devnull, STDOUT_FILENO) < 0)
 		zed_log_die("Failed to dup /dev/null onto stdout: %s",
 		    strerror(errno));
 
-	if (dup2(fd, STDERR_FILENO) < 0)
+	if (dup2(devnull, STDERR_FILENO) < 0)
 		zed_log_die("Failed to dup /dev/null onto stderr: %s",
 		    strerror(errno));
 
-	if (close(fd) < 0)
+	if (close(devnull) < 0)
 		zed_log_die("Failed to close /dev/null: %s", strerror(errno));
+
+	/* Notify parent that daemonization is complete. */
+	zed_log_pipe_close_writes();
 }
 
 /*
@@ -184,33 +234,36 @@ main(int argc, char *argv[])
 	if (geteuid() != 0)
 		zed_log_die("Must be run as root");
 
-	(void) umask(0);
-
-	_setup_sig_handlers();
-
 	zed_conf_parse_file(zcp);
 
 	zed_file_close_from(STDERR_FILENO + 1);
 
+	(void) umask(0);
+
 	if (chdir("/") < 0)
 		zed_log_die("Failed to change to root directory");
 
 	if (zed_conf_scan_dir(zcp) < 0)
 		exit(EXIT_FAILURE);
 
-	if (zcp->do_memlock)
-		_lock_memory();
-
 	if (!zcp->do_foreground) {
-		_become_daemon();
+		_start_daemonize();
 		zed_log_syslog_open(LOG_DAEMON);
-		zed_log_stderr_close();
 	}
-	zed_log_msg(LOG_NOTICE,
-	    "ZFS Event Daemon %s-%s", ZFS_META_VERSION, ZFS_META_RELEASE);
+	_setup_sig_handlers();
+
+	if (zcp->do_memlock)
+		_lock_memory();
 
 	(void) zed_conf_write_pid(zcp);
 
+	if (!zcp->do_foreground)
+		_finish_daemonize();
+
+	zed_log_msg(LOG_NOTICE,
+	    "ZFS Event Daemon %s-%s (PID %d)",
+	    ZFS_META_VERSION, ZFS_META_RELEASE, (int) getpid());
+
 	if (zed_conf_open_state(zcp) < 0)
 		exit(EXIT_FAILURE);