fs_mgr_overlayfs: Fix submount propagation type after remount
When juggling submounts, the temp dir holding the references to
submounts should be MS_PRIVATE. Otherwise the submount propagation type
would be changed according to section 5d,
https://www.kernel.org/doc/Documentation/filesystems/sharedsubtree.txt
For example, a root mount tree of
<mountpoint> <shared/private>
/dev shared
/product shared
/product/app private
When executing "remount /product", "/product/app" would be moved to a
temp dir under "/dev", which is MS_SHARED. This would also change the
state of the mount to MS_SHARED.
/dev shared
/product shared
/dev/temp shared (moved from /product/app)
Thus after "/product" overlay is mounted, and "/dev/temp" is moved back,
"/product/app" would be unintentionally changed to MS_SHARED.
/dev shared
/product private
/product shared (overlayfs override)
/product/app shared (moved from /dev/temp)
This change fixes this issue by setting up a standalone mount tree under
/dev to hold the temporary moved submounts.
This way we don't need to modify the state of "/dev" whatsoever, and can
guarantee the propagation type of mounts moved in and out of the temp
dir are not changed.
Bug: 306124139
Test: adb-remount-test
Test: remount a parent mount and then verify the mount flag of submount
Change-Id: I8174257cff6b93dd95303c6ab49b42f90b02df0c
diff --git a/fs_mgr/fs_mgr_overlayfs_mount.cpp b/fs_mgr/fs_mgr_overlayfs_mount.cpp
index ae7ed4c..4cadeec 100644
--- a/fs_mgr/fs_mgr_overlayfs_mount.cpp
+++ b/fs_mgr/fs_mgr_overlayfs_mount.cpp
@@ -34,6 +34,7 @@
#include <android-base/file.h>
#include <android-base/macros.h>
#include <android-base/properties.h>
+#include <android-base/scopeguard.h>
#include <android-base/strings.h>
#include <android-base/unique_fd.h>
#include <ext4_utils/ext4_utils.h>
@@ -58,6 +59,9 @@
constexpr char kCacheMountPoint[] = "/cache";
constexpr char kPhysicalDevice[] = "/dev/block/by-name/";
+// Mount tree to temporarily hold references to submounts.
+constexpr char kMoveMountTempDir[] = "/dev/remount";
+
constexpr char kLowerdirOption[] = "lowerdir=";
constexpr char kUpperdirOption[] = "upperdir=";
constexpr char kWorkdirOption[] = "workdir=";
@@ -374,24 +378,21 @@
return info;
}
-static bool fs_mgr_overlayfs_mount(const FstabEntry& entry) {
- const auto mount_point = fs_mgr_mount_point(entry.mount_point);
- const auto options = fs_mgr_get_overlayfs_options(entry);
+static bool fs_mgr_overlayfs_mount_one(const FstabEntry& fstab_entry) {
+ const auto mount_point = fs_mgr_mount_point(fstab_entry.mount_point);
+ const auto options = fs_mgr_get_overlayfs_options(fstab_entry);
if (options.empty()) return false;
- auto retval = true;
-
struct MoveEntry {
std::string mount_point;
std::string dir;
bool shared_flag;
};
-
std::vector<MoveEntry> moved_mounts;
- auto parent_private = false;
- auto parent_made_private = false;
- auto dev_private = false;
- auto dev_made_private = false;
+
+ bool retval = true;
+ bool move_dir_shared = true;
+ bool parent_shared = true;
// There could be multiple mount entries with the same mountpoint.
// Group these entries together with stable_sort, and keep only the last entry of a group.
@@ -411,19 +412,24 @@
// mountinfo is reversed twice, so still is in lexical sorted order.
for (const auto& entry : mountinfo) {
- if ((entry.mount_point == mount_point) && !entry.shared_flag) {
- parent_private = true;
+ if (entry.mount_point == kMoveMountTempDir) {
+ move_dir_shared = entry.shared_flag;
}
- if ((entry.mount_point == "/dev") && !entry.shared_flag) {
- dev_private = true;
+ if (entry.mount_point == mount_point ||
+ (mount_point == "/system" && entry.mount_point == "/")) {
+ parent_shared = entry.shared_flag;
}
}
+ // Precondition is that kMoveMountTempDir is MS_PRIVATE, otherwise don't try to move any
+ // submount in to or out of it.
+ if (move_dir_shared) {
+ mountinfo.clear();
+ }
+
// Need to make the original mountpoint MS_PRIVATE, so that the overlayfs can be MS_MOVE.
// This could happen if its parent mount is remounted later.
- if (!parent_private) {
- parent_made_private = fs_mgr_overlayfs_set_shared_mount(mount_point, false);
- }
+ fs_mgr_overlayfs_set_shared_mount(mount_point, false);
for (const auto& entry : mountinfo) {
// Find all immediate submounts.
@@ -440,8 +446,8 @@
// mountinfo is in lexical order, so no need to worry about |entry| being a parent mount of
// entries of |moved_mounts|.
- // use as the bound directory in /dev.
- MoveEntry new_entry{entry.mount_point, "/dev/TemporaryDir-XXXXXX", entry.shared_flag};
+ MoveEntry new_entry{entry.mount_point, kMoveMountTempDir + "/TemporaryDir-XXXXXX"s,
+ entry.shared_flag};
{
AutoSetFsCreateCon createcon;
auto new_context = fs_mgr_get_context(entry.mount_point);
@@ -497,10 +503,6 @@
// Move submounts back.
for (const auto& entry : moved_mounts) {
- if (!dev_private && !dev_made_private) {
- dev_made_private = fs_mgr_overlayfs_set_shared_mount("/dev", false);
- }
-
if (!fs_mgr_overlayfs_move_mount(entry.dir, entry.mount_point)) {
retval = false;
} else if (entry.shared_flag &&
@@ -509,10 +511,8 @@
}
rmdir(entry.dir.c_str());
}
- if (dev_made_private) {
- fs_mgr_overlayfs_set_shared_mount("/dev", true);
- }
- if (parent_made_private) {
+ // If the original (overridden) mount was MS_SHARED, then set the overlayfs mount to MS_SHARED.
+ if (parent_shared) {
fs_mgr_overlayfs_set_shared_mount(mount_point, true);
}
@@ -730,6 +730,25 @@
if (!OverlayfsSetupAllowed()) {
return false;
}
+
+ // Ensure kMoveMountTempDir is standalone mount tree with 'private' propagation by bind mounting
+ // to itself and set to MS_PRIVATE.
+ // Otherwise mounts moved in to it would have their propagation type changed unintentionally.
+ // Section 5d, https://www.kernel.org/doc/Documentation/filesystems/sharedsubtree.txt
+ if (!fs_mgr_overlayfs_already_mounted(kMoveMountTempDir, false)) {
+ if (mkdir(kMoveMountTempDir, 0755) && errno != EEXIST) {
+ PERROR << "mkdir " << kMoveMountTempDir;
+ }
+ if (mount(kMoveMountTempDir, kMoveMountTempDir, nullptr, MS_BIND, nullptr)) {
+ PERROR << "bind mount " << kMoveMountTempDir;
+ }
+ }
+ fs_mgr_overlayfs_set_shared_mount(kMoveMountTempDir, false);
+ android::base::ScopeGuard umountDir([]() {
+ umount(kMoveMountTempDir);
+ rmdir(kMoveMountTempDir);
+ });
+
auto ret = true;
auto scratch_can_be_mounted = !fs_mgr_overlayfs_already_mounted(kScratchMountPoint, false);
for (const auto& entry : fs_mgr_overlayfs_candidate_list(*fstab)) {
@@ -742,7 +761,7 @@
scratch_can_be_mounted = false;
TryMountScratch();
}
- ret &= fs_mgr_overlayfs_mount(entry);
+ ret &= fs_mgr_overlayfs_mount_one(entry);
}
return ret;
}