Merge pull request #9199 from poettering/copy-file-atomic

make copy_file_atomic() use O_TMPFILE to create the destination file

4 files changed, 74 insertions, 21 deletions

diff --git a/TODO b/TODO
index c16712e61b..bcfe24ee30 100644
--- a/TODO
+++ b/TODO
@@ -14,6 +14,8 @@ Janitorial Clean-ups:
 
 * Rearrange tests so that the various test-xyz.c match a specific src/basic/xyz.c again
 
+* copy.c: set the right chattrs before copying files and others after
+
 * rework mount.c and swap.c to follow proper state enumeration/deserialization
   semantics, like we do for device.c now
 
@@ -25,8 +27,6 @@ Features:
 * Add OnTimezoneChange= and OnTimeChange= stanzas to .timer units in order to
   schedule events based on time and timezone changes.
 
-* add O_TMPFILE support to copy_file_atomic()
-
 * nspawn: greater control over selinux label?
 
 * cgroups: figure out if we can somehow communicate in a cleaner way whether a
diff --git a/src/basic/copy.c b/src/basic/copy.c
index 1921047714..2b4a481c25 100644
--- a/src/basic/copy.c
+++ b/src/basic/copy.c
@@ -718,31 +718,55 @@ int copy_file(const char *from, const char *to, int flags, mode_t mode, unsigned
 }
 
 int copy_file_atomic(const char *from, const char *to, mode_t mode, unsigned chattr_flags, CopyFlags copy_flags) {
-        _cleanup_free_ char *t = NULL;
+        _cleanup_(unlink_and_freep) char *t = NULL;
+        _cleanup_close_ int fdt = -1;
         int r;
 
         assert(from);
         assert(to);
 
-        r = tempfn_random(to, NULL, &t);
-        if (r < 0)
-                return r;
+        /* We try to use O_TMPFILE here to create the file if we can. Note that that 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) {
+                r = tempfn_random(to, NULL, &t);
+                if (r < 0)
+                        return r;
+
+                fdt = open(t, O_CREAT|O_EXCL|O_NOFOLLOW|O_NOCTTY|O_WRONLY|O_CLOEXEC, 0600);
+                if (fdt < 0) {
+                        t = mfree(t);
+                        return -errno;
+                }
+        } else {
+                fdt = open_tmpfile_linkable(to, O_WRONLY|O_CLOEXEC, &t);
+                if (fdt < 0)
+                        return fdt;
+        }
 
-        r = copy_file(from, t, O_NOFOLLOW|O_EXCL, mode, chattr_flags, copy_flags);
+        if (chattr_flags != 0)
+                (void) chattr_fd(fdt, chattr_flags, (unsigned) -1);
+
+        r = copy_file_fd(from, fdt, copy_flags);
         if (r < 0)
                 return r;
 
+        if (fchmod(fdt, mode) < 0)
+                return -errno;
+
         if (copy_flags & COPY_REPLACE) {
-                r = renameat(AT_FDCWD, t, AT_FDCWD, to);
+                if (renameat(AT_FDCWD, t, AT_FDCWD, to) < 0)
+                        return -errno;
+        } else {
+                r = link_tmpfile(fdt, t, to);
                 if (r < 0)
-                        r = -errno;
-        } else
-                r = rename_noreplace(AT_FDCWD, t, AT_FDCWD, to);
-        if (r < 0) {
-                (void) unlink(t);
-                return r;
+                        return r;
         }
 
+        t = mfree(t);
         return 0;
 }
 
diff --git a/src/basic/fileio.c b/src/basic/fileio.c
index 5837163fed..11223d5db8 100644
--- a/src/basic/fileio.c
+++ b/src/basic/fileio.c
@@ -1560,21 +1560,29 @@ int read_nul_string(FILE *f, char **ret) {
 }
 
 int mkdtemp_malloc(const char *template, char **ret) {
-        char *p;
+        _cleanup_free_ char *p = NULL;
+        int r;
 
-        assert(template);
         assert(ret);
 
-        p = strdup(template);
+        if (template)
+                p = strdup(template);
+        else {
+                const char *tmp;
+
+                r = tmp_dir(&tmp);
+                if (r < 0)
+                        return r;
+
+                p = strjoin(tmp, "/XXXXXX");
+        }
         if (!p)
                 return -ENOMEM;
 
-        if (!mkdtemp(p)) {
-                free(p);
+        if (!mkdtemp(p))
                 return -errno;
-        }
 
-        *ret = p;
+        *ret = TAKE_PTR(p);
         return 0;
 }
 
diff --git a/src/test/test-copy.c b/src/test/test-copy.c
index a02746c76d..dcac5f15f7 100644
--- a/src/test/test-copy.c
+++ b/src/test/test-copy.c
@@ -240,7 +240,27 @@ static void test_copy_bytes_regular_file(const char *src, bool try_reflink, uint
         unlink(fn3);
 }
 
+static void test_copy_atomic(void) {
+        _cleanup_(rm_rf_physical_and_freep) char *p = NULL;
+        const char *q;
+        int r;
+
+        assert_se(mkdtemp_malloc(NULL, &p) >= 0);
+
+        q = strjoina(p, "/fstab");
+
+        r = copy_file_atomic("/etc/fstab", q, 0644, 0, COPY_REFLINK);
+        if (r == -ENOENT)
+                return;
+
+        assert_se(copy_file_atomic("/etc/fstab", q, 0644, 0, COPY_REFLINK) == -EEXIST);
+
+        assert_se(copy_file_atomic("/etc/fstab", q, 0644, 0, COPY_REPLACE) >= 0);
+}
+
 int main(int argc, char *argv[]) {
+        log_set_max_level(LOG_DEBUG);
+
         test_copy_file();
         test_copy_file_fd();
         test_copy_tree();
@@ -251,6 +271,7 @@ int main(int argc, char *argv[]) {
         test_copy_bytes_regular_file(argv[0], true, 1000);
         test_copy_bytes_regular_file(argv[0], false, 32000); /* larger than copy buffer size */
         test_copy_bytes_regular_file(argv[0], true, 32000);
+        test_copy_atomic();
 
         return 0;
 }