From b177311aee0b3cf17af0e1760e18dfb60d99e024 Mon Sep 17 00:00:00 2001
From: Wayne Davison <wayne@opencoder.net>
Date: Thu, 4 Jun 2020 17:55:20 -0700
Subject: [PATCH] Use a lock to not fail on a left-over pid file.

---
 clientserver.c | 62 ++++++++++++++++++++++++++++++++++++++------------
 rsyncd.conf.yo |  7 +++---
 socket.c       |  3 +++
 3 files changed, 55 insertions(+), 17 deletions(-)

diff --git a/clientserver.c b/clientserver.c
index 3af97d84..d56f5d52 100644
--- a/clientserver.c
+++ b/clientserver.c
@@ -66,6 +66,7 @@ extern gid_t our_gid;
 char *auth_user;
 int read_only = 0;
 int module_id = -1;
+int pid_file_fd = -1;
 struct chmod_mode_struct *daemon_chmod_modes;
 
 /* module_dirlen is the length of the module_dir string when in daemon
@@ -1149,26 +1150,59 @@ int start_daemon(int f_in, int f_out)
 static void create_pid_file(void)
 {
 	char *pid_file = lp_pid_file();
-	char pidbuf[16];
-	pid_t pid = getpid();
-	int fd, len;
+	char pidbuf[32];
+	STRUCT_STAT st1, st2;
+	char *fail = NULL;
 
 	if (!pid_file || !*pid_file)
 		return;
 
-	cleanup_set_pid(pid);
-	if ((fd = do_open(pid_file, O_WRONLY|O_CREAT|O_EXCL, 0666)) == -1) {
-	  failure:
-		cleanup_set_pid(0);
-		fprintf(stderr, "failed to create pid file %s: %s\n", pid_file, strerror(errno));
-		rsyserr(FLOG, errno, "failed to create pid file %s", pid_file);
+	/* These tests make sure that a temp-style lock dir is handled safely. */
+	st1.st_mode = 0;
+	if (do_lstat(pid_file, &st1) == 0 && !S_ISREG(st1.st_mode) && unlink(pid_file) < 0)
+		fail = "unlink";
+	else if ((pid_file_fd = do_open(pid_file, O_RDWR|O_CREAT, 0664)) < 0)
+		fail = S_ISREG(st1.st_mode) ? "open" : "create";
+	else if (!lock_range(pid_file_fd, 0, 4))
+		fail = "lock";
+	else if (do_fstat(pid_file_fd, &st1) < 0)
+		fail = "fstat opened";
+	else if (st1.st_size >= (int)sizeof pidbuf)
+		fail = "find small";
+	else if (do_lstat(pid_file, &st2) < 0)
+		fail = "lstat";
+	else if (!S_ISREG(st1.st_mode))
+		fail = "avoid file overwrite race for";
+	else if (st1.st_dev != st2.st_dev || st1.st_ino != st2.st_ino)
+		fail = "verify stat info for";
+#ifdef HAVE_FTRUNCATE
+	else if (do_ftruncate(pid_file_fd, 0) < 0)
+		fail = "truncate";
+#endif
+	else {
+		pid_t pid = getpid();
+		int len = snprintf(pidbuf, sizeof pidbuf, "%d\n", (int)pid);
+#ifndef HAVE_FTRUNCATE
+		/* What can we do with a too-long file and no truncate? I guess we'll add extra newlines. */
+		while (len < st1.st_size) /* We already verfified that size+1 chars fits in the buffer. */
+			pidbuf[len++] = '\n';
+		/* We don't need the buffer to end in a '\0' (and we may not have room to add it). */
+#endif
+		if (write(pid_file_fd, pidbuf, len) != len)
+			 fail = "write";
+		cleanup_set_pid(pid); /* Mark the file for removal on exit, even if the write failed. */
+	}
+
+	if (fail) {
+		char msg[1024];
+		snprintf(msg, sizeof msg, "failed to %s pid file %s: %s\n",
+			fail, pid_file, strerror(errno));
+		fputs(msg, stderr);
+		rprintf(FLOG, "%s", msg);
 		exit_cleanup(RERR_FILEIO);
 	}
-	snprintf(pidbuf, sizeof pidbuf, "%d\n", (int)pid);
-	len = strlen(pidbuf);
-	if (write(fd, pidbuf, len) != len)
-		goto failure;
-	close(fd);
+
+	/* The file is left open so that the lock remains valid. It is closed in our forked child procs. */
 }
 
 /* Become a daemon, discarding the controlling terminal. */
diff --git a/rsyncd.conf.yo b/rsyncd.conf.yo
index aac4a7f2..c8338664 100644
--- a/rsyncd.conf.yo
+++ b/rsyncd.conf.yo
@@ -103,9 +103,10 @@ This can be overridden by the bf(--dparam=motdfile=FILE)
 command-line option when starting the daemon.
 
 dit(bf(pid file)) This parameter tells the rsync daemon to write
-its process ID to that file.  If the file already exists, the rsync
-daemon will abort rather than overwrite the file.
-This can be overridden by the bf(--dparam=pidfile=FILE)
+its process ID to that file.  The rsync keeps the file locked so that
+it can know when it is safe to overwrite an existing file.
+
+The filename can be overridden by the bf(--dparam=pidfile=FILE)
 command-line option when starting the daemon.
 
 dit(bf(port)) You can override the default port the daemon will listen on
diff --git a/socket.c b/socket.c
index 70fb1695..11ab4a83 100644
--- a/socket.c
+++ b/socket.c
@@ -38,6 +38,7 @@ extern char *bind_address;
 extern char *sockopts;
 extern int default_af_hint;
 extern int connect_timeout;
+extern int pid_file_fd;
 
 #ifdef HAVE_SIGACTION
 static struct sigaction sigact;
@@ -609,6 +610,8 @@ void start_accept_loop(int port, int (*fn)(int, int))
 
 		if ((pid = fork()) == 0) {
 			int ret;
+			if (pid_file_fd >= 0)
+				close(pid_file_fd);
 			for (i = 0; sp[i] >= 0; i++)
 				close(sp[i]);
 			/* Re-open log file in child before possibly giving
-- 
2.47.2