]> git.ipfire.org Git - thirdparty/systemd.git/commitdiff
tmpfile-util: teach link_tmpfile() to optionally replace files
authorLennart Poettering <lennart@poettering.net>
Fri, 3 Mar 2023 10:27:42 +0000 (11:27 +0100)
committerLennart Poettering <lennart@poettering.net>
Fri, 3 Mar 2023 12:44:12 +0000 (13:44 +0100)
src/basic/tmpfile-util.c
src/basic/tmpfile-util.h
src/boot/bootctl-install.c
src/coredump/coredump.c
src/portable/portable.c
src/shared/copy.c
src/test/test-tmpfiles.c

index 95adf9d374403c065d28b51d2ee8a4f634e2f55b..379d81d5c8c3abe89175a344f6a6995f1042da85 100644 (file)
@@ -14,6 +14,7 @@
 #include "path-util.h"
 #include "process-util.h"
 #include "random-util.h"
+#include "stat-util.h"
 #include "stdio-util.h"
 #include "string-util.h"
 #include "tmpfile-util.h"
@@ -328,24 +329,71 @@ int fopen_tmpfile_linkable(const char *target, int flags, char **ret_path, FILE
         return 0;
 }
 
-int link_tmpfile(int fd, const char *path, const char *target) {
+static int link_fd(int fd, int newdirfd, const char *newpath) {
+        int r;
+
+        assert(fd >= 0);
+        assert(newdirfd >= 0 || newdirfd == AT_FDCWD);
+        assert(newpath);
+
+        /* Try symlinking via /proc/fd/ first. */
+        r = RET_NERRNO(linkat(AT_FDCWD, FORMAT_PROC_FD_PATH(fd), newdirfd, newpath, AT_SYMLINK_FOLLOW));
+        if (r != -ENOENT)
+                return r;
+
+        /* Fall back to symlinking via AT_EMPTY_PATH as fallback (this requires CAP_DAC_READ_SEARCH and a
+         * more recent kernel, but does not require /proc/ mounted) */
+        if (proc_mounted() != 0)
+                return r;
+
+        return RET_NERRNO(linkat(fd, "", newdirfd, newpath, AT_EMPTY_PATH));
+}
+
+int link_tmpfile(int fd, const char *path, const char *target, bool replace) {
+        _cleanup_free_ char *tmp = NULL;
+        int r;
+
         assert(fd >= 0);
         assert(target);
 
-        /* Moves a temporary file created with open_tmpfile() above into its final place. if "path" is NULL an fd
-         * created with O_TMPFILE is assumed, and linkat() is used. Otherwise it is assumed O_TMPFILE is not supported
-         * on the directory, and renameat2() is used instead.
-         *
-         * Note that in both cases we will not replace existing files. This is because linkat() does not support this
-         * operation currently (renameat2() does), and there is no nice way to emulate this. */
+        /* Moves a temporary file created with open_tmpfile() above into its final place. If "path" is NULL
+         * an fd created with O_TMPFILE is assumed, and linkat() is used. Otherwise it is assumed O_TMPFILE
+         * is not supported on the directory, and renameat2() is used instead. */
+
+        if (path) {
+                if (replace)
+                        return RET_NERRNO(rename(path, target));
 
-        if (path)
                 return rename_noreplace(AT_FDCWD, path, AT_FDCWD, target);
+        }
+
+        r = link_fd(fd, AT_FDCWD, target);
+        if (r != -EEXIST || !replace)
+                return r;
+
+        /* So the target already exists and we were asked to replace it. That sucks a bit, since the kernel's
+         * linkat() logic does not allow that. We work-around this by linking the file to a random name
+         * first, and then renaming that to the final name. This reintroduces the race O_TMPFILE kinda is
+         * trying to fix, but at least the vulnerability window (i.e. where the file is linked into the file
+         * system under a temporary name) is very short. */
+
+        r = tempfn_random(target, NULL, &tmp);
+        if (r < 0)
+                return r;
+
+        if (link_fd(fd, AT_FDCWD, tmp) < 0)
+                return -EEXIST; /* propagate original error */
 
-        return RET_NERRNO(linkat(AT_FDCWD, FORMAT_PROC_FD_PATH(fd), AT_FDCWD, target, AT_SYMLINK_FOLLOW));
+        r = RET_NERRNO(rename(tmp, target));
+        if (r < 0) {
+                (void) unlink(tmp);
+                return r;
+        }
+
+        return 0;
 }
 
-int flink_tmpfile(FILE *f, const char *path, const char *target) {
+int flink_tmpfile(FILE *f, const char *path, const char *target, bool replace) {
         int fd, r;
 
         assert(f);
@@ -359,7 +407,7 @@ int flink_tmpfile(FILE *f, const char *path, const char *target) {
         if (r < 0)
                 return r;
 
-        return link_tmpfile(fd, path, target);
+        return link_tmpfile(fd, path, target, replace);
 }
 
 int mkdtemp_malloc(const char *template, char **ret) {
index e5b7709e3f9dee234ef47944a359e19e7f85fe3a..4665dafb24c0e10c0ebf614def8e1ce60c066a31 100644 (file)
@@ -25,8 +25,8 @@ int open_tmpfile_unlinkable(const char *directory, int flags);
 int open_tmpfile_linkable(const char *target, int flags, char **ret_path);
 int fopen_tmpfile_linkable(const char *target, int flags, char **ret_path, FILE **ret_file);
 
-int link_tmpfile(int fd, const char *path, const char *target);
-int flink_tmpfile(FILE *f, const char *path, const char *target);
+int link_tmpfile(int fd, const char *path, const char *target, bool replace);
+int flink_tmpfile(FILE *f, const char *path, const char *target, bool replace);
 
 int mkdtemp_malloc(const char *template, char **ret);
 int mkdtemp_open(const char *template, int flags, char **ret);
index ebb0d486c922779d21956d2544608e7147e53133..624b88abe372dd46bb9cc82078781916cec8ce55 100644 (file)
@@ -426,12 +426,14 @@ static int install_binaries(const char *esp_path, const char *arch, bool force)
 static int install_loader_config(const char *esp_path) {
         _cleanup_(unlink_and_freep) char *t = NULL;
         _cleanup_fclose_ FILE *f = NULL;
-        const char *p;
+        _cleanup_free_ char *p = NULL;
         int r;
 
         assert(arg_make_entry_directory >= 0);
 
-        p = prefix_roota(esp_path, "/loader/loader.conf");
+        p = path_join(esp_path, "/loader/loader.conf");
+        if (!p)
+                return log_oom();
         if (access(p, F_OK) >= 0) /* Silently skip creation if the file already exists (early check) */
                 return 0;
 
@@ -447,7 +449,7 @@ static int install_loader_config(const char *esp_path) {
                 fprintf(f, "default %s-*\n", arg_entry_token);
         }
 
-        r = flink_tmpfile(f, t, p);
+        r = flink_tmpfile(f, t, p, /* replace= */ false);
         if (r == -EEXIST)
                 return 0; /* Silently skip creation if the file exists now (recheck) */
         if (r < 0)
@@ -476,7 +478,7 @@ static int install_loader_specification(const char *root) {
 
         fprintf(f, "type1\n");
 
-        r = flink_tmpfile(f, t, p);
+        r = flink_tmpfile(f, t, p, /* replace= */ false);
         if (r == -EEXIST)
                 return 0; /* Silently skip creation if the file exists now (recheck) */
         if (r < 0)
index d9db98bf323a1a4d8b6e13cfcf46d2267ebb44be..d5d1f49d08aa7ebed29a6b57fef50b7b55b72566 100644 (file)
@@ -277,7 +277,7 @@ static int fix_permissions(
         if (r < 0)
                 return log_error_errno(r, "Failed to sync coredump %s: %m", coredump_tmpfile_name(filename));
 
-        r = link_tmpfile(fd, filename, target);
+        r = link_tmpfile(fd, filename, target, /* replace= */ false);
         if (r < 0)
                 return log_error_errno(r, "Failed to move coredump %s into place: %m", target);
 
index 7811833fac0a49fb18142418db1c7e1e32571faf..377e1a9937057cb780fa42d998d925652624a77b 100644 (file)
@@ -1188,7 +1188,7 @@ static int attach_unit_file(
                 if (fchmod(fd, 0644) < 0)
                         return log_debug_errno(errno, "Failed to change unit file access mode for '%s': %m", path);
 
-                r = link_tmpfile(fd, tmp, path);
+                r = link_tmpfile(fd, tmp, path, /* replace= */ false);
                 if (r < 0)
                         return log_debug_errno(r, "Failed to install unit file '%s': %m", path);
 
index 9963e10f4dba91874c116ed1cbb6af2d8448f753..146293acdff700be4fb4903695d2b41463272f4f 100644 (file)
@@ -1452,43 +1452,16 @@ int copy_file_atomic_full(
         assert(from);
         assert(to);
 
-        /* We try to use O_TMPFILE here to create the file if we can. Note that this only works if COPY_REPLACE is not
-         * set though as we need to use linkat() for linking the O_TMPFILE file into the file system but that system
-         * call can't replace existing files. Hence, if COPY_REPLACE is set we create a temporary name in the file
-         * system right-away and unconditionally which we then can renameat() to the right name after we completed
-         * writing it. */
-
-        if (copy_flags & COPY_REPLACE) {
-                _cleanup_free_ char *f = NULL;
-
-                r = tempfn_random(to, NULL, &f);
+        if (copy_flags & COPY_MAC_CREATE) {
+                r = mac_selinux_create_file_prepare(to, S_IFREG);
                 if (r < 0)
                         return r;
-
-                if (copy_flags & COPY_MAC_CREATE) {
-                        r = mac_selinux_create_file_prepare(to, S_IFREG);
-                        if (r < 0)
-                                return r;
-                }
-                fdt = open(f, O_CREAT|O_EXCL|O_NOFOLLOW|O_NOCTTY|O_WRONLY|O_CLOEXEC, 0600);
-                if (copy_flags & COPY_MAC_CREATE)
-                        mac_selinux_create_file_clear();
-                if (fdt < 0)
-                        return -errno;
-
-                t = TAKE_PTR(f);
-        } else {
-                if (copy_flags & COPY_MAC_CREATE) {
-                        r = mac_selinux_create_file_prepare(to, S_IFREG);
-                        if (r < 0)
-                                return r;
-                }
-                fdt = open_tmpfile_linkable(to, O_WRONLY|O_CLOEXEC, &t);
-                if (copy_flags & COPY_MAC_CREATE)
-                        mac_selinux_create_file_clear();
-                if (fdt < 0)
-                        return fdt;
         }
+        fdt = open_tmpfile_linkable(to, O_WRONLY|O_CLOEXEC, &t);
+        if (copy_flags & COPY_MAC_CREATE)
+                mac_selinux_create_file_clear();
+        if (fdt < 0)
+                return fdt;
 
         if (chattr_mask != 0)
                 (void) chattr_fd(fdt, chattr_flags, chattr_mask & CHATTR_EARLY_FL, NULL);
@@ -1506,14 +1479,9 @@ int copy_file_atomic_full(
                         return -errno;
         }
 
-        if (copy_flags & COPY_REPLACE) {
-                if (renameat(AT_FDCWD, t, AT_FDCWD, to) < 0)
-                        return -errno;
-        } else {
-                r = link_tmpfile(fdt, t, to);
-                if (r < 0)
-                        return r;
-        }
+        r = link_tmpfile(fdt, t, to, copy_flags & COPY_REPLACE);
+        if (r < 0)
+                return r;
 
         t = mfree(t);
 
index 83d11f5cf65682120b8a8dc0141fedb3bb84cdfc..0a856200c7ed250bc74d93423617161278fe6912 100644 (file)
@@ -52,12 +52,28 @@ TEST(tmpfiles) {
         assert_se(write(fd, "foobar\n", 7) == 7);
 
         assert_se(touch(d) >= 0);
-        assert_se(link_tmpfile(fd, tmp, d) == -EEXIST);
+        assert_se(link_tmpfile(fd, tmp, d, /* replace= */ false) == -EEXIST);
         assert_se(unlink(d) >= 0);
-        assert_se(link_tmpfile(fd, tmp, d) >= 0);
+        assert_se(link_tmpfile(fd, tmp, d, /* replace= */ false) >= 0);
 
         assert_se(read_one_line_file(d, &line) >= 0);
         assert_se(streq(line, "foobar"));
+
+        fd = safe_close(fd);
+        tmp = mfree(tmp);
+
+        fd = open_tmpfile_linkable(d, O_RDWR|O_CLOEXEC, &tmp);
+        assert_se(fd >= 0);
+
+        assert_se(write(fd, "waumiau\n", 8) == 8);
+
+        assert_se(link_tmpfile(fd, tmp, d, /* replace= */ false) == -EEXIST);
+        assert_se(link_tmpfile(fd, tmp, d, /* replace= */ true) >= 0);
+
+        line = mfree(line);
+        assert_se(read_one_line_file(d, &line) >= 0);
+        assert_se(streq(line, "waumiau"));
+
         assert_se(unlink(d) >= 0);
 }