File 6000-Revert-tmpfiles-whenever-creating-an-inode... of Package systemd

Overview Repositories Revisions Requests Users Attributes Meta

File 6000-Revert-tmpfiles-whenever-creating-an-inode-immediate.patch of Package systemd (Revision 531611379ed727af9facd957401a6600)

Currently displaying revision 531611379ed727af9facd957401a6600 , Show latest

From 3aabdad41fecf8004a1e0e4ca4bfc7b2f935404c Mon Sep 17 00:00:00 2001
From: Franck Bui <fbui@suse.com>
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 18bb75715b..9c2740f6ce 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"
@@ -1527,9 +1526,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;
         }
@@ -1629,7 +1630,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);
@@ -1649,27 +1649,35 @@ 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(
@@ -1677,13 +1685,11 @@ static int create_directory_or_subvolume(
                 mode_t mode,
                 bool subvol,
                 bool allow_failure,
-                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);
@@ -1703,7 +1709,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
@@ -1719,40 +1725,41 @@ 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_full_errno(allow_failure ? LOG_INFO : LOG_ERR,
                                               r,
                                               "Failed to create directory or subvolume \"%s\"%s: %m",
                                               path,
                                               allow_failure ? ", ignoring" : "");
 
-                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;
 }
@@ -1760,30 +1767,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, i->allow_failure, &st, &creation);
+        fd = create_directory_or_subvolume(path, i->mode, /* subvol= */ false, i->allow_failure, &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, i->allow_failure, &st, &creation);
+        fd = create_directory_or_subvolume(path, i->mode, /* subvol = */ true, i->allow_failure, &creation);
         if (fd == -EEXIST)
                 return 0;
         if (fd < 0)
@@ -1806,7 +1811,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;
 
@@ -1847,11 +1852,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);
@@ -1871,166 +1874,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, i->allow_failure);
+        pfd = path_open_parent_safe(path, i->allow_failure);
         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) {
@@ -2499,7 +2452,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

Places

File 6000-Revert-tmpfiles-whenever-creating-an-inode-immediate.patch of Package systemd (Revision 531611379ed727af9facd957401a6600)

Places