From 56697c42646f002c4f52f9569a8cb75b45f704d8 Mon Sep 17 00:00:00 2001 From: Chris Dunlap Date: Wed, 1 Oct 2014 14:56:07 -0700 Subject: [PATCH] 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 Signed-off-by: Brian Behlendorf Issue #2756 --- cmd/zed/zed.c | 3 +- cmd/zed/zed_conf.c | 107 +++++++++++++++++++++++++++++++-------------- cmd/zed/zed_conf.h | 1 + 3 files changed, 78 insertions(+), 33 deletions(-) diff --git a/cmd/zed/zed.c b/cmd/zed/zed.c index b8dbc442c0..2205f7367b 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 5e21a3db9b..3f38945c03 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)); - } else if (fprintf(fp, "%d\n", (int) getpid()) == EOF) { - zed_log_msg(LOG_WARNING, "Failed to write PID file \"%s\": %s", + 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)); - } else if (fclose(fp) == EOF) { - zed_log_msg(LOG_WARNING, "Failed to close PID file \"%s\": %s", + 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 (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 (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 126075842b..20e04039b0 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 */