util: Move program locking out of dispatch_commands()

The dispatch_commands() helper takes a program lock and holds it while the
command is being executed. This was added for the use of xdp_filter, and is
really only needed for that; none of the other utilities really need
locking to synchronise with itself (and libxdp has its own locking for the
dispatcher).

Move the locking out of dispatch_commands() and instead add explicit lock
and unlock calls to each of the xdp-filter commands. While doing so, also
move the locking to use the same directory-fd based approach as the libxdp
locking uses. This has the benefit of not writing any lock files, so the
lock is automatically cleared when the application exits, even if it
crashes. This should fix any issues with stale lock files sticking around,
which we did not have any good way to clean up.

Signed-off-by: Toke Høiland-Jørgensen <toke@redhat.com>

lib/util/params.c

@@ -797,11 +797,7 @@ int dispatch_commands(const char *argv0, int argc, char **argv,
 	if (err)
 		goto out;
 
-	if (prog_lock_get(prog_name))
-		goto out;
-
 	ret = cmd->func(cfg, pin_root_path);
-	prog_lock_release(0);
 out:
 	free(cfg);
 	return ret;

lib/util/util.c

@@ -6,6 +6,7 @@
 #include <string.h>
 #include <fcntl.h>
 #include <signal.h>
+#include <sys/file.h>
 #include <sys/types.h>
 #include <sys/resource.h>
 #include <sys/vfs.h>
@@ -711,139 +712,46 @@ int check_bpf_environ(void)
 	return 0;
 }
 
-static const char *lock_dir = RUNDIR;
-static char *prog_lock_file = NULL;
-static int prog_lock_fd = -1;
-static pid_t prog_pid = 0;
 
-void prog_lock_release(int signal)
+int prog_lock_acquire(const char *dir)
 {
-	struct sigaction sigact = { .sa_flags = SA_RESETHAND };
-	int err;
-
-	if (prog_lock_fd < 0 || !prog_lock_file)
-		return;
+	int lock_fd, err = 0;
+retry:
+	lock_fd = open(dir, O_DIRECTORY);
+	if (lock_fd < 0) {
+		if (errno == ENOENT && !mkdir(dir, S_IRWXU))
+			goto retry;
 
-	sigaction(SIGHUP, &sigact, NULL);
-	sigaction(SIGINT, &sigact, NULL);
-	sigaction(SIGSEGV, &sigact, NULL);
-	sigaction(SIGFPE, &sigact, NULL);
-	sigaction(SIGTERM, &sigact, NULL);
+		err = -errno;
+		pr_warn("Couldn't open lock directory at %s: %s\n",
+			dir, strerror(-err));
+		return err;
+	}
 
-	err = unlink(prog_lock_file);
+	err = flock(lock_fd, LOCK_EX);
 	if (err) {
 		err = -errno;
-		pr_warn("Unable to unlink lock file: %s\n", strerror(-err));
-		goto out;
+		pr_warn("Couldn't flock fd %d: %s\n", lock_fd, strerror(-err));
+		close(lock_fd);
+		return err;
 	}
 
-	close(prog_lock_fd);
-	free(prog_lock_file);
-	prog_lock_fd = -1;
-	prog_lock_file = NULL;
-
-out:
-	if (signal) {
-		pr_debug("Exiting on signal %d\n", signal);
-		if (prog_pid)
-			kill(prog_pid, signal);
-		else
-			exit(signal);
-	}
+	pr_debug("Acquired lock from %s with fd %d\n", dir, lock_fd);
+	return lock_fd;
 }
 
-int prog_lock_get(const char *progname)
+int prog_lock_release(int lock_fd)
 {
-	char buf[PATH_MAX];
 	int err;
-	struct sigaction sigact = { .sa_handler = prog_lock_release };
-
-	if (prog_lock_fd >= 0) {
-		pr_warn("Attempt to get prog_lock twice.\n");
-		return -EFAULT;
-	}
-
-	if (!prog_lock_file) {
-		err = try_snprintf(buf, sizeof(buf), "%s/%s.lck", lock_dir,
-				   progname);
-		if (err)
-			return err;
-
-		prog_lock_file = strdup(buf);
-		if (!prog_lock_file)
-			return -ENOMEM;
-	}
-
-	prog_pid = getpid();
-
-	if (sigaction(SIGHUP, &sigact, NULL) ||
-	    sigaction(SIGINT, &sigact, NULL) ||
-	    sigaction(SIGSEGV, &sigact, NULL) ||
-	    sigaction(SIGFPE, &sigact, NULL) ||
-	    sigaction(SIGTERM, &sigact, NULL)) {
-		err = -errno;
-		pr_warn("Unable to install signal handler: %s\n", strerror(-err));
-		return err;
-	}
-
-	prog_lock_fd = open(prog_lock_file, O_WRONLY | O_CREAT | O_EXCL, 0644);
-	if (prog_lock_fd < 0) {
-		err = -errno;
-		if (err == -EEXIST) {
-			pid_t pid = 0;
-			char buf[100];
-			ssize_t len;
-			int fd;
-
-			fd = open(prog_lock_file, O_RDONLY);
-			if (fd < 0) {
-				err = -errno;
-				pr_warn("Unable to open lockfile for reading: %s\n",
-					strerror(-err));
-				return err;
-			}
 
-			len = read(fd, buf, sizeof(buf) - 1);
-			err = -errno;
-			close(fd);
-			if (len > 0) {
-				buf[len] = '\0';
-				pid = strtoul(buf, NULL, 10);
-			}
-			if (!pid || err) {
-				pr_warn("Unable to read PID from lockfile: %s\n",
-					strerror(-err));
-				return err;
-			}
-			pr_warn("Unable to get program lock: Already held by pid %d\n",
-				pid);
-		} else {
-			pr_warn("Unable to get program lock: %s\n", strerror(-err));
-		}
-		return err;
-	}
-
-	err = dprintf(prog_lock_fd, "%d\n", prog_pid);
-	if (err < 0) {
-		err = -errno;
-		pr_warn("Unable to write pid to lock file: %s\n", strerror(-err));
-		goto out_err;
-	}
-
-	err = fsync(prog_lock_fd);
+	err = flock(lock_fd, LOCK_UN);
 	if (err) {
 		err = -errno;
-		pr_warn("Unable fsync() lock file: %s\n", strerror(-err));
-		goto out_err;
+		pr_warn("Couldn't unlock fd %d: %s\n", lock_fd, strerror(-err));
+	} else {
+		pr_debug("Released lock fd %d\n", lock_fd);
 	}
-
-	return 0;
-out_err:
-	unlink(prog_lock_file);
-	close(prog_lock_fd);
-	free(prog_lock_file);
-	prog_lock_file = NULL;
-	prog_lock_fd = -1;
+	close(lock_fd);
 	return err;
 }
 

lib/util/util.h

@@ -88,8 +88,8 @@ int unlink_pinned_map(int dir_fd, const char *map_name);
 
 const char *action2str(__u32 action);
 
-int prog_lock_get(const char *progname);
-void prog_lock_release(int signal);
+int prog_lock_acquire(const char *directory);
+int prog_lock_release(int lock_fd);
 
 const char *get_libbpf_version(void);
 int iface_print_status(const struct iface *iface);