From: Kai Lüke Date: Wed, 29 Apr 2026 13:04:51 +0000 (+0900) Subject: mount-util: Compact list of sub mounts after dropping X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=7eabeaed0e6495ff74bc5f6cce98ec8209cef540;p=thirdparty%2Fsystemd.git mount-util: Compact list of sub mounts after dropping When nested mounts appear under a sysext hierarchy like this: mkdir -p /opt/trigger/ mount -t tmpfs tmpfs /opt/trigger mkdir -p /opt/trigger/inner mount -t tmpfs tmpfs /opt/trigger/inner Then systemd-sysext merge hit an assertion reported in https://github.com/flatcar/Flatcar/issues/2111 because when it iterates over the list of sub mounts it doesn't expect entries with NULL in the path from the dropped entries. Instead of having to deal with entries with path NULL, better sort the holes from dropping to the end and then reduce the array length. --- diff --git a/src/shared/mount-util.c b/src/shared/mount-util.c index cda0070b14e..1654e49d84f 100644 --- a/src/shared/mount-util.c +++ b/src/shared/mount-util.c @@ -1754,21 +1754,36 @@ DEFINE_ARRAY_FREE_FUNC(sub_mount_array_free, SubMount, sub_mount_clear); static int sub_mount_compare(const SubMount *a, const SubMount *b) { assert(a); assert(b); - assert(a->path); - assert(b->path); + + /* sub_mount_drop() creates NULL paths which we order to the end so that after the sort we can + * truncate the array. Done manually because path_compare() orders NULL before non-NULL. */ + if (!a->path || !b->path) + return CMP(!a->path, !b->path); return path_compare(a->path, b->path); } -static void sub_mount_drop(SubMount *s, size_t n) { - assert(s || n == 0); +static void sub_mount_drop(SubMount *s, size_t *n) { + assert(n); + assert(s || *n == 0); + + /* Works on a sorted array. Drops mounts that are covered by the preceding entry's recursive + * open_tree() clone, clearing the slot in place. Then sorts again for the NULL paths to be shifted + * past the kept count. */ - for (size_t m = 0, i = 1; i < n; i++) { + size_t kept = *n > 0; + for (size_t m = 0, i = 1; i < *n; i++) if (path_startswith(s[i].path, s[m].path)) sub_mount_clear(s + i); - else + else { m = i; - } + kept++; + } + + if (kept < *n) + typesafe_qsort(s, *n, sub_mount_compare); + + *n = kept; } #endif @@ -1843,7 +1858,7 @@ int get_sub_mounts(const char *prefix, SubMount **ret_mounts, size_t *ret_n_moun } typesafe_qsort(mounts, n, sub_mount_compare); - sub_mount_drop(mounts, n); + sub_mount_drop(mounts, &n); *ret_mounts = TAKE_PTR(mounts); *ret_n_mounts = n; diff --git a/src/test/test-mount-util.c b/src/test/test-mount-util.c index 4b232ac598f..135e53a6dd5 100644 --- a/src/test/test-mount-util.c +++ b/src/test/test-mount-util.c @@ -490,6 +490,55 @@ TEST(bind_mount_submounts) { } } +TEST(get_sub_mounts) { + _cleanup_(rm_rf_physical_and_freep) char *a = NULL; + int r; + + CHECK_PRIV; + + ASSERT_OK(mkdtemp_malloc(NULL, &a)); + + r = ASSERT_OK(pidref_safe_fork("(get-sub-mounts)", FORK_COMMON_FLAGS, NULL)); + if (r == 0) { + SubMount *mounts = NULL; + size_t n = 0; + + CLEANUP_ARRAY(mounts, n, sub_mount_array_free); + + /* Reproduces the layout that triggered the assertion crash in systemd-sysext on + * Flatcar (https://github.com/flatcar/Flatcar/issues/2111): a mount nested inside + * another mount under a sysext hierarchy. The dedup pass must keep only the outer + * entry (the inner is covered by the outer's recursive open_tree() clone) and + * compact the array — leaving NULL entries behind would crash the consumer. */ + + _cleanup_free_ char *outer = ASSERT_NOT_NULL(path_join(a, "outer")); + ASSERT_OK_ERRNO(mkdir(outer, 0755)); + ASSERT_OK(mount_nofollow_verbose(LOG_INFO, "tmpfs", outer, "tmpfs", 0, NULL)); + + _cleanup_free_ char *inner = ASSERT_NOT_NULL(path_join(outer, "inner")); + ASSERT_OK_ERRNO(mkdir(inner, 0755)); + ASSERT_OK(mount_nofollow_verbose(LOG_INFO, "tmpfs", inner, "tmpfs", 0, NULL)); + + r = get_sub_mounts(a, &mounts, &n); + if (r == -EOPNOTSUPP) { + log_tests_skipped("libmount support not compiled in"); + _exit(EXIT_SUCCESS); + } + ASSERT_OK(r); + + /* Only the outer entry should survive dedup; the inner is implied by the outer's + * recursive clone. */ + ASSERT_EQ(n, 1u); + ASSERT_NOT_NULL(mounts[0].path); + ASSERT_STREQ(mounts[0].path, outer); + ASSERT_OK(mounts[0].mount_fd); + + ASSERT_OK(umount_recursive(a, 0)); + + _exit(EXIT_SUCCESS); + } +} + TEST(path_is_network_fs_harder) { _cleanup_close_ int dir_fd = -EBADF; int r;