From 9420aaad2680981323b0786c479b14891444f5cb Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Tue, 22 Nov 2022 14:45:56 +0100 Subject: [PATCH 6000/6000] Revert "tmpfiles: whenever creating an inode, immediately O_PATH open it to pin it" This reverts commit 8f6fb95cd069884f4ce0a24eb20efc821ae3bc5e. --- src/tmpfiles/tmpfiles.c | 283 +++++++++++++++++----------------------- 1 file changed, 118 insertions(+), 165 deletions(-) diff --git a/src/tmpfiles/tmpfiles.c b/src/tmpfiles/tmpfiles.c index 784b895577..2c44856c3a 100644 --- a/src/tmpfiles/tmpfiles.c +++ b/src/tmpfiles/tmpfiles.c @@ -31,7 +31,6 @@ #include "dirent-util.h" #include "dissect-image.h" #include "env-util.h" -#include "errno-util.h" #include "escape.h" #include "fd-util.h" #include "fileio.h" @@ -1515,9 +1514,11 @@ static int create_file(Item *i, const char *path) { st = &stbuf; creation = CREATION_EXISTING; } else { - r = write_argument_data(i, fd, path); - if (r < 0) - return r; + if (item_binary_argument(i)) { + r = write_argument_data(i, fd, path); + if (r < 0) + return r; + } creation = CREATION_NORMAL; } @@ -1617,7 +1618,6 @@ static int truncate_file(Item *i, const char *path) { static int copy_files(Item *i) { _cleanup_close_ int dfd = -1, fd = -1; _cleanup_free_ char *bn = NULL; - struct stat st, a; int r; log_debug("Copying tree \"%s\" to \"%s\".", i->argument, i->path); @@ -1637,40 +1637,46 @@ static int copy_files(Item *i) { i->uid_set ? i->uid : UID_INVALID, i->gid_set ? i->gid : GID_INVALID, COPY_REFLINK | COPY_MERGE_EMPTY | COPY_MAC_CREATE | COPY_HARDLINKS); + if (r < 0) { + struct stat a, b; - fd = openat(dfd, bn, O_NOFOLLOW|O_CLOEXEC|O_PATH); - if (fd < 0) { - if (r < 0) /* Look at original error first */ - return log_error_errno(r, "Failed to copy files to %s: %m", i->path); + /* If the target already exists on read-only filesystems, trying + * to create the target will not fail with EEXIST but with + * EROFS. */ + if (r == -EROFS && faccessat(dfd, bn, F_OK, AT_SYMLINK_NOFOLLOW) == 0) + r = -EEXIST; - return log_error_errno(errno, "Failed to openat(%s): %m", i->path); - } + if (r != -EEXIST) + return log_error_errno(r, "Failed to copy files to %s: %m", i->path); - if (fstat(fd, &st) < 0) - return log_error_errno(errno, "Failed to fstat(%s): %m", i->path); + if (stat(i->argument, &a) < 0) + return log_error_errno(errno, "stat(%s) failed: %m", i->argument); - if (stat(i->argument, &a) < 0) - return log_error_errno(errno, "Failed to stat(%s): %m", i->argument); + if (fstatat(dfd, bn, &b, AT_SYMLINK_NOFOLLOW) < 0) + return log_error_errno(errno, "stat(%s) failed: %m", i->path); - if (((st.st_mode ^ a.st_mode) & S_IFMT) != 0) { - log_debug("Can't copy to %s, file exists already and is of different type", i->path); - return 0; + if ((a.st_mode ^ b.st_mode) & S_IFMT) { + log_debug("Can't copy to %s, file exists already and is of different type", i->path); + return 0; + } } - return fd_set_perms(i, fd, i->path, &st, _CREATION_MODE_INVALID); + fd = openat(dfd, bn, O_NOFOLLOW|O_CLOEXEC|O_PATH); + if (fd < 0) + return log_error_errno(errno, "Failed to openat(%s): %m", i->path); + + return fd_set_perms(i, fd, i->path, /* st = */ NULL, _CREATION_MODE_INVALID); } static int create_directory_or_subvolume( const char *path, mode_t mode, bool subvol, - struct stat *ret_st, CreationMode *ret_creation) { _cleanup_free_ char *bn = NULL; _cleanup_close_ int pfd = -1; - CreationMode creation; - struct stat st; + CreationMode c; int r, fd; assert(path); @@ -1690,7 +1696,7 @@ static int create_directory_or_subvolume( log_warning_errno(r, "Cannot parse value of $SYSTEMD_TMPFILES_FORCE_SUBVOL, ignoring."); r = btrfs_is_subvol(empty_to_root(arg_root)) > 0; } - if (r == 0) + if (!r) /* Don't create a subvolume unless the root directory is one, too. We do this under * the assumption that if the root directory is just a plain directory (i.e. very * light-weight), we shouldn't try to split it up into subvolumes (i.e. more @@ -1706,36 +1712,37 @@ static int create_directory_or_subvolume( } else r = 0; - if (!subvol || ERRNO_IS_NOT_SUPPORTED(r)) + if (!subvol || r == -ENOTTY) RUN_WITH_UMASK(0000) r = mkdirat_label(pfd, bn, mode); - creation = r >= 0 ? CREATION_NORMAL : CREATION_EXISTING; - - fd = openat(pfd, bn, O_NOFOLLOW|O_CLOEXEC|O_DIRECTORY|O_PATH); - if (fd < 0) { - /* We couldn't open it because it is not actually a directory? */ - if (errno == ENOTDIR) - return log_error_errno(SYNTHETIC_ERRNO(EEXIST), "\"%s\" already exists and is not a directory.", path); + if (r < 0) { + int k; - /* Then look at the original error */ - if (r < 0) + if (!IN_SET(r, -EEXIST, -EROFS)) return log_error_errno(r, "Failed to create directory or subvolume \"%s\": %m", path); - return log_error_errno(errno, "Failed to open directory/subvolume we just created '%s': %m", path); - } + k = is_dir_full(pfd, bn, /* follow= */ false); + if (k == -ENOENT && r == -EROFS) + return log_error_errno(r, "%s does not exist and cannot be created as the file system is read-only.", path); + if (k < 0) + return log_error_errno(k, "Failed to check if %s exists: %m", path); + if (!k) + return log_warning_errno(SYNTHETIC_ERRNO(EEXIST), + "\"%s\" already exists and is not a directory.", path); - if (fstat(fd, &st) < 0) - return log_error_errno(errno, "Failed to fstat(%s): %m", path); + c = CREATION_EXISTING; + } else + c = CREATION_NORMAL; - assert(S_ISDIR(st.st_mode)); /* we used O_DIRECTORY above */ + log_debug("%s directory \"%s\".", creation_mode_verb_to_string(c), path); - log_debug("%s directory \"%s\".", creation_mode_verb_to_string(creation), path); + fd = openat(pfd, bn, O_NOCTTY|O_CLOEXEC|O_DIRECTORY); + if (fd < 0) + return log_error_errno(errno, "Failed to open directory '%s': %m", bn); - if (ret_st) - *ret_st = st; if (ret_creation) - *ret_creation = creation; + *ret_creation = c; return fd; } @@ -1743,30 +1750,28 @@ static int create_directory_or_subvolume( static int create_directory(Item *i, const char *path) { _cleanup_close_ int fd = -1; CreationMode creation; - struct stat st; assert(i); assert(IN_SET(i->type, CREATE_DIRECTORY, TRUNCATE_DIRECTORY)); - fd = create_directory_or_subvolume(path, i->mode, /* subvol= */ false, &st, &creation); + fd = create_directory_or_subvolume(path, i->mode, /* subvol= */ false, &creation); if (fd == -EEXIST) return 0; if (fd < 0) return fd; - return fd_set_perms(i, fd, path, &st, creation); + return fd_set_perms(i, fd, path, /* st= */ NULL, creation); } static int create_subvolume(Item *i, const char *path) { _cleanup_close_ int fd = -1; CreationMode creation; - struct stat st; int r, q = 0; assert(i); assert(IN_SET(i->type, CREATE_SUBVOLUME, CREATE_SUBVOLUME_NEW_QUOTA, CREATE_SUBVOLUME_INHERIT_QUOTA)); - fd = create_directory_or_subvolume(path, i->mode, /* subvol = */ true, &st, &creation); + fd = create_directory_or_subvolume(path, i->mode, /* subvol = */ true, &creation); if (fd == -EEXIST) return 0; if (fd < 0) @@ -1789,7 +1794,7 @@ static int create_subvolume(Item *i, const char *path) { log_debug("Quota for subvolume \"%s\" already in place, no change made.", i->path); } - r = fd_set_perms(i, fd, path, &st, creation); + r = fd_set_perms(i, fd, path, /* st= */ NULL, creation); if (q < 0) /* prefer the quota change error from above */ return q; @@ -1830,11 +1835,9 @@ static int create_device(Item *i, mode_t file_type) { _cleanup_close_ int dfd = -1, fd = -1; _cleanup_free_ char *bn = NULL; CreationMode creation; - struct stat st; int r; assert(i); - assert(IN_SET(i->type, CREATE_BLOCK_DEVICE, CREATE_CHAR_DEVICE)); assert(IN_SET(file_type, S_IFBLK, S_IFCHR)); r = path_extract_filename(i->path, &bn); @@ -1854,166 +1857,116 @@ static int create_device(Item *i, mode_t file_type) { r = RET_NERRNO(mknodat(dfd, bn, i->mode | file_type, i->major_minor)); mac_selinux_create_file_clear(); } - creation = r >= 0 ? CREATION_NORMAL : CREATION_EXISTING; - - /* Try to open the inode via O_PATH, regardless if we could create it or not. Maybe everything is in - * order anyway and we hence can ignore the error to create the device node */ - fd = openat(dfd, bn, O_NOFOLLOW|O_CLOEXEC|O_PATH); - if (fd < 0) { - /* OK, so opening the inode failed, let's look at the original error then. */ - if (r < 0) { - if (ERRNO_IS_PRIVILEGE(r)) - goto handle_privilege; + if (r < 0) { + struct stat st; - return log_error_errno(r, "Failed to create device node '%s': %m", i->path); + if (r == -EPERM) { + log_debug_errno(r, + "We lack permissions, possibly because of cgroup configuration; " + "skipping creation of device node %s.", i->path); + return 0; } - return log_error_errno(errno, "Failed to open device node '%s' we just created: %m", i->path); - } + if (r != -EEXIST) + return log_error_errno(r, "Failed to create device node %s: %m", i->path); - if (fstat(fd, &st) < 0) - return log_error_errno(errno, "Failed to fstat(%s): %m", i->path); + if (fstatat(dfd, bn, &st, 0) < 0) + return log_error_errno(errno, "stat(%s) failed: %m", i->path); - if (((st.st_mode ^ file_type) & S_IFMT) != 0) { + if ((st.st_mode & S_IFMT) != file_type) { - if (i->append_or_force) { - fd = safe_close(fd); + if (i->append_or_force) { - RUN_WITH_UMASK(0000) { - mac_selinux_create_file_prepare(i->path, file_type); - r = mknodat_atomic(dfd, bn, i->mode | file_type, i->major_minor); - mac_selinux_create_file_clear(); - } - if (ERRNO_IS_PRIVILEGE(r)) - goto handle_privilege; - if (IN_SET(r, -EISDIR, -EEXIST, -ENOTEMPTY)) { - r = rm_rf_child(dfd, bn, REMOVE_PHYSICAL); - if (r < 0) - return log_error_errno(r, "rm -rf %s failed: %m", i->path); + RUN_WITH_UMASK(0000) { + mac_selinux_create_file_prepare(i->path, file_type); + /* FIXME: need to introduce mknodat_atomic() */ + r = mknod_atomic(i->path, i->mode | file_type, i->major_minor); + mac_selinux_create_file_clear(); + } - mac_selinux_create_file_prepare(i->path, file_type); - r = RET_NERRNO(mknodat(dfd, bn, i->mode | file_type, i->major_minor)); - mac_selinux_create_file_clear(); + if (r < 0) + return log_error_errno(r, "Failed to create device node \"%s\": %m", i->path); + creation = CREATION_FORCE; + } else { + log_warning("\"%s\" already exists is not a device node.", i->path); + return 0; } - if (r < 0) - return log_error_errno(r, "Failed to create device node '%s': %m", i->path); - - fd = openat(dfd, bn, O_NOFOLLOW|O_CLOEXEC|O_PATH); - if (fd < 0) - return log_error_errno(errno, "Failed to open device node we just created '%s': %m", i->path); - - /* Validate type before change ownership below */ - if (fstat(fd, &st) < 0) - return log_error_errno(errno, "Failed to fstat(%s): %m", i->path); - - if (((st.st_mode ^ file_type) & S_IFMT) != 0) - return log_error_errno(SYNTHETIC_ERRNO(EBADF), "Device node we just created is not a device node, refusing."); - - creation = CREATION_FORCE; - } else { - log_warning("\"%s\" already exists and is not a device node.", i->path); - return 0; - } - } + } else + creation = CREATION_EXISTING; + } else + creation = CREATION_NORMAL; log_debug("%s %s device node \"%s\" %u:%u.", creation_mode_verb_to_string(creation), i->type == CREATE_BLOCK_DEVICE ? "block" : "char", i->path, major(i->mode), minor(i->mode)); - return fd_set_perms(i, fd, i->path, &st, creation); + fd = openat(dfd, bn, O_NOFOLLOW|O_CLOEXEC|O_PATH); + if (fd < 0) + return log_error_errno(errno, "Failed to openat(%s): %m", i->path); -handle_privilege: - log_debug_errno(r, - "We lack permissions, possibly because of cgroup configuration; " - "skipping creation of device node '%s'.", i->path); - return 0; + return fd_set_perms(i, fd, i->path, /* st = */ NULL, creation); } -static int create_fifo(Item *i) { +static int create_fifo(Item *i, const char *path) { _cleanup_close_ int pfd = -1, fd = -1; _cleanup_free_ char *bn = NULL; CreationMode creation; struct stat st; int r; - assert(i); - assert(i->type == CREATE_FIFO); - r = path_extract_filename(i->path, &bn); if (r < 0) - return log_error_errno(r, "Failed to extract filename from path '%s': %m", i->path); + return log_error_errno(r, "Failed to extract filename from path '%s': %m", path); if (r == O_DIRECTORY) - return log_error_errno(SYNTHETIC_ERRNO(EISDIR), "Cannot open path '%s' for creating FIFO, is a directory.", i->path); + return log_error_errno(SYNTHETIC_ERRNO(EISDIR), "Cannot open path '%s' for creating FIFO, is a directory.", path); - pfd = path_open_parent_safe(i->path); + pfd = path_open_parent_safe(path); if (pfd < 0) return pfd; RUN_WITH_UMASK(0000) { - mac_selinux_create_file_prepare(i->path, S_IFIFO); + mac_selinux_create_file_prepare(path, S_IFIFO); r = RET_NERRNO(mkfifoat(pfd, bn, i->mode)); mac_selinux_create_file_clear(); } - creation = r >= 0 ? CREATION_NORMAL : CREATION_EXISTING; - - /* Open the inode via O_PATH, regardless if we managed to create it or not. Maybe it is is already the FIFO we want */ - fd = openat(pfd, bn, O_NOFOLLOW|O_CLOEXEC|O_PATH); - if (fd < 0) { - if (r < 0) - return log_error_errno(r, "Failed to create FIFO %s: %m", i->path); /* original error! */ - - return log_error_errno(errno, "Failed to open FIFO we just created %s: %m", i->path); - } + if (r < 0) { + if (r != -EEXIST) + return log_error_errno(r, "Failed to create fifo %s: %m", path); - if (fstat(fd, &st) < 0) - return log_error_errno(errno, "Failed to fstat(%s): %m", i->path); + if (fstatat(pfd, bn, &st, AT_SYMLINK_NOFOLLOW) < 0) + return log_error_errno(errno, "stat(%s) failed: %m", path); - if (!S_ISFIFO(st.st_mode)) { + if (!S_ISFIFO(st.st_mode)) { - if (i->append_or_force) { - fd = safe_close(fd); + if (i->append_or_force) { + RUN_WITH_UMASK(0000) { + mac_selinux_create_file_prepare(path, S_IFIFO); + r = mkfifoat_atomic(pfd, bn, i->mode); + mac_selinux_create_file_clear(); + } - RUN_WITH_UMASK(0000) { - mac_selinux_create_file_prepare(i->path, S_IFIFO); - r = mkfifoat_atomic(pfd, bn, i->mode); - mac_selinux_create_file_clear(); - } - if (IN_SET(r, -EISDIR, -EEXIST, -ENOTEMPTY)) { - r = rm_rf_child(pfd, bn, REMOVE_PHYSICAL); if (r < 0) - return log_error_errno(r, "rm -rf %s failed: %m", i->path); - - mac_selinux_create_file_prepare(i->path, S_IFIFO); - r = RET_NERRNO(mkfifoat(pfd, bn, i->mode)); - mac_selinux_create_file_clear(); + return log_error_errno(r, "Failed to create fifo %s: %m", path); + creation = CREATION_FORCE; + } else { + log_warning("\"%s\" already exists and is not a fifo.", path); + return 0; } - if (r < 0) - return log_error_errno(r, "Failed to create FIFO %s: %m", i->path); - - fd = openat(pfd, bn, O_NOFOLLOW|O_CLOEXEC|O_PATH); - if (fd < 0) - return log_error_errno(errno, "Failed to open FIFO we just created '%s': %m", i->path); - - /* Validate type before change ownership below */ - if (fstat(fd, &st) < 0) - return log_error_errno(errno, "Failed to fstat(%s): %m", i->path); + } else + creation = CREATION_EXISTING; + } else + creation = CREATION_NORMAL; - if (!S_ISFIFO(st.st_mode)) - return log_error_errno(SYNTHETIC_ERRNO(EBADF), "FIFO inode we just created is not a FIFO, refusing."); + log_debug("%s fifo \"%s\".", creation_mode_verb_to_string(creation), path); - creation = CREATION_FORCE; - } else { - log_warning("\"%s\" already exists and is not a FIFO.", i->path); - return 0; - } - } - - log_debug("%s fifo \"%s\".", creation_mode_verb_to_string(creation), i->path); + fd = openat(pfd, bn, O_NOFOLLOW|O_CLOEXEC|O_PATH); + if (fd < 0) + return log_error_errno(errno, "Failed to openat(%s): %m", path); - return fd_set_perms(i, fd, i->path, &st, creation); + return fd_set_perms(i, fd, i->path, /* st = */ NULL, creation); } static int create_symlink(Item *i) { @@ -2482,7 +2435,7 @@ static int create_item(Item *i) { if (r < 0) return r; - r = create_fifo(i); + r = create_fifo(i, i->path); if (r < 0) return r; break; -- 2.35.3