]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
fs-util: Add xopenat_lock() 26936/head
authorDaan De Meyer <daan.j.demeyer@gmail.com>
Wed, 22 Mar 2023 16:04:36 +0000 (17:04 +0100)
committerDaan De Meyer <daan.j.demeyer@gmail.com>
Wed, 22 Mar 2023 20:54:20 +0000 (21:54 +0100)
open/create a file/directory and lock it using the given lock type.

src/basic/fs-util.c
src/basic/fs-util.h
src/basic/lock-util.c
src/basic/lock-util.h
src/test/test-fs-util.c

index 19ada73af5a75f9ea78086091c56ba2eb2bd00c0..06bd5705f28ea0dc4b0c7e8c178f4f4a78ae366e 100644 (file)
@@ -3,6 +3,7 @@
 #include <errno.h>
 #include <stddef.h>
 #include <stdlib.h>
+#include <sys/file.h>
 #include <linux/falloc.h>
 #include <linux/magic.h>
 #include <unistd.h>
@@ -13,6 +14,7 @@
 #include "fileio.h"
 #include "fs-util.h"
 #include "hostname-util.h"
+#include "lock-util.h"
 #include "log.h"
 #include "macro.h"
 #include "missing_fcntl.h"
@@ -1099,6 +1101,9 @@ int xopenat(int dir_fd, const char *path, int flags, mode_t mode) {
         bool made = false;
         int r;
 
+        assert(dir_fd >= 0 || dir_fd == AT_FDCWD);
+        assert(path);
+
         if (FLAGS_SET(flags, O_DIRECTORY|O_CREAT)) {
                 r = RET_NERRNO(mkdirat(dir_fd, path, mode));
                 if (r == -EEXIST) {
@@ -1135,3 +1140,42 @@ int xopenat(int dir_fd, const char *path, int flags, mode_t mode) {
 
         return TAKE_FD(fd);
 }
+
+int xopenat_lock(int dir_fd, const char *path, int flags, mode_t mode, LockType locktype, int operation) {
+        _cleanup_close_ int fd = -EBADF;
+        int r;
+
+        assert(dir_fd >= 0 || dir_fd == AT_FDCWD);
+        assert(path);
+        assert(IN_SET(operation & ~LOCK_NB, LOCK_EX, LOCK_SH));
+
+        /* POSIX/UNPOSIX locks don't work on directories (errno is set to -EBADF so let's return early with
+         * the same error here). */
+        if (FLAGS_SET(flags, O_DIRECTORY) && locktype != LOCK_BSD)
+                return -EBADF;
+
+        for (;;) {
+                struct stat st;
+
+                fd = xopenat(dir_fd, path, flags, mode);
+                if (fd < 0)
+                        return fd;
+
+                r = lock_generic(fd, locktype, operation);
+                if (r < 0)
+                        return r;
+
+                /* If we acquired the lock, let's check if the file/directory still exists in the file
+                 * system. If not, then the previous exclusive owner removed it and then closed it. In such a
+                 * case our acquired lock is worthless, hence try again. */
+
+                if (fstat(fd, &st) < 0)
+                        return -errno;
+                if (st.st_nlink > 0)
+                        break;
+
+                fd = safe_close(fd);
+        }
+
+        return TAKE_FD(fd);
+}
index d08e224aa1b61b3c25762ae2784af4e172489add..cf381dfc266d1ca7fbf1c30e9e37a60d2b5a15c4 100644 (file)
@@ -12,6 +12,7 @@
 
 #include "alloc-util.h"
 #include "errno-util.h"
+#include "lock-util.h"
 #include "time-util.h"
 #include "user-util.h"
 
@@ -132,3 +133,5 @@ int open_mkdir_at(int dirfd, const char *path, int flags, mode_t mode);
 int openat_report_new(int dirfd, const char *pathname, int flags, mode_t mode, bool *ret_newly_created);
 
 int xopenat(int dir_fd, const char *path, int flags, mode_t mode);
+
+int xopenat_lock(int dir_fd, const char *path, int flags, mode_t mode, LockType locktype, int operation);
index 46391bb5ccee7c3d366e4a1ad1af03ddda1e0a65..8ca30a50f0b70a11c9847e14f3a0c34d3a8753ff 100644 (file)
@@ -18,7 +18,6 @@
 int make_lock_file_at(int dir_fd, const char *p, int operation, LockFile *ret) {
         _cleanup_close_ int fd = -EBADF, dfd = -EBADF;
         _cleanup_free_ char *t = NULL;
-        int r;
 
         assert(dir_fd >= 0 || dir_fd == AT_FDCWD);
         assert(p);
@@ -38,28 +37,9 @@ int make_lock_file_at(int dir_fd, const char *p, int operation, LockFile *ret) {
         if (!t)
                 return -ENOMEM;
 
-        for (;;) {
-                struct stat st;
-
-                fd = openat(dfd, p, O_CREAT|O_RDWR|O_NOFOLLOW|O_CLOEXEC|O_NOCTTY, 0600);
-                if (fd < 0)
-                        return -errno;
-
-                r = unposix_lock(fd, operation);
-                if (r < 0)
-                        return r == -EAGAIN ? -EBUSY : r;
-
-                /* If we acquired the lock, let's check if the file still exists in the file system. If not,
-                 * then the previous exclusive owner removed it and then closed it. In such a case our
-                 * acquired lock is worthless, hence try again. */
-
-                if (fstat(fd, &st) < 0)
-                        return -errno;
-                if (st.st_nlink > 0)
-                        break;
-
-                fd = safe_close(fd);
-        }
+        fd = xopenat_lock(dfd, p, O_CREAT|O_RDWR|O_NOFOLLOW|O_CLOEXEC|O_NOCTTY, 0600, LOCK_UNPOSIX, operation);
+        if (fd < 0)
+                return fd == -EAGAIN ? -EBUSY : fd;
 
         *ret = (LockFile) {
                 .dir_fd = TAKE_FD(dfd),
@@ -183,3 +163,18 @@ void unposix_unlockpp(int **fd) {
         (void) fcntl_lock(**fd, LOCK_UN, /*ofd=*/ true);
         *fd = NULL;
 }
+
+int lock_generic(int fd, LockType type, int operation) {
+        assert(fd >= 0);
+
+        switch (type) {
+        case LOCK_BSD:
+                return RET_NERRNO(flock(fd, operation));
+        case LOCK_POSIX:
+                return posix_lock(fd, operation);
+        case LOCK_UNPOSIX:
+                return unposix_lock(fd, operation);
+        default:
+                assert_not_reached();
+        }
+}
index 3659f2b3a0d625dc427e33f78913529d32b624d1..e7744476bbeefadfdf95648ab84874fdedcc09c1 100644 (file)
@@ -32,3 +32,11 @@ void unposix_unlockpp(int **fd);
 
 #define CLEANUP_UNPOSIX_UNLOCK(fd)                                   \
         _cleanup_(unposix_unlockpp) _unused_ int *CONCATENATE(_cleanup_unposix_unlock_, UNIQ) = &(fd)
+
+typedef enum LockType {
+        LOCK_BSD,
+        LOCK_POSIX,
+        LOCK_UNPOSIX,
+} LockType;
+
+int lock_generic(int fd, LockType type, int operation);
index f115a40188f65d8149e1a20bee86fc840cb7d424..2ad9c5b565a38b206d7ec080bfa43fb898131f72 100644 (file)
@@ -13,6 +13,7 @@
 #include "macro.h"
 #include "mkdir.h"
 #include "path-util.h"
+#include "process-util.h"
 #include "random-util.h"
 #include "rm-rf.h"
 #include "stdio-util.h"
@@ -1221,8 +1222,8 @@ TEST(openat_report_new) {
 }
 
 TEST(xopenat) {
-        _cleanup_close_ int tfd = -EBADF, fd = -EBADF;
         _cleanup_(rm_rf_physical_and_freep) char *t = NULL;
+        _cleanup_close_ int tfd = -EBADF, fd = -EBADF;
 
         assert_se((tfd = mkdtemp_open(NULL, 0, &t)) >= 0);
 
@@ -1245,6 +1246,56 @@ TEST(xopenat) {
         fd = safe_close(fd);
 }
 
+TEST(xopenat_lock) {
+        _cleanup_(rm_rf_physical_and_freep) char *t = NULL;
+        _cleanup_close_ int tfd = -EBADF, fd = -EBADF;
+        siginfo_t si;
+
+        assert_se((tfd = mkdtemp_open(NULL, 0, &t)) >= 0);
+
+        /* Test that we can acquire an exclusive lock on a directory in one process, remove the directory,
+         * and close the file descriptor and still properly create the directory and acquire the lock in
+         * another process.  */
+
+        fd = xopenat_lock(tfd, "abc", O_CREAT|O_DIRECTORY|O_CLOEXEC, 0755, LOCK_BSD, LOCK_EX);
+        assert_se(fd >= 0);
+        assert_se(faccessat(tfd, "abc", F_OK, 0) >= 0);
+        assert_se(fd_verify_directory(fd) >= 0);
+        assert_se(xopenat_lock(tfd, "abc", O_DIRECTORY|O_CLOEXEC, 0755, LOCK_BSD, LOCK_EX|LOCK_NB) == -EAGAIN);
+
+        pid_t pid = fork();
+        assert_se(pid >= 0);
+
+        if (pid == 0) {
+                safe_close(fd);
+
+                fd = xopenat_lock(tfd, "abc", O_CREAT|O_DIRECTORY|O_CLOEXEC, 0755, LOCK_BSD, LOCK_EX);
+                assert_se(fd >= 0);
+                assert_se(faccessat(tfd, "abc", F_OK, 0) >= 0);
+                assert_se(fd_verify_directory(fd) >= 0);
+                assert_se(xopenat_lock(tfd, "abc", O_DIRECTORY|O_CLOEXEC, 0755, LOCK_BSD, LOCK_EX|LOCK_NB) == -EAGAIN);
+
+                _exit(EXIT_SUCCESS);
+        }
+
+        /* We need to give the child process some time to get past the xopenat() call in xopenat_lock() and
+         * block in the call to lock_generic() waiting for the lock to become free. We can't modify
+         * xopenat_lock() to signal an eventfd to let us know when that has happened, so we just sleep for a
+         * little and assume that's enough time for the child process to get along far enough. It doesn't
+         * matter if it doesn't get far enough, in that case we just won't trigger the fallback logic in
+         * xopenat_lock(), but the test will still succeed. */
+        assert_se(usleep(20 * USEC_PER_MSEC) >= 0);
+
+        assert_se(unlinkat(tfd, "abc", AT_REMOVEDIR) >= 0);
+        fd = safe_close(fd);
+
+        assert_se(wait_for_terminate(pid, &si) >= 0);
+        assert_se(si.si_code == CLD_EXITED);
+
+        assert_se(xopenat_lock(tfd, "abc", 0, 0755, LOCK_POSIX, LOCK_EX) == -EBADF);
+        assert_se(xopenat_lock(tfd, "def", O_DIRECTORY, 0755, LOCK_POSIX, LOCK_EX) == -EBADF);
+}
+
 static int intro(void) {
         arg_test_dir = saved_argv[1];
         return EXIT_SUCCESS;