From: Andre Noll <maan@tuebingen.mpg.de>
Date: Tue, 19 Sep 2023 14:31:49 +0000 (+0200)
Subject: Fix --config-file for relative paths.
X-Git-Url: http://git.tuebingen.mpg.de/?p=dss.git;a=commitdiff_plain

Fix --config-file for relative paths.

The dss lock works by first turning the given config file path
argument into a canonical absolute path using dss_realpath(), then
hashing this absolute path to obtain a key ID for semget(2).

If the given path is relative, we have to compute the ID before
changing to the destination directory because dss_realpath() needs to
call stat(2) to detect symlinks, and this system call will fail if the
current working directory has changed. This is currently not the case
as we change to the destination directory early in check_config().

If dss_realpath() fails, we silently use the unmodified path argument
for hashing to deal with the case that the default config does not
exist. As a result, if relative paths are given, the key ID depends
on whether or not change_to_dest_dir() was called. This is the case
for the run subcommanmd, but not for the kill subcommand. Thus the
kill subcommand does not work as expected if a relative path is given.

Fix this by grabbing the lock before changing the working directory
in all cases.
---

diff --git a/dss.c b/dss.c
index 84b6751..0992ec6 100644
--- a/dss.c
+++ b/dss.c
@@ -1122,7 +1122,6 @@ static int change_to_dest_dir(void)
 
 static int check_config(void)
 {
-	int ret;
 	uint32_t unit_interval = OPT_UINT32_VAL(DSS, UNIT_INTERVAL);
 	uint32_t num_intervals = OPT_UINT32_VAL(DSS, NUM_INTERVALS);
 
@@ -1147,9 +1146,6 @@ static int check_config(void)
 			DSS_ERROR_LOG(("--dest-dir required\n"));
 			return -E_SYNTAX;
 		}
-		ret = change_to_dest_dir();
-		if (ret < 0)
-			return ret;
 	}
 	DSS_DEBUG_LOG(("number of intervals: %i\n", num_intervals));
 	return 1;
@@ -1620,12 +1616,23 @@ static int com_run(void)
 		DSS_ERROR_LOG(("pid %d\n", (int)pid));
 		return -E_ALREADY_RUNNING;
 	}
+	/*
+	 * Order is important here: Since daemon_init() forks, it would drop
+	 * the lock if it had been acquired already. Changing the cwd before
+	 * grabbing the lock causes stat(2) to fail in case a relative config
+	 * file path was given, which results in a different key ID for
+	 * locking. Therefore we must first daemonize, then lock, then change
+	 * the cwd.
+	 */
 	if (OPT_GIVEN(RUN, DAEMON)) {
 		fd = daemon_init();
 		daemonized = true;
 		logfile = open_log(OPT_STRING_VAL(RUN, LOGFILE));
 	}
 	lock_dss_or_die();
+	ret = change_to_dest_dir();
+	if (ret < 0)
+		return ret;
 	dump_dss_config("startup");
 	ret = install_sighandler(SIGHUP);
 	if (ret < 0)
@@ -1661,6 +1668,9 @@ static int com_prune(void)
 	bool try_hard;
 
 	lock_dss_or_die();
+	ret = change_to_dest_dir();
+	if (ret < 0)
+		return ret;
 	switch (OPT_UINT32_VAL(PRUNE, DISK_SPACE)) {
 	case FDS_LOW: try_hard = true; break;
 	case FDS_HIGH: try_hard = false; break;
@@ -1717,6 +1727,9 @@ static int com_create(void)
 	char **rsync_argv;
 
 	lock_dss_or_die();
+	ret = change_to_dest_dir();
+	if (ret < 0)
+		return ret;
 	if (OPT_GIVEN(DSS, DRY_RUN)) {
 		int i;
 		char *msg = NULL;
@@ -1762,11 +1775,14 @@ EXPORT_CMD_HANDLER(create);
 
 static int com_ls(void)
 {
-	int i;
+	int i, ret;
 	struct snapshot_list sl;
 	struct snapshot *s;
 	int64_t now = get_current_time();
 
+	ret = change_to_dest_dir();
+	if (ret < 0)
+		return ret;
 	dss_get_snapshot_list(&sl);
 	FOR_EACH_SNAPSHOT(s, i, &sl) {
 		int64_t d;