From 0db4b7a95bc080cc48f190210b7752af4e8e2794 Mon Sep 17 00:00:00 2001 From: Luis Hector Chavez Date: Wed, 21 Feb 2018 21:19:07 -0800 Subject: [PATCH] Change the remount command to be more container-friendly Passing in MS_REMOUNT | MS_BIND tells the kernel to only remove the MS_RDONLY flag and keep all the other flags the same. This is also the only kind of remount that is allowed in Linux containers (to be more precise, within user namespaces). This change also attempts to always run the remount command when dealing with /, since containers will almost always run a loop device, and since the number of the device changes, it is not convenient to put it in the fstab. Plus, the container won't have permission to modify it, but might be able to perform the remount. Bug: 72178046 Test: `adb remount` works in both sailfish and Chrome OS Change-Id: I9e8ec8afcd57f67875a312824667768b3aa89faa --- adb/remount_service.cpp | 26 +++++++++++++++++++------- 1 file changed, 19 insertions(+), 7 deletions(-) diff --git a/adb/remount_service.cpp b/adb/remount_service.cpp index 32ed28f59..a4c7a5a3d 100644 --- a/adb/remount_service.cpp +++ b/adb/remount_service.cpp @@ -62,9 +62,9 @@ static std::string find_fstab_mount(const char* dir) { // The proc entry for / is full of lies, so check fstab instead. // /proc/mounts lists rootfs and /dev/root, neither of which is what we want. -static std::string find_mount(const char* dir) { - if (strcmp(dir, "/") == 0) { - return find_fstab_mount(dir); +static std::string find_mount(const char* dir, bool is_root) { + if (is_root) { + return find_fstab_mount(dir); } else { return find_proc_mount(dir); } @@ -86,17 +86,29 @@ static bool remount_partition(int fd, const char* dir) { if (!directory_exists(dir)) { return true; } - std::string dev = find_mount(dir); - if (dev.empty()) { + bool is_root = strcmp(dir, "/") == 0; + std::string dev = find_mount(dir, is_root); + // Even if the device for the root is not found, we still try to remount it + // as rw. This typically only happens when running Android in a container: + // the root will almost always be in a loop device, which is dynamic, so + // it's not convenient to put in the fstab. + if (dev.empty() && !is_root) { return true; } - if (!make_block_device_writable(dev)) { + if (!dev.empty() && !make_block_device_writable(dev)) { WriteFdFmt(fd, "remount of %s failed; couldn't make block device %s writable: %s\n", dir, dev.c_str(), strerror(errno)); return false; } + if (mount(dev.c_str(), dir, "none", MS_REMOUNT | MS_BIND, nullptr) == -1) { + // This is useful for cases where the superblock is already marked as + // read-write, but the mount itself is read-only, such as containers + // where the remount with just MS_REMOUNT is forbidden by the kernel. + WriteFdFmt(fd, "remount of the %s mount failed: %s.\n", dir, strerror(errno)); + return false; + } if (mount(dev.c_str(), dir, "none", MS_REMOUNT, nullptr) == -1) { - WriteFdFmt(fd, "remount of %s failed: %s\n", dir, strerror(errno)); + WriteFdFmt(fd, "remount of the %s superblock failed: %s\n", dir, strerror(errno)); return false; } return true;