diff --git a/src/basic/tmpfile-util.c b/src/basic/tmpfile-util.c index 95adf9d374..379d81d5c8 100644 --- a/src/basic/tmpfile-util.c +++ b/src/basic/tmpfile-util.c @@ -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); + } - return RET_NERRNO(linkat(AT_FDCWD, FORMAT_PROC_FD_PATH(fd), AT_FDCWD, target, AT_SYMLINK_FOLLOW)); + 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 */ + + 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) { diff --git a/src/basic/tmpfile-util.h b/src/basic/tmpfile-util.h index e5b7709e3f..4665dafb24 100644 --- a/src/basic/tmpfile-util.h +++ b/src/basic/tmpfile-util.h @@ -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); diff --git a/src/boot/bootctl-install.c b/src/boot/bootctl-install.c index ebb0d486c9..624b88abe3 100644 --- a/src/boot/bootctl-install.c +++ b/src/boot/bootctl-install.c @@ -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) diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c index d9db98bf32..d5d1f49d08 100644 --- a/src/coredump/coredump.c +++ b/src/coredump/coredump.c @@ -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); diff --git a/src/portable/portable.c b/src/portable/portable.c index 7811833fac..377e1a9937 100644 --- a/src/portable/portable.c +++ b/src/portable/portable.c @@ -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); diff --git a/src/shared/copy.c b/src/shared/copy.c index 9963e10f4d..146293acdf 100644 --- a/src/shared/copy.c +++ b/src/shared/copy.c @@ -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); diff --git a/src/shared/hwdb-util.c b/src/shared/hwdb-util.c index 45910a36e3..785611f8c4 100644 --- a/src/shared/hwdb-util.c +++ b/src/shared/hwdb-util.c @@ -274,7 +274,6 @@ static int trie_insert(struct trie *trie, struct trie_node *node, const char *se } struct trie_f { - FILE *f; struct trie *trie; uint64_t strings_off; @@ -295,15 +294,14 @@ static void trie_store_nodes_size(struct trie_f *trie, struct trie_node *node, b trie->strings_off += compat ? sizeof(struct trie_value_entry_f) : sizeof(struct trie_value_entry2_f); } -static int64_t trie_store_nodes(struct trie_f *trie, struct trie_node *node, bool compat) { - struct trie_node_f n = { - .prefix_off = htole64(trie->strings_off + node->prefix_off), - .children_count = node->children_count, - .values_count = htole64(node->values_count), - }; +static int64_t trie_store_nodes(struct trie_f *trie, FILE *f, struct trie_node *node, bool compat) { _cleanup_free_ struct trie_child_entry_f *children = NULL; int64_t node_off; + assert(trie); + assert(f); + assert(node); + if (node->children_count) { children = new(struct trie_child_entry_f, node->children_count); if (!children) @@ -314,7 +312,7 @@ static int64_t trie_store_nodes(struct trie_f *trie, struct trie_node *node, boo for (uint64_t i = 0; i < node->children_count; i++) { int64_t child_off; - child_off = trie_store_nodes(trie, node->children[i].child, compat); + child_off = trie_store_nodes(trie, f, node->children[i].child, compat); if (child_off < 0) return child_off; @@ -324,14 +322,20 @@ static int64_t trie_store_nodes(struct trie_f *trie, struct trie_node *node, boo }; } + struct trie_node_f n = { + .prefix_off = htole64(trie->strings_off + node->prefix_off), + .children_count = node->children_count, + .values_count = htole64(node->values_count), + }; + /* write node */ - node_off = ftello(trie->f); - fwrite(&n, sizeof(struct trie_node_f), 1, trie->f); + node_off = ftello(f); + fwrite(&n, sizeof(struct trie_node_f), 1, f); trie->nodes_count++; /* append children array */ if (node->children_count) { - fwrite(children, sizeof(struct trie_child_entry_f), node->children_count, trie->f); + fwrite(children, sizeof(struct trie_child_entry_f), node->children_count, f); trie->children_count += node->children_count; } @@ -345,7 +349,7 @@ static int64_t trie_store_nodes(struct trie_f *trie, struct trie_node *node, boo .file_priority = htole16(node->values[i].file_priority), }; - fwrite(&v, compat ? sizeof(struct trie_value_entry_f) : sizeof(struct trie_value_entry2_f), 1, trie->f); + fwrite(&v, compat ? sizeof(struct trie_value_entry_f) : sizeof(struct trie_value_entry2_f), 1, f); } trie->values_count += node->values_count; @@ -355,11 +359,26 @@ static int64_t trie_store_nodes(struct trie_f *trie, struct trie_node *node, boo static int trie_store(struct trie *trie, const char *filename, bool compat) { struct trie_f t = { .trie = trie, + .strings_off = sizeof(struct trie_header_f), }; - _cleanup_free_ char *filename_tmp = NULL; - int64_t pos; - int64_t root_off; - int64_t size; + _cleanup_(unlink_and_freep) char *filename_tmp = NULL; + _cleanup_fclose_ FILE *f = NULL; + int64_t pos, root_off, size; + int r; + + assert(trie); + assert(filename); + + /* calculate size of header, nodes, children entries, value entries */ + trie_store_nodes_size(&t, trie->root, compat); + + r = fopen_tmpfile_linkable(filename, O_WRONLY|O_CLOEXEC, &filename_tmp, &f); + if (r < 0) + return r; + + if (fchmod(fileno(f), 0444) < 0) + return -errno; + struct trie_header_f h = { .signature = HWDB_SIG, .tool_version = htole64(PROJECT_VERSION), @@ -368,48 +387,32 @@ static int trie_store(struct trie *trie, const char *filename, bool compat) { .child_entry_size = htole64(sizeof(struct trie_child_entry_f)), .value_entry_size = htole64(compat ? sizeof(struct trie_value_entry_f) : sizeof(struct trie_value_entry2_f)), }; - int r; - - /* calculate size of header, nodes, children entries, value entries */ - t.strings_off = sizeof(struct trie_header_f); - trie_store_nodes_size(&t, trie->root, compat); - - r = fopen_temporary(filename, &t.f, &filename_tmp); - if (r < 0) - return r; - (void) fchmod(fileno(t.f), 0444); /* write nodes */ - if (fseeko(t.f, sizeof(struct trie_header_f), SEEK_SET) < 0) - goto error_fclose; + if (fseeko(f, sizeof(struct trie_header_f), SEEK_SET) < 0) + return -errno; - root_off = trie_store_nodes(&t, trie->root, compat); + root_off = trie_store_nodes(&t, f, trie->root, compat); h.nodes_root_off = htole64(root_off); - pos = ftello(t.f); + pos = ftello(f); h.nodes_len = htole64(pos - sizeof(struct trie_header_f)); /* write string buffer */ - fwrite(trie->strings->buf, trie->strings->len, 1, t.f); + fwrite(trie->strings->buf, trie->strings->len, 1, f); h.strings_len = htole64(trie->strings->len); /* write header */ - size = ftello(t.f); + size = ftello(f); h.file_size = htole64(size); - if (fseeko(t.f, 0, SEEK_SET) < 0) - goto error_fclose; - fwrite(&h, sizeof(struct trie_header_f), 1, t.f); + if (fseeko(f, 0, SEEK_SET) < 0) + return -errno; + fwrite(&h, sizeof(struct trie_header_f), 1, f); - if (ferror(t.f)) - goto error_fclose; - if (fflush(t.f) < 0) - goto error_fclose; - if (fsync(fileno(t.f)) < 0) - goto error_fclose; - if (rename(filename_tmp, filename) < 0) - goto error_fclose; + r = flink_tmpfile(f, filename_tmp, filename, /* replace= */ true); + if (r < 0) + return r; /* write succeeded */ - fclose(t.f); log_debug("=== trie on-disk ==="); log_debug("size: %8"PRIi64" bytes", size); @@ -423,12 +426,6 @@ static int trie_store(struct trie *trie, const char *filename, bool compat) { log_debug("string store: %8zu bytes", trie->strings->len); log_debug("strings start: %8"PRIu64, t.strings_off); return 0; - - error_fclose: - r = -errno; - fclose(t.f); - (void) unlink(filename_tmp); - return r; } static int insert_data(struct trie *trie, char **match_list, char *line, const char *filename, diff --git a/src/test/test-tmpfiles.c b/src/test/test-tmpfiles.c index 83d11f5cf6..0a856200c7 100644 --- a/src/test/test-tmpfiles.c +++ b/src/test/test-tmpfiles.c @@ -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); }