repart: make vfat creation reproducible (#42446)

Two fixes to get this byte-stable:

- `fd_copy_directory()` was using `FOREACH_DIRENT_ALL`, which doesn't
give stable ordering. Read all paths, sort, then iterate.
- `mcopy -s` depends on `readdir()` ordering and thus isn't
reproducible. Implement the recursion/sorting here and only invoke
mcopy/mmd per dir.

First change increases memory usage, as we don't stream the paths
anymore, second increases the number of context switches when invoking
external tools. Both should be fine given the ESP content should usually
be pretty limited.

I'd like to write a test for this, but didn't come up with a way that
doesn't require privileges and would surface the error reliably.

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;

src/shared/mkfs-util.c

@@ -148,10 +148,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);
@@ -167,53 +307,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(

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);
 }