From 2646b86dd6a84f2e7ebc625203d44e6d1923c5bd Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Wed, 22 Mar 2023 17:04:36 +0100 Subject: [PATCH] fs-util: Add xopenat_lock() open/create a file/directory and lock it using the given lock type. --- src/basic/fs-util.c | 44 ++++++++++++++++++++++++++++++++++ src/basic/fs-util.h | 3 +++ src/basic/lock-util.c | 41 ++++++++++++++----------------- src/basic/lock-util.h | 8 +++++++ src/test/test-fs-util.c | 53 ++++++++++++++++++++++++++++++++++++++++- 5 files changed, 125 insertions(+), 24 deletions(-) diff --git a/src/basic/fs-util.c b/src/basic/fs-util.c index 19ada73af5a..06bd5705f28 100644 --- a/src/basic/fs-util.c +++ b/src/basic/fs-util.c @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -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); +} diff --git a/src/basic/fs-util.h b/src/basic/fs-util.h index d08e224aa1b..cf381dfc266 100644 --- a/src/basic/fs-util.h +++ b/src/basic/fs-util.h @@ -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); diff --git a/src/basic/lock-util.c b/src/basic/lock-util.c index 46391bb5cce..8ca30a50f0b 100644 --- a/src/basic/lock-util.c +++ b/src/basic/lock-util.c @@ -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(); + } +} diff --git a/src/basic/lock-util.h b/src/basic/lock-util.h index 3659f2b3a0d..e7744476bbe 100644 --- a/src/basic/lock-util.h +++ b/src/basic/lock-util.h @@ -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); diff --git a/src/test/test-fs-util.c b/src/test/test-fs-util.c index f115a40188f..2ad9c5b565a 100644 --- a/src/test/test-fs-util.c +++ b/src/test/test-fs-util.c @@ -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;