extend -K to accept a mount propagation type
By default, minijail will mark all mounts as private. The -K flag
allows them to skip that step which will retain all existing mount
settings. We now have scenarios where we want to share some mount
points, so lets extend -K to accept the propagation mode. This lets
people use -Kslave and mark all the mount points as slaves.
Bug: chromium:813131
Test: `make check` and using -Kslave allows changes in the parent namespace
Change-Id: I571e402a383ecf60a6104f87ef97b76710a34d38
diff --git a/libminijail.c b/libminijail.c
index 900acd5..f9fb0e9 100644
--- a/libminijail.c
+++ b/libminijail.c
@@ -126,7 +126,6 @@
int set_ambient_caps : 1;
int vfs : 1;
int enter_vfs : 1;
- int skip_remount_private : 1;
int pids : 1;
int ipc : 1;
int uts : 1;
@@ -177,6 +176,7 @@
struct mountpoint *mounts_head;
struct mountpoint *mounts_tail;
size_t mounts_count;
+ unsigned long remount_mode;
size_t tmpfs_size;
char *cgroups[MAX_CGROUPS];
size_t cgroup_count;
@@ -216,7 +216,6 @@
{
j->flags.vfs = 0;
j->flags.enter_vfs = 0;
- j->flags.skip_remount_private = 0;
j->flags.remount_proc_ro = 0;
j->flags.pids = 0;
j->flags.do_init = 0;
@@ -224,6 +223,7 @@
j->flags.pid_file = 0;
j->flags.cgroups = 0;
j->flags.forward_signals = 0;
+ j->remount_mode = 0;
}
/*
@@ -234,7 +234,6 @@
{
int vfs = j->flags.vfs;
int enter_vfs = j->flags.enter_vfs;
- int skip_remount_private = j->flags.skip_remount_private;
int remount_proc_ro = j->flags.remount_proc_ro;
int userns = j->flags.userns;
if (j->user)
@@ -248,7 +247,6 @@
/* Now restore anything we meant to keep. */
j->flags.vfs = vfs;
j->flags.enter_vfs = enter_vfs;
- j->flags.skip_remount_private = skip_remount_private;
j->flags.remount_proc_ro = remount_proc_ro;
j->flags.userns = userns;
/* Note, |pids| will already have been used before this call. */
@@ -258,7 +256,9 @@
struct minijail API *minijail_new(void)
{
- return calloc(1, sizeof(struct minijail));
+ struct minijail *j = calloc(1, sizeof(struct minijail));
+ j->remount_mode = MS_PRIVATE;
+ return j;
}
void API minijail_change_uid(struct minijail *j, uid_t uid)
@@ -441,9 +441,14 @@
j->securebits_skip_mask = securebits_skip_mask;
}
+void API minijail_remount_mode(struct minijail *j, unsigned long mode)
+{
+ j->remount_mode = mode;
+}
+
void API minijail_skip_remount_private(struct minijail *j)
{
- j->flags.skip_remount_private = 1;
+ j->remount_mode = 0;
}
void API minijail_namespace_pids(struct minijail *j)
@@ -1486,7 +1491,7 @@
pdie("failed to fchdir to old /");
/*
- * If j->flags.skip_remount_private was enabled for minijail_enter(),
+ * If skip_remount_private was enabled for minijail_enter(),
* there could be a shared mount point under |oldroot|. In that case,
* mounts under this shared mount point will be unmounted below, and
* this unmounting will propagate to the original mount namespace
@@ -1945,13 +1950,13 @@
if (unshare(CLONE_NEWNS))
pdie("unshare(CLONE_NEWNS) failed");
/*
- * Unless asked not to, remount all filesystems as private.
- * If they are shared, new bind mounts will creep out of our
- * namespace.
+ * By default, remount all filesystems as private, unless
+ * - Passed a specific remount mode, in which case remount with that,
+ * - Asked not to remount at all, in which case skip the mount(2) call.
* https://www.kernel.org/doc/Documentation/filesystems/sharedsubtree.txt
*/
- if (!j->flags.skip_remount_private) {
- if (mount(NULL, "/", NULL, MS_REC | MS_PRIVATE, NULL))
+ if (j->remount_mode) {
+ if (mount(NULL, "/", NULL, MS_REC | j->remount_mode, NULL))
pdie("mount(NULL, /, NULL, MS_REC | MS_PRIVATE,"
" NULL) failed");
}
diff --git a/libminijail.h b/libminijail.h
index 04e4153..2200422 100644
--- a/libminijail.h
+++ b/libminijail.h
@@ -101,6 +101,7 @@
* minijail_namespace_vfs(). You very likely don't need this.
*/
void minijail_skip_remount_private(struct minijail *j);
+void minijail_remount_mode(struct minijail *j, unsigned long mode);
void minijail_namespace_ipc(struct minijail *j);
void minijail_namespace_uts(struct minijail *j);
int minijail_namespace_set_hostname(struct minijail *j, const char *name);
diff --git a/minijail0.1 b/minijail0.1
index 7c535e0..ce8e67c 100644
--- a/minijail0.1
+++ b/minijail0.1
@@ -84,10 +84,28 @@
If the destination does not exist, it will be created as a directory (including
missing parent directories).
.TP
-\fB-K\fR
+\fB-K[mode]\fR
Don't mark all existing mounts as MS_PRIVATE.
This option is \fBdangerous\fR as it negates most of the functionality of \fB-v\fR.
You very likely don't need this.
+
+You may specify a mount propagation mode in which case, that will be used
+instead of the default MS_PRIVATE. See the \fBmount\fR(2) man page and the
+kernel docs \fIDocumentation/filesystems/sharedsubtree.txt\fR for more
+technical details, but a brief guide:
+
+.IP
+\[bu] \fBslave\fR Changes in the parent mount namespace will propagate in, but
+changes in this mount namespace will not propagate back out. This is usually
+what people want to use.
+.IP
+\[bu] \fBprivate\fR No changes in either mount namespace will propagate.
+This is the default behavior if you don't specify \fB-K\fR.
+.IP
+\[bu] \fBshared\fR Changes in the parent and this mount namespace will freely
+propagate back and forth. This is not recommended.
+.IP
+\[bu] \fBunbindable\fR Mark all mounts as unbindable.
.TP
\fB-l\fR
Run inside a new IPC namespace. This option makes the program's System V IPC
diff --git a/minijail0_cli.c b/minijail0_cli.c
index 8d3240e..b6aa593 100644
--- a/minijail0_cli.c
+++ b/minijail0_cli.c
@@ -10,6 +10,7 @@
#include <stdlib.h>
#include <string.h>
#include <sys/capability.h>
+#include <sys/mount.h>
#include <sys/types.h>
#include <unistd.h>
@@ -335,6 +336,24 @@
}
}
+static void set_remount_mode(struct minijail *j, const char *mode)
+{
+ unsigned long msmode;
+ if (!strcmp(mode, "shared"))
+ msmode = MS_SHARED;
+ else if (!strcmp(mode, "private"))
+ msmode = MS_PRIVATE;
+ else if (!strcmp(mode, "slave"))
+ msmode = MS_SLAVE;
+ else if (!strcmp(mode, "unbindable"))
+ msmode = MS_UNBINDABLE;
+ else {
+ fprintf(stderr, "Unknown remount mode: '%s'\n", mode);
+ exit(1);
+ }
+ minijail_remount_mode(j, msmode);
+}
+
static void usage(const char *progn)
{
size_t i;
@@ -373,7 +392,8 @@
" -H: Seccomp filter help message.\n"
" -i: Exit immediately after fork (do not act as init).\n"
" -I: Run <program> as init (pid 1) inside a new pid namespace (implies -p).\n"
- " -K: Don't mark all existing mounts as MS_PRIVATE.\n"
+ " -K: Do not change share mode of any existing mounts.\n"
+ " -K<mode>: Mark all existing mounts as <mode> instead of MS_PRIVATE.\n"
" -l: Enter new IPC namespace.\n"
" -L: Report blocked syscalls to syslog when using seccomp filter.\n"
" Forces the following syscalls to be allowed:\n"
@@ -457,7 +477,7 @@
int log_to_stderr = 0;
const char *optstring =
- "+u:g:sS:c:C:P:b:B:V:f:m::M::k:a:e::R:T:vrGhHinNplLt::IUKwyYzd";
+ "+u:g:sS:c:C:P:b:B:V:f:m::M::k:a:e::R:T:vrGhHinNplLt::IUK::wyYzd";
/* clang-format off */
const struct option long_options[] = {
{"help", no_argument, 0, 'h'},
@@ -535,7 +555,10 @@
add_mount(j, optarg);
break;
case 'K':
- minijail_skip_remount_private(j);
+ if (optarg)
+ set_remount_mode(j, optarg);
+ else
+ minijail_skip_remount_private(j);
skip_remount = 1;
break;
case 'P':
diff --git a/minijail0_cli_unittest.cc b/minijail0_cli_unittest.cc
index 2427a01..f74bb3d 100644
--- a/minijail0_cli_unittest.cc
+++ b/minijail0_cli_unittest.cc
@@ -415,3 +415,34 @@
argv[2] = "none,/";
ASSERT_EXIT(parse_args_(argv), testing::ExitedWithCode(1), "");
}
+
+// Valid calls to the remount mode option.
+TEST_F(CliTest, valid_remount_mode) {
+ std::vector<std::string> argv = {"-v", "", "/bin/sh"};
+
+ // Mode is optional.
+ argv[1] = "-K";
+ ASSERT_TRUE(parse_args_(argv));
+
+ // This should list all valid modes.
+ const std::vector<std::string> modes = {
+ "shared",
+ "private",
+ "slave",
+ "unbindable",
+ };
+
+ for (const auto& mode : modes) {
+ argv[1] = "-K" + mode;
+ ASSERT_TRUE(parse_args_(argv));
+ }
+}
+
+// Invalid calls to the remount mode option.
+TEST_F(CliTest, invalid_remount_mode) {
+ std::vector<std::string> argv = {"-v", "", "/bin/sh"};
+
+ // Unknown mode.
+ argv[1] = "-Kfoo";
+ ASSERT_EXIT(parse_args_(argv), testing::ExitedWithCode(1), "");
+}