From 2a72c69c37534bd522a8390a3c3181e2344d30a0 Mon Sep 17 00:00:00 2001 From: Paul Meyer Date: Tue, 2 Jun 2026 12:18:45 +0200 Subject: [PATCH 1/3] copy: walk source directories in sorted order in copy_tree() fd_copy_directory() iterates the source directory with FOREACH_DIRENT_ALL, i.e. plain readdir(). For ext4 sources (and similar) that order depends on the directory hash and varies between hosts, which leaks into destinations that record entries in insertion order. systemd-repart's partition_populate_directory() stages CopyFiles= into a temp directory and then later builds a filesystem from it; with the unsorted walk the staging order ended up baked into the output (most visibly when the destination is vfat, where dir entries are stored in insertion order). Replace the raw readdir loop with readdir_all(RECURSE_DIR_SORT), which collects and qsort()s the entries by name. Dot/dot-dot are filtered by readdir_all() so the explicit dot_or_dot_dot() guard is dropped. Co-developed-by: Claude Opus 4.7 Signed-off-by: Paul Meyer --- src/shared/copy.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/src/shared/copy.c b/src/shared/copy.c index fbcb8e2a30..f61b16caee 100644 --- a/src/shared/copy.c +++ b/src/shared/copy.c @@ -25,6 +25,7 @@ #include "mountpoint-util.h" #include "nulstr-util.h" #include "path-util.h" +#include "recurse-dir.h" #include "rm-rf.h" #include "selinux-util.h" #include "signal-util.h" @@ -1044,6 +1045,7 @@ static int fd_copy_directory( .parent_fd = -EBADF, }; + _cleanup_free_ DirectoryEntries *des = NULL; _cleanup_close_ int fdf = -EBADF, fdt = -EBADF; _cleanup_closedir_ DIR *d = NULL; struct stat dt_st; @@ -1107,14 +1109,20 @@ static int fd_copy_directory( goto finish; } - FOREACH_DIRENT_ALL(de, d, return -errno) { + /* Walk children in deterministic (alphabetical) order. The natural readdir() order depends on the + * source filesystem's directory storage (e.g. ext4 dir hash) and varies across hosts, which leaks + * into the destination when it records entries in insertion order (e.g. vfat). Sorting here keeps + * copy_tree() reproducible regardless of the source filesystem layout. */ + r = readdir_all(dirfd(d), RECURSE_DIR_SORT, &des); + if (r < 0) + return r; + + FOREACH_ARRAY(i, des->entries, des->n_entries) { + struct dirent *de = *i; const char *child_display_path = NULL; _cleanup_free_ char *dp = NULL; struct stat buf; - if (dot_or_dot_dot(de->d_name)) - continue; - r = look_for_signals(copy_flags); if (r < 0) return r; From 12a51745007646c975b22b0b81211d8b509c9aab Mon Sep 17 00:00:00 2001 From: Paul Meyer Date: Sun, 7 Jun 2026 13:40:23 +0200 Subject: [PATCH 2/3] test-copy: cover DENY_CONTENTS directory exclusion Exercise the early "goto finish" path in fd_copy_directory() by putting a directory's inode in the denylist as DENY_CONTENTS, and assert the directory is created in the destination but its contents are not copied. This path previously had no coverage. Co-developed-by: Claude Opus 4.8 Signed-off-by: Paul Meyer --- src/test/test-copy.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/src/test/test-copy.c b/src/test/test-copy.c index 7193014782..290a6bed8f 100644 --- a/src/test/test-copy.c +++ b/src/test/test-copy.c @@ -141,7 +141,7 @@ TEST(copy_tree) { "link2", "dir1/file"); char **hardlinks = STRV_MAKE("hlink", "file", "hlink2", "dir1/file"); - const char *unixsockp, *ignorep; + const char *unixsockp, *ignorep, *denydirp, *denyfilep; struct stat st; int xattr_worked = -1; /* xattr support is optional in temporary directories, hence use it if we can, * but don't fail if we can't */ @@ -194,6 +194,14 @@ TEST(copy_tree) { assert_se(hashmap_ensure_put(&denylist, &inode_hash_ops, cp, INT_TO_PTR(DENY_INODE)) >= 0); TAKE_PTR(cp); + denyfilep = strjoina(original_dir, "denycontents/file"); + assert_se(write_string_file(denyfilep, "denied", WRITE_STRING_FILE_CREATE|WRITE_STRING_FILE_MKDIR_0755) == 0); + denydirp = strjoina(original_dir, "denycontents"); + assert_se(RET_NERRNO(stat(denydirp, &st)) >= 0); + assert_se(cp = memdup(&st, sizeof(st))); + assert_se(hashmap_ensure_put(&denylist, &inode_hash_ops, cp, INT_TO_PTR(DENY_CONTENTS)) >= 0); + TAKE_PTR(cp); + assert_se(copy_tree(original_dir, copy_dir, UID_INVALID, GID_INVALID, COPY_REFLINK|COPY_MERGE|COPY_HARDLINKS, denylist, NULL) == 0); STRV_FOREACH(p, files) { @@ -252,6 +260,13 @@ TEST(copy_tree) { ignorep = strjoina(copy_dir, "ignore/file"); assert_se(RET_NERRNO(access(ignorep, F_OK)) == -ENOENT); + /* The DENY_CONTENTS directory itself must exist in the copy, but its contents must not have been copied. */ + denydirp = strjoina(copy_dir, "denycontents"); + assert_se(stat(denydirp, &st) >= 0); + assert_se(S_ISDIR(st.st_mode)); + denyfilep = strjoina(copy_dir, "denycontents/file"); + assert_se(RET_NERRNO(access(denyfilep, F_OK)) == -ENOENT); + (void) rm_rf(copy_dir, REMOVE_ROOT|REMOVE_PHYSICAL); (void) rm_rf(original_dir, REMOVE_ROOT|REMOVE_PHYSICAL); } From f191ca982ce9eac85207807a65489909308f7d8f Mon Sep 17 00:00:00 2001 From: Paul Meyer Date: Tue, 2 Jun 2026 12:19:10 +0200 Subject: [PATCH 3/3] mkfs-util: drive the vfat mtools recursion ourselves for reproducibility do_mcopy() invoked 'mcopy -s' with the top-level sources sorted by name, but then relied on mtools' built-in recursion to walk into subdirectories. mtools uses platform readdir(), so the order in which it inserts entries into the freshly-formatted FAT image inherits whatever order the source filesystem returns. With an ext4 staging directory the directory hash changes the readdir order between hosts (and between runs after edits), and because FAT stores directory entries in insertion order the resulting ESP differs byte-for-byte across otherwise identical builds. Stop using 'mcopy -s'. Walk the source ourselves with readdir_all_at(RECURSE_DIR_SORT) at every level and emit per-item mtools commands interleaved per parent: each subdirectory is created with 'mmd' before the recursion descends into it, and consecutive sibling files are batched into a single 'mcopy' invocation so the fork cost stays proportional to the number of directory transitions rather than to the total number of files. Combined with the existing 'mkfs.fat --invariant' and SOURCE_DATE_EPOCH plumbing, this makes vfat populated via repart byte-stable regardless of the staging filesystem's readdir() behaviour. Co-developed-by: Claude Opus 4.7 Signed-off-by: Paul Meyer --- src/shared/mkfs-util.c | 196 +++++++++++++++++++++++++++++++---------- 1 file changed, 148 insertions(+), 48 deletions(-) diff --git a/src/shared/mkfs-util.c b/src/shared/mkfs-util.c index e45a106ed5..0ba2fdf69f 100644 --- a/src/shared/mkfs-util.c +++ b/src/shared/mkfs-util.c @@ -98,10 +98,150 @@ static int mangle_fat_label(const char *s, char **ret) { return 0; } -static int do_mcopy(const char *node, const char *root) { - _cleanup_free_ char *mcopy = NULL; - _cleanup_strv_free_ char **argv = NULL; +static int mtools_exec(char *const *argv) { + int r; + + assert(argv); + assert(argv[0]); + + r = pidref_safe_fork( + "(mtools)", + FORK_RESET_SIGNALS|FORK_RLIMIT_NOFILE_SAFE|FORK_DEATHSIG_SIGTERM|FORK_LOG|FORK_WAIT|FORK_STDOUT_TO_STDERR|FORK_CLOSE_ALL_FDS, + /* ret= */ NULL); + if (r < 0) + return r; + if (r == 0) { + /* Avoid failures caused by mismatch in expectations between mkfs.vfat and mtools by + * disabling the stricter checks using MTOOLS_SKIP_CHECK. Force TZ=UTC and forward + * SOURCE_DATE_EPOCH so that mtools produces deterministic FAT timestamps. */ + execve(argv[0], argv, + STRV_MAKE("MTOOLS_SKIP_CHECK=1", + "TZ=UTC", + strv_find_prefix(environ, "SOURCE_DATE_EPOCH="))); + + log_error_errno(errno, "Failed to execute %s: %m", argv[0]); + _exit(EXIT_FAILURE); + } + + return 0; +} + +static int mcopy_flush_files( + const char *mcopy_bin, + const char *node, + const char *dest_rel, + char ***file_batch) { + + _cleanup_strv_free_ char **argv = NULL, **batch = TAKE_PTR(*file_batch); + _cleanup_free_ char *dest = NULL; + + assert(mcopy_bin); + assert(node); + assert(dest_rel); + assert(file_batch); + + if (strv_isempty(batch)) + return 0; + + /* mcopy treats ::dir/ as the destination directory. The trailing slash makes it copy the + * source files into it rather than renaming a single source to that path. */ + dest = strjoin("::", dest_rel, "/"); + if (!dest) + return log_oom(); + + argv = strv_new(mcopy_bin, "-p", "-Q", "-m", "-i", node); + if (!argv) + return log_oom(); + + STRV_FOREACH(p, batch) + if (strv_extend(&argv, *p) < 0) + return log_oom(); + + if (strv_extend(&argv, dest) < 0) + return log_oom(); + + return mtools_exec(argv); +} + +static int do_mcopy_recurse( + const char *mcopy_bin, + const char *mmd_bin, + const char *node, + const char *src_root, + const char *dest_rel) { + _cleanup_free_ DirectoryEntries *de = NULL; + _cleanup_strv_free_ char **file_batch = NULL; + int r; + + assert(mcopy_bin); + assert(mmd_bin); + assert(node); + assert(src_root); + assert(dest_rel); + + /* Walk the source in deterministic (alphabetical) order so the FAT directory entries are + * inserted in a host-independent sequence. We can't rely on `mcopy -s` to do this, as mtools + * recurses via the platform's readdir() so the order is FS dependent. Instead we drive the + * recursion here and issue per-item mmd/mcopy invocations interleaved per parent + * directory, batching consecutive sibling files so the fork cost stays bounded. */ + r = readdir_all_at(AT_FDCWD, src_root, RECURSE_DIR_SORT|RECURSE_DIR_ENSURE_TYPE, &de); + if (r < 0) + return log_error_errno(r, "Failed to read '%s' contents: %m", src_root); + + for (size_t i = 0; i < de->n_entries; i++) { + struct dirent *ent = de->entries[i]; + _cleanup_free_ char *src = NULL; + + if (!IN_SET(ent->d_type, DT_REG, DT_DIR)) { + log_debug("%s/%s is not a file/directory which are the only file types supported by vfat, ignoring", + src_root, ent->d_name); + continue; + } + + src = path_join(src_root, ent->d_name); + if (!src) + return log_oom(); + + if (ent->d_type == DT_REG) { + if (strv_consume(&file_batch, TAKE_PTR(src)) < 0) + return log_oom(); + continue; + } + + /* Directory. Flush pending file siblings first so the parent FAT directory's entry + * order matches the sorted enumeration above, then create the subdir and recurse. */ + r = mcopy_flush_files(mcopy_bin, node, dest_rel, &file_batch); + if (r < 0) + return r; + + _cleanup_free_ char *dst = strjoin("::", dest_rel, "/", ent->d_name); + if (!dst) + return log_oom(); + + /* Note: mmd accepts only -D and -i; there is no -Q quiet flag like mcopy has. */ + _cleanup_strv_free_ char **argv = strv_new(mmd_bin, "-i", node, dst); + if (!argv) + return log_oom(); + + r = mtools_exec(argv); + if (r < 0) + return r; + + _cleanup_free_ char *child_rel = strjoin(dest_rel, "/", ent->d_name); + if (!child_rel) + return log_oom(); + + r = do_mcopy_recurse(mcopy_bin, mmd_bin, node, src, child_rel); + if (r < 0) + return r; + } + + return mcopy_flush_files(mcopy_bin, node, dest_rel, &file_batch); +} + +static int do_mcopy(const char *node, const char *root) { + _cleanup_free_ char *mcopy = NULL, *mmd = NULL; int r; assert(node); @@ -117,53 +257,13 @@ static int do_mcopy(const char *node, const char *root) { if (r < 0) return log_error_errno(r, "Failed to determine whether mcopy binary exists: %m"); - argv = strv_new(mcopy, "-s", "-p", "-Q", "-m", "-i", node); - if (!argv) - return log_oom(); - - /* mcopy copies the top level directory instead of everything in it so we have to pass all - * the subdirectories to mcopy instead to end up with the correct directory structure. */ - - r = readdir_all_at(AT_FDCWD, root, RECURSE_DIR_SORT|RECURSE_DIR_ENSURE_TYPE, &de); + r = find_executable("mmd", &mmd); + if (r == -ENOENT) + return log_error_errno(SYNTHETIC_ERRNO(EPROTONOSUPPORT), "Could not find mmd binary."); if (r < 0) - return log_error_errno(r, "Failed to read '%s' contents: %m", root); + return log_error_errno(r, "Failed to determine whether mmd binary exists: %m"); - for (size_t i = 0; i < de->n_entries; i++) { - _cleanup_free_ char *p = NULL; - - p = path_join(root, de->entries[i]->d_name); - if (!p) - return log_oom(); - - if (!IN_SET(de->entries[i]->d_type, DT_REG, DT_DIR)) { - log_debug("%s is not a file/directory which are the only file types supported by vfat, ignoring", p); - continue; - } - - if (strv_consume(&argv, TAKE_PTR(p)) < 0) - return log_oom(); - } - - if (strv_extend(&argv, "::") < 0) - return log_oom(); - - r = pidref_safe_fork( - "(mcopy)", - FORK_RESET_SIGNALS|FORK_RLIMIT_NOFILE_SAFE|FORK_DEATHSIG_SIGTERM|FORK_LOG|FORK_WAIT|FORK_STDOUT_TO_STDERR|FORK_CLOSE_ALL_FDS, - /* ret= */ NULL); - if (r < 0) - return r; - if (r == 0) { - /* Avoid failures caused by mismatch in expectations between mkfs.vfat and mcopy by disabling - * the stricter mcopy checks using MTOOLS_SKIP_CHECK. */ - execve(mcopy, argv, STRV_MAKE("MTOOLS_SKIP_CHECK=1", "TZ=UTC", strv_find_prefix(environ, "SOURCE_DATE_EPOCH="))); - - log_error_errno(errno, "Failed to execute mcopy: %m"); - - _exit(EXIT_FAILURE); - } - - return 0; + return do_mcopy_recurse(mcopy, mmd, node, root, ""); } int make_filesystem(