switchroot: Stop making /sysroot mount private

Back in 2b8d586c5, /sysroot was changed to be a private mount so that
submounts of /var do not propagate back to the stateroot /var. That's
laudible, but it makes /sysroot different than every other shared mount
in the root namespace. In particular, it means that submounts of
/sysroot do not propagate into separate mount namespaces.

Rather than make /sysroot private, make /var a slave+shared mount so
that it receives mount events from /sysroot but not vice versa. That
achieves the same effect of preventing /var submount events from
propagating back to /sysroot while allowing /sysroot mount events to
propagate forward like every other system mount. See
mount_namespaces(7)[1] and the linux shared subtrees[2] documentation
for details on slave+shared mount propagation.

When /var is mounted in the initramfs, this is accomplished with
mount(2) syscalls. When /var is mounted after switching to the real
root, the mount propagation flags are applied as options in the
generated var.mount unit. This depends on a mount(8) feature that has
been present since util-linux 2.23. That's available in RHEL 7 and every
non-EOL Debian and Ubuntu release. Applying the propagation from
var.mount fixes a small race, too. Previously, if a /var submount was
added before /sysroot was made private, it would have propagated back
into /sysroot. That was possible since ostree-remount.service orders
itself after var.mount but not before any /var submounts.

1. https://man7.org/linux/man-pages/man7/mount_namespaces.7.html
2. https://docs.kernel.org/filesystems/sharedsubtree.html

Fixes: #2086

diff --git a/src/libostree/ostree-impl-system-generator.c b/src/libostree/ostree-impl-system-generator.c
index c5fe503287436aa77ce7e30606d8b8557fa2dd35..6968c7388480a4255453eed537450f3e83ad384e 100644 (file)
--- a/src/libostree/ostree-impl-system-generator.c
+++ b/src/libostree/ostree-impl-system-generator.c
@@ -232,6 +232,15 @@ fstab_generator (const char *ostree_target, const bool is_aboot, const char *nor
    *
    * Note that our unit doesn't run if systemd.volatile is enabled;
    * see https://github.com/ostreedev/ostree/pull/856
+   *
+   * To avoid having submounts of /var propagate into $stateroot/var, the mount
+   * is made with slave+shared propagation. This means that /var will receive
+   * mount events from the parent /sysroot mount, but not vice versa. Adding a
+   * shared peer group below the slave group means that submounts of /var will
+   * inherit normal shared propagation. See mount_namespaces(7), Linux
+   * Documentation/filesystems/sharedsubtree.txt and
+   * https://github.com/ostreedev/ostree/issues/2086. This also happens in
+   * ostree-prepare-root.c for the INITRAMFS_MOUNT_VAR case.
    */
   if (!g_output_stream_printf (outstream, &bytes_written, cancellable, error,
                                "##\n# Automatically generated by ostree-system-generator\n##\n\n"
@@ -243,7 +252,7 @@ fstab_generator (const char *ostree_target, const bool is_aboot, const char *nor
                                "[Mount]\n"
                                "Where=%s\n"
                                "What=%s\n"
-                               "Options=bind\n",
+                               "Options=bind,slave,shared\n",
                                var_path, stateroot_var_path))
     return FALSE;
   if (!g_output_stream_flush (outstream, cancellable, error))

diff --git a/src/switchroot/ostree-prepare-root.c b/src/switchroot/ostree-prepare-root.c
index 70bf78f6301a78294c460f38cc453c0a1a9be135..7754673eebeed02e40fc8537da1ce25b8d323aef 100644 (file)
--- a/src/switchroot/ostree-prepare-root.c
+++ b/src/switchroot/ostree-prepare-root.c
@@ -644,6 +644,16 @@ main (int argc, char *argv[])
     {
       if (mount ("../../var", TMP_SYSROOT "/var", NULL, MS_BIND | MS_SILENT, NULL) < 0)
         err (EXIT_FAILURE, "failed to bind mount ../../var to var");
+
+      /* To avoid having submounts of /var propagate into $stateroot/var, the
+       * mount is made with slave+shared propagation. See the comment in
+       * ostree-impl-system-generator.c when /var isn't mounted in the
+       * initramfs for further explanation.
+       */
+      if (mount (NULL, TMP_SYSROOT "/var", NULL, MS_SLAVE | MS_SILENT, NULL) < 0)
+        err (EXIT_FAILURE, "failed to change /var to slave mount");
+      if (mount (NULL, TMP_SYSROOT "/var", NULL, MS_SHARED | MS_SILENT, NULL) < 0)
+        err (EXIT_FAILURE, "failed to change /var to slave+shared mount");
     }
 
   /* This can be used by other things to signal ostree is in use */
@@ -687,16 +697,5 @@ main (int argc, char *argv[])
         err (EXIT_FAILURE, "failed to make /sysroot read-only");
     }
 
-  /* The /sysroot mount needs to be private to avoid having a mount for e.g. /var/cache
-   * also propagate to /sysroot/ostree/deploy/$stateroot/var/cache
-   *
-   * Now in reality, today this is overridden by systemd: the *actual* way we fix this up
-   * is in ostree-remount.c.  But let's do it here to express the semantics we want
-   * at the very start (perhaps down the line systemd will have compile/runtime option
-   * to say that the initramfs environment did everything right from the start).
-   */
-  if (mount ("none", "sysroot", NULL, MS_PRIVATE | MS_SILENT, NULL) < 0)
-    err (EXIT_FAILURE, "remounting 'sysroot' private");
-
   exit (EXIT_SUCCESS);
 }

diff --git a/src/switchroot/ostree-remount.c b/src/switchroot/ostree-remount.c
index 3babb75141cc367f703e61143d338ad32cd24184..f0a4b3d925bee22aaab6216346c5d37f7fcaac3b 100644 (file)
--- a/src/switchroot/ostree-remount.c
+++ b/src/switchroot/ostree-remount.c
@@ -167,15 +167,6 @@ main (int argc, char *argv[])
   }
   g_autoptr (GVariantDict) ostree_run_metadata = g_variant_dict_new (ostree_run_metadata_v);
 
-  /* The /sysroot mount needs to be private to avoid having a mount for e.g. /var/cache
-   * also propagate to /sysroot/ostree/deploy/$stateroot/var/cache
-   *
-   * Today systemd remounts / (recursively) as shared, so we're undoing that as early
-   * as possible.  See also a copy of this in ostree-prepare-root.c.
-   */
-  if (mount ("none", "/sysroot", NULL, MS_REC | MS_PRIVATE, NULL) < 0)
-    perror ("warning: While remounting /sysroot MS_PRIVATE");
-
   const char *transient_etc = NULL;
   g_variant_dict_lookup (ostree_run_metadata, OTCORE_RUN_BOOTED_KEY_TRANSIENT_ETC, "&s",
                          &transient_etc);

diff --git a/tests/kolainst/destructive/mount-propagation.sh b/tests/kolainst/destructive/mount-propagation.sh
index b7fc87e696b54258701c3bc8a92c04fce3d9b588..981a04b768650523071ba3bd9a8254cbcd0b1f04 100755 (executable)
--- a/tests/kolainst/destructive/mount-propagation.sh
+++ b/tests/kolainst/destructive/mount-propagation.sh
@@ -67,10 +67,9 @@ test_mounts() {
   assert_has_mount /sysroot/bar
   assert_not_has_mount "/sysroot/ostree/deploy/${stateroot}/var/foo"
 
-  # Repeat with the sub mount namespace. Since /sysroot is marked private,
-  # /sysroot/bar will not be propagated into it.
+  # Repeat with the sub mount namespace.
   assert_has_mount /var/foo "${ns_pid}"
-  assert_not_has_mount /sysroot/bar "${ns_pid}"
+  assert_has_mount /sysroot/bar "${ns_pid}"
   assert_not_has_mount "/sysroot/ostree/deploy/${stateroot}/var/foo" "${ns_pid}"
 }