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 cda0070b14e5471edbab6df3f27f3aab160105f3..1654e49d84fc5d9bebc8db106f199d519a7c1560 100644 (file)
--- 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 4b232ac598fa0802b959390775cc48fa0fcb0888..135e53a6dd52c82bc89547b170ecf36728772a8f 100644 (file)
--- 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;