SymbX/systemd
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge pull request #28017 from poettering/link-tmpfile-flags

Browse Source 
teach link_tmpfile_at() to properly sync
This commit is contained in:
Lennart Poettering
2023-06-13 13:18:42 +02:00
committed by GitHub
parent d5a937a62a 8dfd099b50
commit 05d53d529a
8 changed files with 62 additions and 47 deletions
Download Patch File Download Diff File Expand all files Collapse all files
src/basic/tmpfile-util.c
+42 -29
View File
@@ -17,6 +17,7 @@
#include "stat-util.h"
#include "stdio-util.h"
#include "string-util.h"
#include "sync-util.h"
#include "tmpfile-util.h"
#include "umask-util.h"
@@ -349,7 +350,7 @@ static int link_fd(int fd, int newdirfd, const char *newpath) {
return RET_NERRNO(linkat(fd, "", newdirfd, newpath, AT_EMPTY_PATH));
}
int link_tmpfile_at(int fd, int dir_fd, const char *path, const char *target, bool replace) {
int link_tmpfile_at(int fd, int dir_fd, const char *path, const char *target, LinkTmpfileFlags flags) {
_cleanup_free_ char *tmp = NULL;
int r;
@@ -361,40 +362,52 @@ int link_tmpfile_at(int fd, int dir_fd, const char *path, const char *target, bo
* 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(renameat(dir_fd, path, dir_fd, target));
if (FLAGS_SET(flags, LINK_TMPFILE_SYNC) && fsync(fd) < 0)
return -errno;
return rename_noreplace(dir_fd, path, dir_fd, target);
if (path) {
if (FLAGS_SET(flags, LINK_TMPFILE_REPLACE))
r = RET_NERRNO(renameat(dir_fd, path, dir_fd, target));
else
r = rename_noreplace(dir_fd, path, dir_fd, target);
if (r < 0)
return r;
} else {
r = link_fd(fd, dir_fd, target);
if (r != -EEXIST || !FLAGS_SET(flags, LINK_TMPFILE_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, dir_fd, tmp) < 0)
return -EEXIST; /* propagate original error */
r = RET_NERRNO(renameat(dir_fd, tmp, dir_fd, target));
if (r < 0) {
(void) unlinkat(dir_fd, tmp, 0);
return r;
}
}
r = link_fd(fd, dir_fd, 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, dir_fd, tmp) < 0)
return -EEXIST; /* propagate original error */
r = RET_NERRNO(renameat(dir_fd, tmp, dir_fd, target));
if (r < 0) {
(void) unlinkat(dir_fd, tmp, 0);
return r;
if (FLAGS_SET(flags, LINK_TMPFILE_SYNC)) {
r = fsync_full(fd);
if (r < 0)
return r;
}
return 0;
}
int flink_tmpfile(FILE *f, const char *path, const char *target, bool replace) {
int flink_tmpfile(FILE *f, const char *path, const char *target, LinkTmpfileFlags flags) {
int fd, r;
assert(f);
@@ -404,11 +417,11 @@ int flink_tmpfile(FILE *f, const char *path, const char *target, bool replace) {
if (fd < 0) /* Not all FILE* objects encapsulate fds */
return -EBADF;
r = fflush_sync_and_check(f);
r = fflush_and_check(f);
if (r < 0)
return r;
return link_tmpfile(fd, path, target, replace);
return link_tmpfile(fd, path, target, flags);
}
int mkdtemp_malloc(const char *template, char **ret) {
src/basic/tmpfile-util.h
+10 -4
View File
@@ -29,11 +29,17 @@ static inline int open_tmpfile_linkable(const char *target, int flags, char **re
}
int fopen_tmpfile_linkable(const char *target, int flags, char **ret_path, FILE **ret_file);
int link_tmpfile_at(int fd, int dir_fd, const char *path, const char *target, bool replace);
static inline int link_tmpfile(int fd, const char *path, const char *target, bool replace) {
return link_tmpfile_at(fd, AT_FDCWD, path, target, replace);
typedef enum LinkTmpfileFlags {
LINK_TMPFILE_REPLACE = 1 << 0,
LINK_TMPFILE_SYNC = 1 << 1,
} LinkTmpfileFlags;
int link_tmpfile_at(int fd, int dir_fd, const char *path, const char *target, LinkTmpfileFlags flags);
static inline int link_tmpfile(int fd, const char *path, const char *target, LinkTmpfileFlags flags) {
return link_tmpfile_at(fd, AT_FDCWD, path, target, flags);
}
int flink_tmpfile(FILE *f, const char *path, const char *target, bool replace);
int flink_tmpfile(FILE *f, const char *path, const char *target, LinkTmpfileFlags flags);
int mkdtemp_malloc(const char *template, char **ret);
int mkdtemp_open(const char *template, int flags, char **ret);
src/boot/bootctl-install.c
+2 -2
View File
@@ -454,7 +454,7 @@ static int install_loader_config(const char *esp_path) {
fprintf(f, "default %s-*\n", arg_entry_token);
}
r = flink_tmpfile(f, t, p, /* replace= */ false);
r = flink_tmpfile(f, t, p, LINK_TMPFILE_SYNC);
if (r == -EEXIST)
return 0; /* Silently skip creation if the file exists now (recheck) */
if (r < 0)
@@ -483,7 +483,7 @@ static int install_loader_specification(const char *root) {
fprintf(f, "type1\n");
r = flink_tmpfile(f, t, p, /* replace= */ false);
r = flink_tmpfile(f, t, p, LINK_TMPFILE_SYNC);
if (r == -EEXIST)
return 0; /* Silently skip creation if the file exists now (recheck) */
if (r < 0)
src/coredump/coredump.c
+1 -5
View File
@@ -269,11 +269,7 @@ static int fix_permissions(
(void) fix_acl(fd, uid, allow_user);
(void) fix_xattr(fd, context);
r = fsync_full(fd);
if (r < 0)
return log_error_errno(r, "Failed to sync coredump %s: %m", coredump_tmpfile_name(filename));
r = link_tmpfile(fd, filename, target, /* replace= */ false);
r = link_tmpfile(fd, filename, target, LINK_TMPFILE_SYNC);
if (r < 0)
return log_error_errno(r, "Failed to move coredump %s into place: %m", target);
src/portable/portable.c
+1 -1
View File
@@ -1295,7 +1295,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, /* replace= */ false);
r = link_tmpfile(fd, tmp, path, LINK_TMPFILE_SYNC);
if (r < 0)
return log_debug_errno(r, "Failed to install unit file '%s': %m", path);
src/shared/copy.c
+1 -1
View File
@@ -1465,7 +1465,7 @@ int copy_file_atomic_at_full(
return -errno;
}
r = link_tmpfile_at(fdt, dir_fdt, t, to, copy_flags & COPY_REPLACE);
r = link_tmpfile_at(fdt, dir_fdt, t, to, (copy_flags & COPY_REPLACE) ? LINK_TMPFILE_REPLACE : 0);
if (r < 0)
return r;
src/shared/hwdb-util.c
+1 -1
View File
@@ -408,7 +408,7 @@ static int trie_store(struct trie *trie, const char *filename, bool compat) {
return -errno;
fwrite(&h, sizeof(struct trie_header_f), 1, f);
r = flink_tmpfile(f, filename_tmp, filename, /* replace= */ true);
r = flink_tmpfile(f, filename_tmp, filename, LINK_TMPFILE_REPLACE|LINK_TMPFILE_SYNC);
if (r < 0)
return r;
src/test/test-tmpfile-util.c
+4 -4
View File
@@ -278,9 +278,9 @@ TEST(link_tmpfile) {
assert_se(write(fd, "foobar\n", 7) == 7);
assert_se(touch(d) >= 0);
assert_se(link_tmpfile(fd, tmp, d, /* replace= */ false) == -EEXIST);
assert_se(link_tmpfile(fd, tmp, d, /* flags= */ 0) == -EEXIST);
assert_se(unlink(d) >= 0);
assert_se(link_tmpfile(fd, tmp, d, /* replace= */ false) >= 0);
assert_se(link_tmpfile(fd, tmp, d, /* flags= */ 0) >= 0);
assert_se(read_one_line_file(d, &line) >= 0);
assert_se(streq(line, "foobar"));
@@ -293,8 +293,8 @@ TEST(link_tmpfile) {
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);
assert_se(link_tmpfile(fd, tmp, d, /* flags= */ 0) == -EEXIST);
assert_se(link_tmpfile(fd, tmp, d, LINK_TMPFILE_REPLACE) >= 0);
line = mfree(line);
assert_se(read_one_line_file(d, &line) >= 0);