minijail: Copy the mount flags from source when bind-mounting
This change makes all bindmounts copy the mount flags from the source
path if a remount is issued. It also fixes a bug where ro mounts could
never become rw.
Bug: 111325710
Test: make tests
Test: Repro scenario in https://crbug.com/862171
Change-Id: Ia87ea2933f1ab1b8a9fd2efd6f832c51a5a8f7a2
diff --git a/system_unittest.cc b/system_unittest.cc
index e0d4d2b..60d5c03 100644
--- a/system_unittest.cc
+++ b/system_unittest.cc
@@ -11,6 +11,7 @@
#include <stdlib.h>
#include <string.h>
#include <sys/stat.h>
+#include <sys/statvfs.h>
#include <unistd.h>
#include <gtest/gtest.h>
@@ -210,15 +211,39 @@
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));
+ EXPECT_EQ(0, setup_mount_destination(nullptr, kValidDir, 0, 0, false,
+ nullptr));
+ EXPECT_EQ(0, setup_mount_destination(nullptr, "/proc", 0, 0, true, nullptr));
+ EXPECT_EQ(0, setup_mount_destination(nullptr, "/dev", 0, 0, false, nullptr));
+}
+
+// Mount flags should be obtained for bind-mounts
+TEST(setup_mount_destination, mount_flags) {
+ struct statvfs stvfs_buf;
+ ASSERT_EQ(0, statvfs("/proc", &stvfs_buf));
+
+ std::string path = get_temp_path();
+ ASSERT_NE(std::string(), path);
+
+ unsigned long mount_flags = -1;
+ // Passing -1 for user ID/group ID tells chown to make no changes.
+ EXPECT_EQ(0, setup_mount_destination("/proc", path.c_str(), -1, -1, true,
+ &mount_flags));
+ EXPECT_EQ(stvfs_buf.f_flag, mount_flags);
+ EXPECT_EQ(0, rmdir(path.c_str()));
+
+ // Same thing holds for children of a mount.
+ mount_flags = -1;
+ EXPECT_EQ(0, setup_mount_destination("/proc/self", path.c_str(), -1, -1, true,
+ &mount_flags));
+ EXPECT_EQ(stvfs_buf.f_flag, mount_flags);
+ EXPECT_EQ(0, rmdir(path.c_str()));
}
// 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));
+ EXPECT_NE(0, setup_mount_destination("foo", kNoSuchDir, 0, 0, true, nullptr));
}
// A mount of a pseudo filesystem should make the destination dir.
@@ -227,7 +252,8 @@
ASSERT_NE(std::string(), path);
// Passing -1 for user ID/group ID tells chown to make no changes.
- EXPECT_EQ(0, setup_mount_destination("none", path.c_str(), -1, -1, false));
+ EXPECT_EQ(0, setup_mount_destination("none", path.c_str(), -1, -1, false,
+ nullptr));
// We check it's a directory by deleting it as such.
EXPECT_EQ(0, rmdir(path.c_str()));
@@ -237,15 +263,18 @@
// invalid user ID, just skip this check.
if (!is_android()) {
EXPECT_NE(0, setup_mount_destination("none", path.c_str(),
- UINT_MAX / 2, UINT_MAX / 2, false));
+ UINT_MAX / 2, UINT_MAX / 2, false,
+ nullptr));
}
}
// 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));
+ EXPECT_NE(0, setup_mount_destination(kNoSuchDir, kNoSuchDir, 0, 0, false,
+ nullptr));
+ EXPECT_NE(0, setup_mount_destination(kNoSuchDir, kNoSuchDir, 0, 0, true,
+ nullptr));
}
// A bind mount of a directory should create the destination dir.
@@ -254,7 +283,8 @@
ASSERT_NE(std::string(), path);
// Passing -1 for user ID/group ID tells chown to make no changes.
- EXPECT_EQ(0, setup_mount_destination(kValidDir, path.c_str(), -1, -1, true));
+ EXPECT_EQ(0, setup_mount_destination(kValidDir, path.c_str(), -1, -1, true,
+ nullptr));
// We check it's a directory by deleting it as such.
EXPECT_EQ(0, rmdir(path.c_str()));
}
@@ -265,7 +295,8 @@
ASSERT_NE(std::string(), path);
// Passing -1 for user ID/group ID tells chown to make no changes.
- EXPECT_EQ(0, setup_mount_destination(kValidFile, path.c_str(), -1, -1, true));
+ EXPECT_EQ(0, setup_mount_destination(kValidFile, path.c_str(), -1, -1, true,
+ nullptr));
// We check it's a file by deleting it as such.
EXPECT_EQ(0, unlink(path.c_str()));
}
@@ -276,7 +307,8 @@
ASSERT_NE(std::string(), path);
// Passing -1 for user ID/group ID tells chown to make no changes.
- EXPECT_EQ(0, setup_mount_destination(kValidCharDev, path.c_str(), -1, -1, false));
+ EXPECT_EQ(0, setup_mount_destination(kValidCharDev, path.c_str(), -1, -1,
+ false, nullptr));
// We check it's a directory by deleting it as such.
EXPECT_EQ(0, rmdir(path.c_str()));
}
