From e79b8c2092308b63202666af4dae16600c318b22 Mon Sep 17 00:00:00 2001 From: Elliott Hughes Date: Tue, 28 Jul 2020 11:09:03 -0700 Subject: [PATCH] Cleanup for #inclusivefixit. Test: treehugger Change-Id: I651689e2d59c017a9bde44251d95b57e594f0b5b --- init/mount_namespace.cpp | 59 +++++++++------------------------------- init/service_utils.cpp | 11 ++++---- 2 files changed, 19 insertions(+), 51 deletions(-) diff --git a/init/mount_namespace.cpp b/init/mount_namespace.cpp index f8359bc64..59cc140e4 100644 --- a/init/mount_namespace.cpp +++ b/init/mount_namespace.cpp @@ -44,50 +44,17 @@ namespace android { namespace init { namespace { -static bool BindMount(const std::string& source, const std::string& mount_point, - bool recursive = false) { - unsigned long mountflags = MS_BIND; - if (recursive) { - mountflags |= MS_REC; - } - if (mount(source.c_str(), mount_point.c_str(), nullptr, mountflags, nullptr) == -1) { +static bool BindMount(const std::string& source, const std::string& mount_point) { + if (mount(source.c_str(), mount_point.c_str(), nullptr, MS_BIND | MS_REC, nullptr) == -1) { PLOG(ERROR) << "Failed to bind mount " << source; return false; } return true; } -static bool MakeShared(const std::string& mount_point, bool recursive = false) { - unsigned long mountflags = MS_SHARED; - if (recursive) { - mountflags |= MS_REC; - } +static bool ChangeMount(const std::string& mount_point, unsigned long mountflags) { if (mount(nullptr, mount_point.c_str(), nullptr, mountflags, nullptr) == -1) { - PLOG(ERROR) << "Failed to change propagation type to shared"; - return false; - } - return true; -} - -static bool MakeSlave(const std::string& mount_point, bool recursive = false) { - unsigned long mountflags = MS_SLAVE; - if (recursive) { - mountflags |= MS_REC; - } - if (mount(nullptr, mount_point.c_str(), nullptr, mountflags, nullptr) == -1) { - PLOG(ERROR) << "Failed to change propagation type to slave"; - return false; - } - return true; -} - -static bool MakePrivate(const std::string& mount_point, bool recursive = false) { - unsigned long mountflags = MS_PRIVATE; - if (recursive) { - mountflags |= MS_REC; - } - if (mount(nullptr, mount_point.c_str(), nullptr, mountflags, nullptr) == -1) { - PLOG(ERROR) << "Failed to change propagation type to private"; + PLOG(ERROR) << "Failed to remount " << mount_point << " as " << std::hex << mountflags; return false; } return true; @@ -225,17 +192,17 @@ bool SetupMountNamespaces() { // needed for /foo/bar, then we will make /foo/bar as a mount point (by // bind-mounting by to itself) and set the propagation type of the mount // point to private. - if (!MakeShared("/", true /*recursive*/)) return false; + if (!ChangeMount("/", MS_SHARED | MS_REC)) return false; // /apex is a private mountpoint to give different sets of APEXes for // the bootstrap and default mount namespaces. The processes running with // the bootstrap namespace get APEXes from the read-only partition. - if (!(MakePrivate("/apex"))) return false; + if (!(ChangeMount("/apex", MS_PRIVATE))) return false; // /linkerconfig is a private mountpoint to give a different linker configuration // based on the mount namespace. Subdirectory will be bind-mounted based on current mount // namespace - if (!(MakePrivate("/linkerconfig"))) return false; + if (!(ChangeMount("/linkerconfig", MS_PRIVATE))) return false; // The two mount namespaces present challenges for scoped storage, because // vold, which is responsible for most of the mounting, lives in the @@ -266,15 +233,15 @@ bool SetupMountNamespaces() { if (!mkdir_recursive("/mnt/user", 0755)) return false; if (!mkdir_recursive("/mnt/installer", 0755)) return false; if (!mkdir_recursive("/mnt/androidwritable", 0755)) return false; - if (!(BindMount("/mnt/user", "/mnt/installer", true))) return false; - if (!(BindMount("/mnt/user", "/mnt/androidwritable", true))) return false; + if (!(BindMount("/mnt/user", "/mnt/installer"))) return false; + if (!(BindMount("/mnt/user", "/mnt/androidwritable"))) return false; // First, make /mnt/installer and /mnt/androidwritable a slave bind mount - if (!(MakeSlave("/mnt/installer"))) return false; - if (!(MakeSlave("/mnt/androidwritable"))) return false; + if (!(ChangeMount("/mnt/installer", MS_SLAVE))) return false; + if (!(ChangeMount("/mnt/androidwritable", MS_SLAVE))) return false; // Then, make it shared again - effectively creating a new peer group, that // will be inherited by new mount namespaces. - if (!(MakeShared("/mnt/installer"))) return false; - if (!(MakeShared("/mnt/androidwritable"))) return false; + if (!(ChangeMount("/mnt/installer", MS_SHARED))) return false; + if (!(ChangeMount("/mnt/androidwritable", MS_SHARED))) return false; bootstrap_ns_fd.reset(OpenMountNamespace()); bootstrap_ns_id = GetMountNamespaceId(); diff --git a/init/service_utils.cpp b/init/service_utils.cpp index 05e632b68..f2383d7a6 100644 --- a/init/service_utils.cpp +++ b/init/service_utils.cpp @@ -60,13 +60,14 @@ Result EnterNamespace(int nstype, const char* path) { Result SetUpMountNamespace(bool remount_proc, bool remount_sys) { constexpr unsigned int kSafeFlags = MS_NODEV | MS_NOEXEC | MS_NOSUID; - // Recursively remount / as slave like zygote does so unmounting and mounting /proc - // doesn't interfere with the parent namespace's /proc mount. This will also - // prevent any other mounts/unmounts initiated by the service from interfering - // with the parent namespace but will still allow mount events from the parent + // Recursively remount / as MS_SLAVE like zygote does so that + // unmounting and mounting /proc doesn't interfere with the parent + // namespace's /proc mount. This will also prevent any other + // mounts/unmounts initiated by the service from interfering with the + // parent namespace but will still allow mount events from the parent // namespace to propagate to the child. if (mount("rootfs", "/", nullptr, (MS_SLAVE | MS_REC), nullptr) == -1) { - return ErrnoError() << "Could not remount(/) recursively as slave"; + return ErrnoError() << "Could not remount(/) recursively as MS_SLAVE"; } // umount() then mount() /proc and/or /sys