extend -K to accept a mount propagation typeandroid-p-preview-1

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

5 files changed, 94 insertions, 16 deletions

diff --git a/libminijail.c b/libminijail.c
index 900acd5..f9fb0e9 100644
--- a/libminijail.c
+++ b/libminijail.c
@@ -126,7 +126,6 @@ struct minijail {
 		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 minijail {
 	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 @@ void minijail_preenter(struct minijail *j)
 {
 	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 @@ void minijail_preenter(struct minijail *j)
 	j->flags.pid_file = 0;
 	j->flags.cgroups = 0;
 	j->flags.forward_signals = 0;
+	j->remount_mode = 0;
 }
 
 /*
@@ -234,7 +234,6 @@ void minijail_preexec(struct minijail *j)
 {
 	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 @@ void minijail_preexec(struct minijail *j)
 	/* 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 @@ void minijail_preexec(struct minijail *j)
 
 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 @@ void API minijail_skip_setting_securebits(struct minijail *j,
 	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 @@ static int enter_pivot_root(const struct minijail *j)
 		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 @@ void API minijail_enter(const struct minijail *j)
 		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 @@ void minijail_skip_setting_securebits(struct minijail *j,
  * 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 @@ must be an absolute path (e.g. \fI/dev/sda1\fR and not \fIsda1\fR).
 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 use_profile(struct minijail *j, const char *profile,
 	}
 }
 
+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 @@ static void usage(const char *progn)
 	       "  -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 parse_args(struct minijail *j, int argc, char * const argv[],
 	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 @@ int parse_args(struct minijail *j, int argc, char * const argv[],
 			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 @@ TEST_F(CliTest, invalid_mount) {
   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), "");
+}