abort when bind mounting a non-existent source path
If you try to pass a bogus path like -b /asdf,/asdf, minijail doesn't
mind and goes ahead and creates the destination (which also creates
the source), and then does a bind mount. We should instead abort --
if you really want to bind mount a new directory or file, the daemon
should explicitly create the path correctly.
For the -k option, we were stating the pseudo source which could lead
to bad behavior. e.g. If there was a file in the cwd named "none" or
"proc", we'd stat() it, and then change the destination setup logic.
The current behavior is also a little idiosyncratic: if the source
and dest are the same, there's no error, but if you try to mount to
a different path (-b /asdf,/foo), it'll fail. Or if you try to use
a chroot/pivot root, it'll fail.
We now enforce absolute paths for sources with the -b & -k options.
This shouldn't be a problem in general, and it makes the behavior a
bit more consistent.
Bug: None
Test: unittests pass
Test: betty VM boots and cheets_StartAndroid passes
Change-Id: I26310ba45b8e463533485de879a19e578d66b0e6
diff --git a/system_unittest.cc b/system_unittest.cc
index f4490ca..0ecfcde 100644
--- a/system_unittest.cc
+++ b/system_unittest.cc
@@ -29,6 +29,15 @@
// A random path that really really should not exist on the host.
const char kNoSuchDir[] = "/.x/..x/...x/path/should/not/exist/";
+// A random file that should exist.
+const char kValidFile[] = "/etc/passwd";
+
+// A random directory that should exist.
+const char kValidDir[] = "/";
+
+// A random character device that should exist.
+const char kValidCharDev[] = "/dev/null";
+
// Return a temp filename in the cwd that this test can manipulate.
// It will not exist when it returns, and the user has to free the memory.
char *get_temp_path() {
@@ -121,4 +130,80 @@
EXPECT_EQ(5u, fread(data, 1, sizeof(data), fp));
fclose(fp);
EXPECT_EQ(0, strcmp(data, "1234\n"));
+
+ free(path);
+}
+
+// If the destination exists, there's nothing to do.
+TEST(setup_mount_destination, dest_exists) {
+ // Pick some paths that should always exist. We pass in invalid pointers
+ // for other args so we crash if the dest check doesn't short circuit.
+ EXPECT_EQ(0, setup_mount_destination(nullptr, kValidDir, 0, 0, false));
+ EXPECT_EQ(0, setup_mount_destination(nullptr, "/proc", 0, 0, true));
+ EXPECT_EQ(0, setup_mount_destination(nullptr, "/dev", 0, 0, false));
+}
+
+// When given a bind mount where the source is relative, reject it.
+TEST(setup_mount_destination, reject_relative_bind) {
+ // Pick a destination we know doesn't exist.
+ EXPECT_NE(0, setup_mount_destination("foo", kNoSuchDir, 0, 0, true));
+}
+
+// A mount of a pseudo filesystem should make the destination dir.
+TEST(setup_mount_destination, create_pseudo_fs) {
+ char *path = get_temp_path();
+ ASSERT_NE(nullptr, path);
+
+ // Passing -1 for uid/gid tells chown to make no changes.
+ EXPECT_EQ(0, setup_mount_destination("none", path, -1, -1, false));
+ // We check it's a directory by deleting it as such.
+ EXPECT_EQ(0, rmdir(path));
+
+ free(path);
+}
+
+// If the source path does not exist, we should error out.
+TEST(setup_mount_destination, missing_source) {
+ // The missing dest path is so we can exercise the source logic.
+ EXPECT_NE(0, setup_mount_destination(kNoSuchDir, kNoSuchDir, 0, 0, false));
+ EXPECT_NE(0, setup_mount_destination(kNoSuchDir, kNoSuchDir, 0, 0, true));
+}
+
+// A bind mount of a directory should create the destination dir.
+TEST(setup_mount_destination, create_bind_dir) {
+ char *path = get_temp_path();
+ ASSERT_NE(nullptr, path);
+
+ // Passing -1 for uid/gid tells chown to make no changes.
+ EXPECT_EQ(0, setup_mount_destination(kValidDir, path, -1, -1, true));
+ // We check it's a directory by deleting it as such.
+ EXPECT_EQ(0, rmdir(path));
+
+ free(path);
+}
+
+// A bind mount of a file should create the destination file.
+TEST(setup_mount_destination, create_bind_file) {
+ char *path = get_temp_path();
+ ASSERT_NE(nullptr, path);
+
+ // Passing -1 for uid/gid tells chown to make no changes.
+ EXPECT_EQ(0, setup_mount_destination(kValidFile, path, -1, -1, true));
+ // We check it's a file by deleting it as such.
+ EXPECT_EQ(0, unlink(path));
+
+ free(path);
+}
+
+// A mount of a character device should create the destination char.
+TEST(setup_mount_destination, create_char_dev) {
+ char *path = get_temp_path();
+ ASSERT_NE(nullptr, path);
+
+ // Passing -1 for uid/gid tells chown to make no changes.
+ EXPECT_EQ(0, setup_mount_destination(kValidCharDev, path, -1, -1, false));
+ // We check it's a directory by deleting it as such.
+ EXPECT_EQ(0, rmdir(path));
+
+ free(path);
}
