sysroot: Handle ro /boot but rw /sysroot

The recent change in https://github.com/coreos/fedora-coreos-config/pull/659
broke some of our tests that do `mount -o remount,rw /sysroot` but
leave `/boot` read-only.

We had code for having `/boot` read-only before `/sysroot` but
in practice we had a file descriptor for `/sysroot` that we opened
before the remount that would happen later on.

Clean things up here so that in the library, we also remount
`/boot` at the same time we remount `/sysroot` if either are readonly.

Delete the legacy code for remounting `/boot` rw if we're not in
a mount namespace.  I am fairly confident most users are either
using the `ostree` CLI, or they're using the mount namespace.
This commit is contained in:
Colin Walters 2021-01-09 18:34:27 +00:00
parent a1c0cffeb3
commit 9a526bbaa5
3 changed files with 44 additions and 96 deletions

View File

@ -56,9 +56,6 @@
#define OSTREE_DEPLOYMENT_FINALIZING_ID SD_ID128_MAKE(e8,64,6c,d6,3d,ff,46,25,b7,79,09,a8,e7,a4,09,94)
#endif
static gboolean
is_ro_mount (const char *path);
/*
* Like symlinkat() but overwrites (atomically) an existing
* symlink.
@ -2225,57 +2222,6 @@ cleanup_legacy_current_symlinks (OstreeSysroot *self,
return TRUE;
}
/* Detect whether or not @path refers to a read-only mountpoint. This is
* currently just used to handle a potentially read-only /boot by transiently
* remounting it read-write. In the future we might also do this for e.g.
* /sysroot.
*/
static gboolean
is_ro_mount (const char *path)
{
#ifdef HAVE_LIBMOUNT
/* Dragging in all of this crud is apparently necessary just to determine
* whether something is a mount point.
*
* Systemd has a totally different implementation in
* src/basic/mount-util.c.
*/
struct libmnt_table *tb = mnt_new_table_from_file ("/proc/self/mountinfo");
struct libmnt_fs *fs;
struct libmnt_cache *cache;
gboolean is_mount = FALSE;
struct statvfs stvfsbuf;
if (!tb)
return FALSE;
/* to canonicalize all necessary paths */
cache = mnt_new_cache ();
mnt_table_set_cache (tb, cache);
fs = mnt_table_find_target(tb, path, MNT_ITER_BACKWARD);
is_mount = fs && mnt_fs_get_target (fs);
#ifdef HAVE_MNT_UNREF_CACHE
mnt_unref_table (tb);
mnt_unref_cache (cache);
#else
mnt_free_table (tb);
mnt_free_cache (cache);
#endif
if (!is_mount)
return FALSE;
/* We *could* parse the options, but it seems more reliable to
* introspect the actual mount at runtime.
*/
if (statvfs (path, &stvfsbuf) == 0)
return (stvfsbuf.f_flag & ST_RDONLY) != 0;
#endif
return FALSE;
}
/**
* ostree_sysroot_write_deployments:
* @self: Sysroot
@ -2577,42 +2523,13 @@ ostree_sysroot_write_deployments_with_options (OstreeSysroot *self,
}
else
{
gboolean boot_was_ro_mount = FALSE;
if (self->booted_deployment)
boot_was_ro_mount = is_ro_mount ("/boot");
g_debug ("boot is ro: %s", boot_was_ro_mount ? "yes" : "no");
if (boot_was_ro_mount)
{
if (mount ("/boot", "/boot", NULL, MS_REMOUNT | MS_SILENT, NULL) < 0)
return glnx_throw_errno_prefix (error, "Remounting /boot read-write");
}
if (!_ostree_sysroot_query_bootloader (self, &bootloader, cancellable, error))
return FALSE;
bootloader_is_atomic = bootloader != NULL && _ostree_bootloader_is_atomic (bootloader);
/* Note equivalent of try/finally here */
gboolean success = write_deployments_bootswap (self, new_deployments, opts, bootloader,
&syncstats, cancellable, error);
/* Below here don't set GError until the if (!success) check.
* Note we only bother remounting if a mount namespace isn't in use.
* */
if (boot_was_ro_mount && !self->mount_namespace_in_use)
{
if (mount ("/boot", "/boot", NULL, MS_REMOUNT | MS_RDONLY | MS_SILENT, NULL) < 0)
{
/* Only make this a warning because we don't want to
* completely bomb out if some other process happened to
* jump in and open a file there. See above TODO
* around doing this in a new mount namespace.
*/
g_printerr ("warning: Failed to remount /boot read-only: %s\n", strerror (errno));
}
}
if (!success)
if (!write_deployments_bootswap (self, new_deployments, opts, bootloader,
&syncstats, cancellable, error))
return FALSE;
}

View File

@ -295,6 +295,31 @@ _ostree_sysroot_ensure_boot_fd (OstreeSysroot *self, GError **error)
return TRUE;
}
static gboolean
remount_writable (const char *path, gboolean *did_remount, GError **error)
{
*did_remount = FALSE;
struct statvfs stvfsbuf;
if (statvfs (path, &stvfsbuf) < 0)
{
if (errno != ENOENT)
return glnx_throw_errno_prefix (error, "statvfs(%s)", path);
else
return TRUE;
}
if ((stvfsbuf.f_flag & ST_RDONLY) != 0)
{
/* OK, let's remount writable. */
if (mount (path, path, NULL, MS_REMOUNT | MS_RELATIME, "") < 0)
return glnx_throw_errno_prefix (error, "Remounting %s read-write", path);
*did_remount = TRUE;
g_debug ("remounted %s writable", path);
}
return TRUE;
}
/* Remount /sysroot read-write if necessary */
gboolean
_ostree_sysroot_ensure_writable (OstreeSysroot *self,
@ -314,19 +339,19 @@ _ostree_sysroot_ensure_writable (OstreeSysroot *self,
if (!self->root_is_ostree_booted)
return TRUE;
/* Check if /sysroot is a read-only mountpoint */
struct statvfs stvfsbuf;
if (statvfs ("/sysroot", &stvfsbuf) < 0)
return glnx_throw_errno_prefix (error, "fstatvfs(/sysroot)");
if ((stvfsbuf.f_flag & ST_RDONLY) == 0)
return TRUE;
/* In these cases we also require /boot */
if (!_ostree_sysroot_ensure_boot_fd (self, error))
return FALSE;
/* OK, let's remount writable. */
if (mount ("/sysroot", "/sysroot", NULL, MS_REMOUNT | MS_RELATIME, "") < 0)
return glnx_throw_errno_prefix (error, "Remounting /sysroot read-write");
gboolean did_remount_sysroot = FALSE;
if (!remount_writable ("/sysroot", &did_remount_sysroot, error))
return FALSE;
gboolean did_remount_boot = FALSE;
if (!remount_writable ("/boot", &did_remount_boot, error))
return FALSE;
/* Reopen our fd */
glnx_close_fd (&self->sysroot_fd);
/* Now close and reopen our file descriptors */
ostree_sysroot_unload (self);
if (!ensure_sysroot_fd (self, error))
return FALSE;

View File

@ -34,6 +34,12 @@
#define OT_VARIANT_BUILDER_INITIALIZER {{{0,}}}
#endif
static inline const char *
ot_booltostr (int b)
{
return b ? "true" : "false";
}
#define ot_gobject_refz(o) (o ? g_object_ref (o) : o)
#define ot_transfer_out_value(outp, srcp) G_STMT_START { \