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

system_unittest.cc

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()));
 }