summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorLennart Poettering <lennart@poettering.net>2018-06-05 16:52:22 +0200
committerSven Eden <yamakuzure@gmx.net>2018-08-24 16:47:08 +0200
commit3f87ae2530d8bb022f54526c3a568165a34acf05 (patch)
treef405abf1d38e45e2a10d40aa352bdc18ca19e51d
parent0d77192b13d5fa28456d0d1bea6c63458fd66cb3 (diff)
copy: rework copy_file_atomic() to copy the specified file via O_TMPFILE if possible
-rw-r--r--TODO7
-rw-r--r--src/basic/copy.c103
-rw-r--r--src/test/test-copy.c21
3 files changed, 68 insertions, 63 deletions
diff --git a/TODO b/TODO
index 2cf8a4cfc..7b940d5c4 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
@@ -36,9 +36,6 @@ Features:
should be revisited to make clearer and also work if the payload elogind runs
with full privs and without userns.
-* portables: introduce a new unit file directory /etc/elogind/system.attached/
- or so, where we attach portable services to
-
* cgroups: use inotify to get notified when somebody else modifies cgroups
owned by us, then log a friendly warning.
diff --git a/src/basic/copy.c b/src/basic/copy.c
index 964613f8c..5b53f2e0f 100644
--- a/src/basic/copy.c
+++ b/src/basic/copy.c
@@ -29,7 +29,6 @@
#include "io-util.h"
//#include "macro.h"
#include "missing.h"
-//#include "mount-util.h"
//#include "string-util.h"
#include "strv.h"
#include "time-util.h"
@@ -37,12 +36,7 @@
#include "user-util.h"
//#include "xattr-util.h"
-#define COPY_BUFFER_SIZE (16U*1024U)
-
-/* A safety net for descending recursively into file system trees to copy. On Linux PATH_MAX is 4096, which means the
- * deepest valid path one can build is around 2048, which we hence use as a safety net here, to not spin endlessly in
- * case of bind mount cycles and suchlike. */
-#define COPY_DEPTH_MAX 2048U
+#define COPY_BUFFER_SIZE (16*1024u)
static ssize_t try_copy_file_range(
int fd_in, loff_t *off_in,
@@ -488,7 +482,6 @@ static int fd_copy_directory(
int dt,
const char *to,
dev_t original_device,
- unsigned depth_left,
uid_t override_uid,
gid_t override_gid,
CopyFlags copy_flags) {
@@ -502,9 +495,6 @@ static int fd_copy_directory(
assert(st);
assert(to);
- if (depth_left == 0)
- return -ENAMETOOLONG;
-
if (from)
fdf = openat(df, from, O_RDONLY|O_DIRECTORY|O_CLOEXEC|O_NOCTTY|O_NOFOLLOW);
else
@@ -543,40 +533,13 @@ static int fd_copy_directory(
continue;
}
- if (S_ISDIR(buf.st_mode)) {
- /*
- * Don't descend into directories on other file systems, if this is requested. We do a simple
- * .st_dev check here, which basically comes for free. Note that we do this check only on
- * directories, not other kind of file system objects, for two reason:
- *
- * • The kernel's overlayfs pseudo file system that overlays multiple real file systems
- * propagates the .st_dev field of the file system a file originates from all the way up
- * through the stack to stat(). It doesn't do that for directories however. This means that
- * comparing .st_dev on non-directories suggests that they all are mount points. To avoid
- * confusion we hence avoid relying on this check for regular files.
- *
- * • The main reason we do this check at all is to protect ourselves from bind mount cycles,
- * where we really want to avoid descending down in all eternity. However the .st_dev check
- * is usually not sufficient for this protection anyway, as bind mount cycles from the same
- * file system onto itself can't be detected that way. (Note we also do a recursion depth
- * check, which is probably the better protection in this regard, which is why
- * COPY_SAME_MOUNT is optional).
- */
-
- if (FLAGS_SET(copy_flags, COPY_SAME_MOUNT)) {
- if (buf.st_dev != original_device)
- continue;
-
- r = fd_is_mount_point(dirfd(d), de->d_name, 0);
- if (r < 0)
- return r;
- if (r > 0)
- continue;
- }
+ if (buf.st_dev != original_device)
+ continue;
- q = fd_copy_directory(dirfd(d), de->d_name, &buf, fdt, de->d_name, original_device, depth_left-1, override_uid, override_gid, copy_flags);
- } else if (S_ISREG(buf.st_mode))
+ if (S_ISREG(buf.st_mode))
q = fd_copy_regular(dirfd(d), de->d_name, &buf, fdt, de->d_name, override_uid, override_gid, copy_flags);
+ else if (S_ISDIR(buf.st_mode))
+ q = fd_copy_directory(dirfd(d), de->d_name, &buf, fdt, de->d_name, original_device, override_uid, override_gid, copy_flags);
else if (S_ISLNK(buf.st_mode))
q = fd_copy_symlink(dirfd(d), de->d_name, &buf, fdt, de->d_name, override_uid, override_gid, copy_flags);
else if (S_ISFIFO(buf.st_mode))
@@ -626,7 +589,7 @@ int copy_tree_at(int fdf, const char *from, int fdt, const char *to, uid_t overr
if (S_ISREG(st.st_mode))
return fd_copy_regular(fdf, from, &st, fdt, to, override_uid, override_gid, copy_flags);
else if (S_ISDIR(st.st_mode))
- return fd_copy_directory(fdf, from, &st, fdt, to, st.st_dev, COPY_DEPTH_MAX, override_uid, override_gid, copy_flags);
+ return fd_copy_directory(fdf, from, &st, fdt, to, st.st_dev, override_uid, override_gid, copy_flags);
else if (S_ISLNK(st.st_mode))
return fd_copy_symlink(fdf, from, &st, fdt, to, override_uid, override_gid, copy_flags);
else if (S_ISFIFO(st.st_mode))
@@ -653,7 +616,7 @@ int copy_directory_fd(int dirfd, const char *to, CopyFlags copy_flags) {
if (!S_ISDIR(st.st_mode))
return -ENOTDIR;
- return fd_copy_directory(dirfd, NULL, &st, AT_FDCWD, to, st.st_dev, COPY_DEPTH_MAX, UID_INVALID, GID_INVALID, copy_flags);
+ return fd_copy_directory(dirfd, NULL, &st, AT_FDCWD, to, st.st_dev, UID_INVALID, GID_INVALID, copy_flags);
}
int copy_directory(const char *from, const char *to, CopyFlags copy_flags) {
@@ -668,7 +631,7 @@ int copy_directory(const char *from, const char *to, CopyFlags copy_flags) {
if (!S_ISDIR(st.st_mode))
return -ENOTDIR;
- return fd_copy_directory(AT_FDCWD, from, &st, AT_FDCWD, to, st.st_dev, COPY_DEPTH_MAX, UID_INVALID, GID_INVALID, copy_flags);
+ return fd_copy_directory(AT_FDCWD, from, &st, AT_FDCWD, to, st.st_dev, UID_INVALID, GID_INVALID, copy_flags);
}
int copy_file_fd(const char *from, int fdt, CopyFlags copy_flags) {
@@ -721,31 +684,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;
- r = copy_file(from, t, O_NOFOLLOW|O_EXCL, mode, chattr_flags, copy_flags);
+ 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;
+ }
+
+ 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/test/test-copy.c b/src/test/test-copy.c
index 0d39d41bb..9dc0ddf95 100644
--- a/src/test/test-copy.c
+++ b/src/test/test-copy.c
@@ -242,7 +242,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);
+
#if 0 /// UNNEEDED by elogind
test_copy_file();
test_copy_file_fd();
@@ -255,6 +275,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;
}