From 7b4801a92140a27343ddc38f4b8f0a2c4995a9fb Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Mon, 25 Feb 2019 16:41:38 +0900 Subject: [PATCH 1/3] Don't bind-mount bionic files Bind-mounting of the bionic files on /bionic/* paths no longer required as there are direct symlinks from bionic files in /system partition to the corresponding bionic files in the runtime APEX. e.g., /system/lib/libc.so -> /apex/com.android.runtime/lib/bionic/libc.so Bug: 125549215 Test: m; devices boots Change-Id: I4a43101c3e3e2e14a81001d6d65a8a4b727df385 --- init/builtins.cpp | 9 --- init/mount_namespace.cpp | 120 ++------------------------------------ init/mount_namespace.h | 1 - rootdir/etc/ld.config.txt | 4 +- rootdir/init.rc | 2 - 5 files changed, 7 insertions(+), 129 deletions(-) diff --git a/init/builtins.cpp b/init/builtins.cpp index 6511d299e..8437e3790 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -1118,14 +1118,6 @@ static Result do_parse_apex_configs(const BuiltinArguments& args) { } } -static Result do_setup_runtime_bionic(const BuiltinArguments& args) { - if (SetupRuntimeBionic()) { - return Success(); - } else { - return Error() << "Failed to setup runtime bionic"; - } -} - static Result do_enter_default_mount_ns(const BuiltinArguments& args) { if (SwitchToDefaultMountNamespace()) { return Success(); @@ -1181,7 +1173,6 @@ const BuiltinFunctionMap::Map& BuiltinFunctionMap::map() const { {"rmdir", {1, 1, {true, do_rmdir}}}, {"setprop", {2, 2, {true, do_setprop}}}, {"setrlimit", {3, 3, {false, do_setrlimit}}}, - {"setup_runtime_bionic", {0, 0, {false, do_setup_runtime_bionic}}}, {"start", {1, 1, {false, do_start}}}, {"stop", {1, 1, {false, do_stop}}}, {"swapon_all", {1, 1, {false, do_swapon_all}}}, diff --git a/init/mount_namespace.cpp b/init/mount_namespace.cpp index 4161df22b..5305dc7ef 100644 --- a/init/mount_namespace.cpp +++ b/init/mount_namespace.cpp @@ -33,37 +33,6 @@ namespace android { namespace init { namespace { -static constexpr const char* kLinkerMountPoint = "/bionic/bin/linker"; -static constexpr const char* kBootstrapLinkerPath = "/system/bin/bootstrap/linker"; -static constexpr const char* kRuntimeLinkerPath = "/apex/com.android.runtime/bin/linker"; - -static constexpr const char* kBionicLibsMountPointDir = "/bionic/lib/"; -static constexpr const char* kBootstrapBionicLibsDir = "/system/lib/bootstrap/"; -static constexpr const char* kRuntimeBionicLibsDir = "/apex/com.android.runtime/lib/bionic/"; - -static constexpr const char* kLinkerMountPoint64 = "/bionic/bin/linker64"; -static constexpr const char* kBootstrapLinkerPath64 = "/system/bin/bootstrap/linker64"; -static constexpr const char* kRuntimeLinkerPath64 = "/apex/com.android.runtime/bin/linker64"; - -static constexpr const char* kBionicLibsMountPointDir64 = "/bionic/lib64/"; -static constexpr const char* kBootstrapBionicLibsDir64 = "/system/lib64/bootstrap/"; -static constexpr const char* kRuntimeBionicLibsDir64 = "/apex/com.android.runtime/lib64/bionic/"; - -static const std::vector kBionicLibFileNames = {"libc.so", "libm.so", "libdl.so"}; - -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) { - PLOG(ERROR) << "Could not bind-mount " << source << " to " << mount_point; - return false; - } - return true; -} - static bool MakeShared(const std::string& mount_point, bool recursive = false) { unsigned long mountflags = MS_SHARED; if (recursive) { @@ -105,34 +74,6 @@ static std::string GetMountNamespaceId() { return ret; } -static bool BindMountBionic(const std::string& linker_source, const std::string& lib_dir_source, - const std::string& linker_mount_point, - const std::string& lib_mount_dir) { - if (access(linker_source.c_str(), F_OK) != 0) { - PLOG(INFO) << linker_source << " does not exist. skipping mounting bionic there."; - // This can happen for 64-bit bionic in 32-bit only device. - // It is okay to skip mounting the 64-bit bionic. - return true; - } - if (!BindMount(linker_source, linker_mount_point)) { - return false; - } - if (!MakePrivate(linker_mount_point)) { - return false; - } - for (const auto& libname : kBionicLibFileNames) { - std::string mount_point = lib_mount_dir + libname; - std::string source = lib_dir_source + libname; - if (!BindMount(source, mount_point)) { - return false; - } - if (!MakePrivate(mount_point)) { - return false; - } - } - return true; -} - static bool IsApexUpdatable() { static bool updatable = android::sysprop::ApexProperties::updatable().value_or(false); return updatable; @@ -154,26 +95,7 @@ bool SetupMountNamespaces() { // point to private. if (!MakeShared("/", true /*recursive*/)) return false; - // Since different files (bootstrap or runtime APEX) should be mounted to - // the same mount point paths (e.g. /bionic/bin/linker, /bionic/lib/libc.so, - // etc.) across the two mount namespaces, we create a private mount point at - // /bionic so that a mount event for the bootstrap bionic in the mount - // namespace for pre-apexd processes is not propagated to the other mount - // namespace for post-apexd process, and vice versa. - // - // Other mount points other than /bionic, however, are all still shared. - if (!BindMount("/bionic", "/bionic", true /*recursive*/)) return false; - if (!MakePrivate("/bionic")) return false; - - // Bind-mount bootstrap bionic. - if (!BindMountBionic(kBootstrapLinkerPath, kBootstrapBionicLibsDir, kLinkerMountPoint, - kBionicLibsMountPointDir)) - return false; - if (!BindMountBionic(kBootstrapLinkerPath64, kBootstrapBionicLibsDir64, kLinkerMountPoint64, - kBionicLibsMountPointDir64)) - return false; - - // /apex is also a private mountpoint to give different sets of APEXes for + // /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; @@ -181,12 +103,11 @@ bool SetupMountNamespaces() { bootstrap_ns_fd.reset(OpenMountNamespace()); bootstrap_ns_id = GetMountNamespaceId(); - // When bionic is updatable via the runtime APEX, we create separate mount + // When APEXes are updatable (e.g. not-flattened), we create separate mount // namespaces for processes that are started before and after the APEX is - // activated by apexd. In the namespace for pre-apexd processes, the bionic - // from the /system partition (that we call bootstrap bionic) is - // bind-mounted. In the namespace for post-apexd processes, the bionic from - // the runtime APEX is bind-mounted. + // activated by apexd. In the namespace for pre-apexd processes, small + // number of essential APEXes (e.g. com.android.runtime) are activated. + // In the namespace for post-apexd processes, all APEXes are activated. bool success = true; if (IsApexUpdatable() && !IsRecoveryMode()) { // Creating a new namespace by cloning, saving, and switching back to @@ -198,15 +119,6 @@ bool SetupMountNamespaces() { default_ns_fd.reset(OpenMountNamespace()); default_ns_id = GetMountNamespaceId(); - // By this unmount, the bootstrap bionic are not mounted in the default - // mount namespace. - if (umount2("/bionic", MNT_DETACH) == -1) { - PLOG(ERROR) << "Cannot unmount /bionic"; - // Don't return here. We have to switch back to the bootstrap - // namespace. - success = false; - } - if (setns(bootstrap_ns_fd.get(), CLONE_NEWNS) == -1) { PLOG(ERROR) << "Cannot switch back to bootstrap mount namespace"; return false; @@ -237,28 +149,6 @@ bool SwitchToDefaultMountNamespace() { return true; } -// TODO(jiyong): remove this when /system/lib/libc.so becomes -// a symlink to /apex/com.android.runtime/lib/bionic/libc.so -bool SetupRuntimeBionic() { - if (IsRecoveryMode()) { - // We don't have multiple namespaces in recovery mode - return true; - } - // Bind-mount bionic from the runtime APEX since it is now available. Note - // that in case of IsApexUpdatable() == false, these mounts are over the - // existing existing bind mounts for the bootstrap bionic, which effectively - // becomes hidden. - if (!BindMountBionic(kRuntimeLinkerPath, kRuntimeBionicLibsDir, kLinkerMountPoint, - kBionicLibsMountPointDir)) - return false; - if (!BindMountBionic(kRuntimeLinkerPath64, kRuntimeBionicLibsDir64, kLinkerMountPoint64, - kBionicLibsMountPointDir64)) - return false; - - LOG(INFO) << "Runtime bionic is set up"; - return true; -} - bool SwitchToBootstrapMountNamespaceIfNeeded() { if (IsRecoveryMode()) { // we don't have multiple namespaces in recovery mode diff --git a/init/mount_namespace.h b/init/mount_namespace.h index 4eef7853e..c41a449f3 100644 --- a/init/mount_namespace.h +++ b/init/mount_namespace.h @@ -20,7 +20,6 @@ namespace android { namespace init { bool SetupMountNamespaces(); -bool SetupRuntimeBionic(); bool SwitchToDefaultMountNamespace(); bool SwitchToBootstrapMountNamespaceIfNeeded(); diff --git a/rootdir/etc/ld.config.txt b/rootdir/etc/ld.config.txt index 552d6856a..641a536bb 100644 --- a/rootdir/etc/ld.config.txt +++ b/rootdir/etc/ld.config.txt @@ -83,7 +83,7 @@ namespace.default.permitted.paths += /%PRODUCT_SERVICES%/app namespace.default.permitted.paths += /%PRODUCT_SERVICES%/priv-app namespace.default.permitted.paths += /data namespace.default.permitted.paths += /mnt/expand -namespace.default.permitted.paths += /bionic/${LIB} +namespace.default.permitted.paths += /apex/com.android.runtime/${LIB}/bionic namespace.default.permitted.paths += /system/${LIB}/bootstrap namespace.default.asan.search.paths = /data/asan/system/${LIB} @@ -119,7 +119,7 @@ namespace.default.asan.permitted.paths += /%PRODUCT_SERVICES%/framework namespace.default.asan.permitted.paths += /%PRODUCT_SERVICES%/app namespace.default.asan.permitted.paths += /%PRODUCT_SERVICES%/priv-app namespace.default.asan.permitted.paths += /mnt/expand -namespace.default.asan.permitted.paths += /bionic/${LIB} +namespace.default.asan.permitted.paths += /apex/com.android.runtime/${LIB}/bionic namespace.default.asan.permitted.paths += /system/${LIB}/bootstrap # Keep in sync with ld.config.txt in the com.android.runtime APEX. diff --git a/rootdir/init.rc b/rootdir/init.rc index 7cac97200..3298ffdf5 100644 --- a/rootdir/init.rc +++ b/rootdir/init.rc @@ -578,8 +578,6 @@ on post-fs-data # Wait for apexd to finish activating APEXes before starting more processes. wait_for_prop apexd.status ready - # TODO(jiyong): remove setup_runtime_bionic - setup_runtime_bionic parse_apex_configs init_user0 From fc97d2a1168c9764584cd01424433ee6742f15f9 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Thu, 28 Feb 2019 15:16:23 +0900 Subject: [PATCH 2/3] Revert "Handle adb sync with Bionic under /bionic" This reverts commit 7c7189c46948761cc668c9362d0b5aefb4004c70. Bug: 125549215 Test: system/core/fs_mgr/tests/adb-remount-test.sh Change-Id: I4ee40cda9c3b94b116dc822c7b9736cfe2c9c9f0 --- adb/daemon/file_sync_service.cpp | 86 -------------------------------- 1 file changed, 86 deletions(-) diff --git a/adb/daemon/file_sync_service.cpp b/adb/daemon/file_sync_service.cpp index 70deb31fd..29bd798d4 100644 --- a/adb/daemon/file_sync_service.cpp +++ b/adb/daemon/file_sync_service.cpp @@ -25,7 +25,6 @@ #include #include #include -#include #include #include #include @@ -211,22 +210,6 @@ done: return WriteFdExactly(s, &msg.dent, sizeof(msg.dent)); } -static bool is_mountpoint(const std::string& path, pid_t tid) { - const std::string mountinfo_path = "/proc/" + std::to_string(tid) + "/mountinfo"; - std::string mountinfo; - if (!android::base::ReadFileToString(mountinfo_path, &mountinfo)) { - PLOG(ERROR) << "Failed to open " << mountinfo_path; - return false; - } - std::vector lines = android::base::Split(mountinfo, "\n"); - return std::find_if(lines.begin(), lines.end(), [&path](const auto& line) { - auto tokens = android::base::Split(line, " "); - // line format is ... - // mountid parentmountid major:minor sourcepath targetpath option ... - return tokens.size() >= 4 && tokens[4] == path; - }) != lines.end(); -} - // Make sure that SendFail from adb_io.cpp isn't accidentally used in this file. #pragma GCC poison SendFail @@ -432,18 +415,6 @@ static bool do_send(int s, const std::string& spec, std::vector& buffer) { struct stat st; bool do_unlink = (lstat(path.c_str(), &st) == -1) || S_ISREG(st.st_mode) || (S_ISLNK(st.st_mode) && !S_ISLNK(mode)); - - // If the path is a file that is a mount point, don't unlink it, but instead - // truncate to zero. If unlinked, existing mounts on the path is all - // unmounted - if (S_ISREG(st.st_mode) && is_mountpoint(path, getpid())) { - do_unlink = false; - if (truncate(path.c_str(), 0) == -1) { - SendSyncFail(s, "truncate to zero failed"); - return false; - } - } - if (do_unlink) { adb_unlink(path.c_str()); } @@ -591,64 +562,7 @@ static bool handle_sync_command(int fd, std::vector& buffer) { return true; } -#if defined(__ANDROID__) -class FileSyncPreparer { - public: - FileSyncPreparer() : saved_ns_fd_(-1), rooted_(getuid() == 0) { - const std::string namespace_path = "/proc/" + std::to_string(gettid()) + "/ns/mnt"; - const int ns_fd = adb_open(namespace_path.c_str(), O_RDONLY | O_CLOEXEC); - if (ns_fd == -1) { - if (rooted_) PLOG(ERROR) << "Failed to save mount namespace"; - return; - } - saved_ns_fd_.reset(ns_fd); - - // Note: this is for the current thread only - if (unshare(CLONE_NEWNS) != 0) { - if (rooted_) PLOG(ERROR) << "Failed to clone mount namespace"; - return; - } - - // Set the propagation type of / to private so that unmount below is - // not propagated to other mount namespaces. - if (mount(nullptr, "/", nullptr, MS_PRIVATE | MS_REC, nullptr) == -1) { - if (rooted_) PLOG(ERROR) << "Could not change propagation type of / to MS_PRIVATE"; - return; - } - - // unmount /bionic which is bind-mount to itself by init. Under /bionic, - // there are other bind mounts for the bionic files. By unmounting this, - // we unmount them all thus revealing the raw file system that is the - // same as the local file system seen by the adb client. - if (umount2("/bionic", MNT_DETACH) == -1 && errno != ENOENT) { - if (rooted_) PLOG(ERROR) << "Could not unmount /bionic to reveal raw filesystem"; - return; - } - } - - ~FileSyncPreparer() { - if (saved_ns_fd_.get() != -1) { - // In fact, this is not strictly required because this thread for file - // sync service will be destroyed after the current transfer is all - // done. However, let's restore the ns in case the same thread is - // reused by multiple transfers in the future refactoring. - if (setns(saved_ns_fd_, CLONE_NEWNS) == -1) { - PLOG(ERROR) << "Failed to restore saved mount namespace"; - } - } - } - - private: - unique_fd saved_ns_fd_; - bool rooted_; -}; -#endif - void file_sync_service(unique_fd fd) { -#if defined(__ANDROID__) - FileSyncPreparer preparer; -#endif - std::vector buffer(SYNC_DATA_MAX); while (handle_sync_command(fd.get(), buffer)) { From 192fdeb4954a177388ef6a6d0fe3794dd054df3b Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Thu, 28 Feb 2019 15:17:22 +0900 Subject: [PATCH 3/3] /bionic path is gone The path no longer exists, and thus the path doesn't need to be unmounted upon remounting. Bug: 125549215 Test: system/core/fs_mgr/tests/adb-remount-test.sh Change-Id: I7b263c755ad7eeaa63a00ad9795a134707698625 --- fs_mgr/fs_mgr_remount.cpp | 25 ------------------------- 1 file changed, 25 deletions(-) diff --git a/fs_mgr/fs_mgr_remount.cpp b/fs_mgr/fs_mgr_remount.cpp index 5c4008cce..e2a4d1607 100644 --- a/fs_mgr/fs_mgr_remount.cpp +++ b/fs_mgr/fs_mgr_remount.cpp @@ -80,29 +80,6 @@ const android::fs_mgr::FstabEntry* is_wrapped(const android::fs_mgr::Fstab& over return &(*it); } -void try_unmount_bionic(android::fs_mgr::Fstab* mounts) { - static constexpr const char* kBionic = "/bionic"; - - auto entry = GetEntryForMountPoint(mounts, kBionic); - if (!entry) return; - - struct statfs buf; - if (::statfs(kBionic, &buf) == -1) { - PLOG(ERROR) << "statfs of " << kBionic; - return; - } - if (buf.f_flags & MS_RDONLY) { - // /bionic is on a read-only partition; can happen for - // non-system-as-root-devices. Don' try to unmount. - return; - } - fs_mgr_set_blk_ro(entry->blk_device, false); - if (::mount(entry->blk_device.c_str(), entry->mount_point.c_str(), entry->fs_type.c_str(), - MS_REMOUNT, nullptr) == -1) { - PLOG(ERROR) << "remount of " << kBionic; - } -} - void MyLogger(android::base::LogId id, android::base::LogSeverity severity, const char* tag, const char* file, unsigned int line, const char* message) { static const char log_characters[] = "VD\0WEFF"; @@ -395,7 +372,5 @@ int main(int argc, char* argv[]) { if (reboot_later) reboot(false); - try_unmount_bionic(&mounts); - return retval; }