Skip to content

Commit

Permalink
switchroot: Stop making /sysroot mount private
Browse files Browse the repository at this point in the history
Back in 2b8d586, /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.

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.

Fixes: #2086
  • Loading branch information
dbnicholson committed Aug 30, 2024
1 parent 877e870 commit 427bc4a
Show file tree
Hide file tree
Showing 4 changed files with 10 additions and 24 deletions.
9 changes: 8 additions & 1 deletion src/libostree/ostree-impl-system-generator.c
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,13 @@ fstab_generator (const char *ostree_target, const bool is_aboot, const char *nor
g_autoptr (GOutputStream) outstream = g_unix_output_stream_new (tmpf.fd, FALSE);
gsize bytes_written;
/* This code is inspired by systemd's fstab-generator.c.
*
* To avoid having submounts of /var propagate into
* /sysroot/ostree/deploy/$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 https://github.com/ostreedev/ostree/issues/2086.
*
* Note that our unit doesn't run if systemd.volatile is enabled;
* see https://github.com/ostreedev/ostree/pull/856
Expand All @@ -243,7 +250,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))
Expand Down
11 changes: 0 additions & 11 deletions src/switchroot/ostree-prepare-root.c
Original file line number Diff line number Diff line change
Expand Up @@ -687,16 +687,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);
}
9 changes: 0 additions & 9 deletions src/switchroot/ostree-remount.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 2 additions & 3 deletions tests/kolainst/destructive/mount-propagation.sh
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,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}"
}

Expand Down

0 comments on commit 427bc4a

Please sign in to comment.