Skip to content

Commit

Permalink
Add "transient" unlock
Browse files Browse the repository at this point in the history
I was thinking a bit more recently about the "live" changes
stuff coreos/rpm-ostree#639
(particularly since coreos/rpm-ostree#2060 )
and I realized reading the last debates in that issue that
there's really a much simpler solution; do exactly the same
thing we do for `ostree admin unlock`, except mount it read-only
by default.

Then, anything that wants to modify it does the same thing
libostree does for `/sysroot` and `/boot` as of recently; create
a new mount namespace and do the modifications there.

The advantages of this are numerous.  First, we already have
all of the code, it's basically just plumbing through a new
entry in the state enumeration and passing `MS_RDONLY` into
the `mount()` system call.

"live" changes here also naturally don't persist, unlike what
we are currently doing in rpm-ostree.
  • Loading branch information
cgwalters committed Aug 7, 2020
1 parent 621e1d7 commit f2773c1
Show file tree
Hide file tree
Showing 6 changed files with 77 additions and 9 deletions.
2 changes: 2 additions & 0 deletions src/libostree/ostree-deployment.c
Original file line number Diff line number Diff line change
Expand Up @@ -318,6 +318,8 @@ ostree_deployment_unlocked_state_to_string (OstreeDeploymentUnlockedState state)
return "hotfix";
case OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT:
return "development";
case OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT:
return "transient";
}
g_assert_not_reached ();
}
Expand Down
3 changes: 2 additions & 1 deletion src/libostree/ostree-deployment.h
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,8 @@ char *ostree_deployment_get_origin_relpath (OstreeDeployment *self);
typedef enum {
OSTREE_DEPLOYMENT_UNLOCKED_NONE,
OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT,
OSTREE_DEPLOYMENT_UNLOCKED_HOTFIX
OSTREE_DEPLOYMENT_UNLOCKED_HOTFIX,
OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT,
} OstreeDeploymentUnlockedState;

_OSTREE_PUBLIC
Expand Down
1 change: 1 addition & 0 deletions src/libostree/ostree-sysroot-private.h
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ struct OstreeSysroot {
#define _OSTREE_SYSROOT_RUNSTATE_STAGED_LOCKED "/run/ostree/staged-deployment-locked"
#define _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_DIR "/run/ostree/deployment-state/"
#define _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_DEVELOPMENT "unlocked-development"
#define _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_TRANSIENT "unlocked-transient"

gboolean
_ostree_sysroot_ensure_writable (OstreeSysroot *self,
Expand Down
28 changes: 21 additions & 7 deletions src/libostree/ostree-sysroot.c
Original file line number Diff line number Diff line change
Expand Up @@ -747,9 +747,13 @@ parse_deployment (OstreeSysroot *self,
ret_deployment->unlocked = OSTREE_DEPLOYMENT_UNLOCKED_NONE;
g_autofree char *unlocked_development_path =
_ostree_sysroot_get_runstate_path (ret_deployment, _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_DEVELOPMENT);
g_autofree char *unlocked_transient_path =
_ostree_sysroot_get_runstate_path (ret_deployment, _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_TRANSIENT);
struct stat stbuf;
if (lstat (unlocked_development_path, &stbuf) == 0)
ret_deployment->unlocked = OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT;
else if (lstat (unlocked_transient_path, &stbuf) == 0)
ret_deployment->unlocked = OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT;
else
{
GKeyFile *origin = ostree_deployment_get_origin (ret_deployment);
Expand Down Expand Up @@ -1932,6 +1936,8 @@ ostree_sysroot_deployment_unlock (OstreeSysroot *self,

const char *ovl_options = NULL;
static const char hotfix_ovl_options[] = "lowerdir=usr,upperdir=.usr-ovl-upper,workdir=.usr-ovl-work";
g_autofree char *unlock_ovldir = NULL;

switch (unlocked_state)
{
case OSTREE_DEPLOYMENT_UNLOCKED_NONE:
Expand All @@ -1951,11 +1957,12 @@ ostree_sysroot_deployment_unlock (OstreeSysroot *self,
}
break;
case OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT:
case OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT:
{
unlock_ovldir = g_strdup ("/var/tmp/ostree-unlock-ovl.XXXXXX");
/* We're just doing transient development/hacking? Okay,
* stick the overlayfs bits in /var/tmp.
*/
char *development_ovldir = strdupa ("/var/tmp/ostree-unlock-ovl.XXXXXX");
const char *development_ovl_upper;
const char *development_ovl_work;

Expand All @@ -1966,14 +1973,14 @@ ostree_sysroot_deployment_unlock (OstreeSysroot *self,
"/usr", usr_mode, error))
return FALSE;

if (g_mkdtemp_full (development_ovldir, 0755) == NULL)
if (g_mkdtemp_full (unlock_ovldir, 0755) == NULL)
return glnx_throw_errno_prefix (error, "mkdtemp");
}

development_ovl_upper = glnx_strjoina (development_ovldir, "/upper");
development_ovl_upper = glnx_strjoina (unlock_ovldir, "/upper");
if (!mkdir_unmasked (AT_FDCWD, development_ovl_upper, usr_mode, cancellable, error))
return FALSE;
development_ovl_work = glnx_strjoina (development_ovldir, "/work");
development_ovl_work = glnx_strjoina (unlock_ovldir, "/work");
if (!mkdir_unmasked (AT_FDCWD, development_ovl_work, usr_mode, cancellable, error))
return FALSE;
ovl_options = glnx_strjoina ("lowerdir=usr,upperdir=", development_ovl_upper,
Expand All @@ -1996,14 +2003,17 @@ ostree_sysroot_deployment_unlock (OstreeSysroot *self,
return glnx_throw_errno_prefix (error, "fork");
else if (mount_child == 0)
{
int mountflags = 0;
if (unlocked_state == OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT)
mountflags |= MS_RDONLY;
/* Child process. Do NOT use any GLib API here; it's not generally fork() safe.
*
* TODO: report errors across a pipe (or use the journal?) rather than
* spewing to stderr.
*/
if (fchdir (deployment_dfd) < 0)
err (1, "fchdir");
if (mount ("overlay", "/usr", "overlay", 0, ovl_options) < 0)
if (mount ("overlay", "/usr", "overlay", mountflags, ovl_options) < 0)
err (1, "mount");
exit (EXIT_SUCCESS);
}
Expand Down Expand Up @@ -2036,15 +2046,19 @@ ostree_sysroot_deployment_unlock (OstreeSysroot *self,
return FALSE;
break;
case OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT:
case OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT:
{
g_autofree char *devpath =
_ostree_sysroot_get_runstate_path (deployment, _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_DEVELOPMENT);
unlocked_state == OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT ?
_ostree_sysroot_get_runstate_path (deployment, _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_DEVELOPMENT)
:
_ostree_sysroot_get_runstate_path (deployment, _OSTREE_SYSROOT_DEPLOYMENT_RUNSTATE_FLAG_TRANSIENT);
g_autofree char *devpath_parent = dirname (g_strdup (devpath));

if (!glnx_shutil_mkdir_p_at (AT_FDCWD, devpath_parent, 0755, cancellable, error))
return FALSE;

if (!g_file_set_contents (devpath, "", 0, error))
if (!g_file_set_contents (devpath, unlock_ovldir, -1, error))
return FALSE;
}
}
Expand Down
18 changes: 17 additions & 1 deletion src/ostree/ot-admin-builtin-unlock.c
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,11 @@
#include <err.h>

static gboolean opt_hotfix;
static gboolean opt_transient;

static GOptionEntry options[] = {
{ "hotfix", 0, 0, G_OPTION_ARG_NONE, &opt_hotfix, "Retain changes across reboots", NULL },
{ "transient", 0, 0, G_OPTION_ARG_NONE, &opt_transient, "Mount overlayfs read-only by default", NULL },
{ NULL }
};

Expand Down Expand Up @@ -67,7 +69,17 @@ ot_admin_builtin_unlock (int argc, char **argv, OstreeCommandInvocation *invocat
goto out;
}

target_state = opt_hotfix ? OSTREE_DEPLOYMENT_UNLOCKED_HOTFIX : OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT;
if (opt_hotfix && opt_transient)
{
glnx_throw (error, "Cannot specify both --hotfix and --transient");
goto out;
}
else if (opt_hotfix)
target_state = OSTREE_DEPLOYMENT_UNLOCKED_HOTFIX;
else if (opt_transient)
target_state = OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT;
else
target_state = OSTREE_DEPLOYMENT_UNLOCKED_DEVELOPMENT;

if (!ostree_sysroot_deployment_unlock (sysroot, booted_deployment,
target_state, cancellable, error))
Expand All @@ -87,6 +99,10 @@ ot_admin_builtin_unlock (int argc, char **argv, OstreeCommandInvocation *invocat
g_print ("Development mode enabled. A writable overlayfs is now mounted on /usr.\n"
"All changes there will be discarded on reboot.\n");
break;
case OSTREE_DEPLOYMENT_UNLOCKED_TRANSIENT:
g_print ("A writable overlayfs is prepared for /usr, but is mounted read-only by default.\n"
"All changes there will be discarded on reboot.\n");
break;
}

ret = TRUE;
Expand Down
34 changes: 34 additions & 0 deletions tests/kolainst/destructive/unlock-transient.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
#!/bin/bash
# Test unlock --transient
set -xeuo pipefail

. ${KOLA_EXT_DATA}/libinsttest.sh

testfile=/usr/share/writable-usr-test

case "${AUTOPKGTEST_REBOOT_MARK:-}" in
"")
require_writable_sysroot
assert_not_has_file "${testfile}"
ostree admin unlock --transient
# It's still read-only
if touch ${testfile}; then
fatal "modified /usr"
fi
# But, we can affect it in a new mount namespace
unshare -m -- /bin/sh -c 'mount -o remount,rw /usr && echo hello from transient unlock >'"${testfile}"
assert_file_has_content "${testfile}" "hello from transient unlock"
# Still can't write to it from the outer namespace
if touch ${testfile} || rm -v "${testfile}" 2>/dev/null; then
fatal "modified ${testfile}"
fi
/tmp/autopkgtest-reboot 2
;;
"2")
if test -f "${testfile}"; then
fatal "${testfile} persisted across reboot?"
fi
echo "ok unlock transient"
;;
*) fatal "Unexpected boot mark ${AUTOPKGTEST_REBOOT_MARK}"
esac

0 comments on commit f2773c1

Please sign in to comment.