Obtain advisory lock on ZED PID file

ZED uses an advisory lock on its state file to protect against
multiple instances running concurrently.  However, work is planned
to move this state information into the kernel, and ZED will still
need to protect against starting multiple instances.

This commit adds an advisory lock on the PID file to protect against
starting multiple instances.  A lock failure can be overridden with
the "-f" (force) command-line option.  The advisory lock on the state
file is being retained for as long as the state information is stored
in the state file.

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

3 files changed, 78 insertions, 33 deletions

diff --git a/cmd/zed/zed.c b/cmd/zed/zed.c
index b8dbc442c..2205f7367 100644
--- a/cmd/zed/zed.c
+++ b/cmd/zed/zed.c
@@ -259,7 +259,8 @@ main(int argc, char *argv[])
 	if (zcp->do_memlock)
 		_lock_memory();
 
-	(void) zed_conf_write_pid(zcp);
+	if ((zed_conf_write_pid(zcp) < 0) && (!zcp->do_force))
+		exit(EXIT_FAILURE);
 
 	if (!zcp->do_foreground)
 		_finish_daemonize();
diff --git a/cmd/zed/zed_conf.c b/cmd/zed/zed_conf.c
index 5e21a3db9..3f38945c0 100644
--- a/cmd/zed/zed_conf.c
+++ b/cmd/zed/zed_conf.c
@@ -58,6 +58,7 @@ zed_conf_create(void)
 	zcp->syslog_facility = LOG_DAEMON;
 	zcp->min_events = ZED_MIN_EVENTS;
 	zcp->max_events = ZED_MAX_EVENTS;
+	zcp->pid_fd = -1;
 	zcp->zedlets = NULL;		/* created via zed_conf_scan_dir() */
 	zcp->state_fd = -1;		/* opened via zed_conf_open_state() */
 	zcp->zfs_hdl = NULL;		/* opened via zed_event_init() */
@@ -105,6 +106,13 @@ zed_conf_destroy(struct zed_conf *zcp)
 			    "Failed to remove PID file \"%s\": %s",
 			    zcp->pid_file, strerror(errno));
 	}
+	if (zcp->pid_fd >= 0) {
+		if (close(zcp->pid_fd) < 0)
+			zed_log_msg(LOG_WARNING,
+			    "Failed to close PID file \"%s\": %s",
+			    zcp->pid_file, strerror(errno));
+		zcp->pid_fd = -1;
+	}
 	if (zcp->conf_file)
 		free(zcp->conf_file);
 
@@ -437,65 +445,101 @@ zed_conf_scan_dir(struct zed_conf *zcp)
  * This must be called after fork()ing to become a daemon (so the correct PID
  * is recorded), but before daemonization is complete and the parent process
  * exits (for synchronization with systemd).
- *
- * FIXME: Only update the PID file after verifying the PID previously stored
- * in the PID file no longer exists or belongs to a foreign process
- * in order to ensure the daemon cannot be started more than once.
- * (This check is currently done by zed_conf_open_state().)
  */
 int
 zed_conf_write_pid(struct zed_conf *zcp)
 {
-	char dirbuf[PATH_MAX];
-	mode_t dirmode = S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH;
+	const mode_t dirmode = S_IRWXU | S_IRGRP | S_IXGRP | S_IROTH | S_IXOTH;
+	const mode_t filemode = S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH;
+	char buf[PATH_MAX];
 	int n;
 	char *p;
 	mode_t mask;
-	FILE *fp;
+	int rv;
 
 	if (!zcp || !zcp->pid_file) {
 		errno = EINVAL;
-		zed_log_msg(LOG_ERR, "Failed to write PID file: %s",
+		zed_log_msg(LOG_ERR, "Failed to create PID file: %s",
 		    strerror(errno));
 		return (-1);
 	}
-	n = strlcpy(dirbuf, zcp->pid_file, sizeof (dirbuf));
-	if (n >= sizeof (dirbuf)) {
+	assert(zcp->pid_fd == -1);
+	/*
+	 * Create PID file directory if needed.
+	 */
+	n = strlcpy(buf, zcp->pid_file, sizeof (buf));
+	if (n >= sizeof (buf)) {
 		errno = ENAMETOOLONG;
-		zed_log_msg(LOG_WARNING, "Failed to write PID file: %s",
+		zed_log_msg(LOG_ERR, "Failed to create PID file: %s",
 		    strerror(errno));
-		return (-1);
+		goto err;
 	}
-	p = strrchr(dirbuf, '/');
+	p = strrchr(buf, '/');
 	if (p)
 		*p = '\0';
 
-	if ((mkdirp(dirbuf, dirmode) < 0) && (errno != EEXIST)) {
-		zed_log_msg(LOG_WARNING,
-		    "Failed to create directory \"%s\": %s",
-		    dirbuf, strerror(errno));
-		return (-1);
+	if ((mkdirp(buf, dirmode) < 0) && (errno != EEXIST)) {
+		zed_log_msg(LOG_ERR, "Failed to create directory \"%s\": %s",
+		    buf, strerror(errno));
+		goto err;
 	}
-	(void) unlink(zcp->pid_file);
-
+	/*
+	 * Obtain PID file lock.
+	 */
 	mask = umask(0);
 	umask(mask | 022);
-	fp = fopen(zcp->pid_file, "w");
+	zcp->pid_fd = open(zcp->pid_file, (O_RDWR | O_CREAT), filemode);
 	umask(mask);
-
-	if (!fp) {
-		zed_log_msg(LOG_WARNING, "Failed to open PID file \"%s\": %s",
+	if (zcp->pid_fd < 0) {
+		zed_log_msg(LOG_ERR, "Failed to open PID file \"%s\": %s",
+		    zcp->pid_file, strerror(errno));
+		goto err;
+	}
+	rv = zed_file_lock(zcp->pid_fd);
+	if (rv < 0) {
+		zed_log_msg(LOG_ERR, "Failed to lock PID file \"%s\": %s",
+		    zcp->pid_file, strerror(errno));
+		goto err;
+	} else if (rv > 0) {
+		pid_t pid = zed_file_is_locked(zcp->pid_fd);
+		if (pid < 0) {
+			zed_log_msg(LOG_ERR,
+			    "Failed to test lock on PID file \"%s\"",
+			    zcp->pid_file);
+		} else if (pid > 0) {
+			zed_log_msg(LOG_ERR,
+			    "Found PID %d bound to PID file \"%s\"",
+			    pid, zcp->pid_file);
+		} else {
+			zed_log_msg(LOG_ERR,
+			    "Inconsistent lock state on PID file \"%s\"",
+			    zcp->pid_file);
+		}
+		goto err;
+	}
+	/*
+	 * Write PID file.
+	 */
+	n = snprintf(buf, sizeof (buf), "%d\n", (int) getpid());
+	if ((n < 0) || (n >= sizeof (buf))) {
+		errno = ERANGE;
+		zed_log_msg(LOG_ERR, "Failed to write PID file \"%s\": %s",
 		    zcp->pid_file, strerror(errno));
-	} else if (fprintf(fp, "%d\n", (int) getpid()) == EOF) {
-		zed_log_msg(LOG_WARNING, "Failed to write PID file \"%s\": %s",
+	} else if (zed_file_write_n(zcp->pid_fd, buf, n) != n) {
+		zed_log_msg(LOG_ERR, "Failed to write PID file \"%s\": %s",
 		    zcp->pid_file, strerror(errno));
-	} else if (fclose(fp) == EOF) {
-		zed_log_msg(LOG_WARNING, "Failed to close PID file \"%s\": %s",
+	} else if (fdatasync(zcp->pid_fd) < 0) {
+		zed_log_msg(LOG_ERR, "Failed to sync PID file \"%s\": %s",
 		    zcp->pid_file, strerror(errno));
 	} else {
 		return (0);
 	}
-	(void) unlink(zcp->pid_file);
+
+err:
+	if (zcp->pid_fd >= 0) {
+		(void) close(zcp->pid_fd);
+		zcp->pid_fd = -1;
+	}
 	return (-1);
 }
 
@@ -503,8 +547,7 @@ zed_conf_write_pid(struct zed_conf *zcp)
  * Open and lock the [zcp] state_file.
  * Return 0 on success, -1 on error.
  *
- * FIXME: If state_file exists, verify ownership & permissions.
- * FIXME: Move lock to pid_file instead.
+ * FIXME: Move state information into kernel.
  */
 int
 zed_conf_open_state(struct zed_conf *zcp)
diff --git a/cmd/zed/zed_conf.h b/cmd/zed/zed_conf.h
index 126075842..20e04039b 100644
--- a/cmd/zed/zed_conf.h
+++ b/cmd/zed/zed_conf.h
@@ -42,6 +42,7 @@ struct zed_conf {
 	int		max_events;		/* RESERVED FOR FUTURE USE */
 	char		*conf_file;		/* abs path to config file */
 	char		*pid_file;		/* abs path to pid file */
+	int		pid_fd;			/* fd to pid file for lock */
 	char		*zedlet_dir;		/* abs path to zedlet dir */
 	zed_strings_t	*zedlets;		/* names of enabled zedlets */
 	char		*state_file;		/* abs path to state file */