From 8b73dfc0a44e069e57592a50800b7ea10270ce83 Mon Sep 17 00:00:00 2001 From: Jiyong Park Date: Fri, 18 Jan 2019 12:29:12 +0900 Subject: [PATCH] Revert "Bionic libs and the dynamic linker are bind mounted" This reverts commit 2599088ff67c10c66a70d3741c41529d3e11c7f5. Reason: Breaks some 3p apps. Bug: 122920047 Test: run the app, login. Change-Id: Idea332b1f91e9d2ac6ebd3879da7820c8ba2284f --- init/builtins.cpp | 82 ----------------------------------------------- init/service.cpp | 76 ------------------------------------------- init/service.h | 25 --------------- rootdir/init.rc | 21 +++--------- 4 files changed, 5 insertions(+), 199 deletions(-) diff --git a/init/builtins.cpp b/init/builtins.cpp index e1a86e4f9..169edbe09 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -1098,86 +1098,6 @@ static Result do_parse_apex_configs(const BuiltinArguments& args) { } } -static Result bind_mount_file(const char* source, const char* mount_point, - bool remount_private) { - if (remount_private && mount(nullptr, mount_point, nullptr, MS_PRIVATE, nullptr) == -1) { - return ErrnoError() << "Could not change " << mount_point << " to a private mount point"; - } - if (mount(source, mount_point, nullptr, MS_BIND, nullptr) == -1) { - return ErrnoError() << "Could not bind-mount " << source << " to " << mount_point; - } - return Success(); -} - -static Result bind_mount_bionic(const char* linker_source, const char* lib_dir_source, - const char* linker_mount_point, const char* lib_mount_dir, - bool remount_private) { - if (access(linker_source, F_OK) != 0) { - return Success(); - } - if (auto result = bind_mount_file(linker_source, linker_mount_point, remount_private); - !result) { - return result; - } - for (auto libname : kBionicLibFileNames) { - std::string mount_point = lib_mount_dir + libname; - std::string source = lib_dir_source + libname; - if (auto result = bind_mount_file(source.c_str(), mount_point.c_str(), remount_private); - !result) { - return result; - } - } - return Success(); -} - -// The bootstrap bionic libs and the bootstrap linker are bind-mounted to -// the mount points for pre-apexd processes. -static Result do_prepare_bootstrap_bionic(const BuiltinArguments& args) { - static bool prepare_bootstrap_bionic_done = false; - if (prepare_bootstrap_bionic_done) { - return Error() << "prepare_bootstrap_bionic was already executed. Cannot be executed again"; - } - if (auto result = bind_mount_bionic(kBootstrapLinkerPath, kBootstrapBionicLibsDir, - kLinkerMountPoint, kBionicLibsMountPointDir, false); - !result) { - return result; - } - if (auto result = bind_mount_bionic(kBootstrapLinkerPath64, kBootstrapBionicLibsDir64, - kLinkerMountPoint64, kBionicLibsMountPointDir64, false); - !result) { - return result; - } - - LOG(INFO) << "prepare_bootstrap_bionic done"; - prepare_bootstrap_bionic_done = true; - return Success(); -} - -// The bionic libs and the dynamic linker from the runtime APEX are bind-mounted -// to the mount points. As a result, the previous mounts done by -// prepare_bootstrap_bionic become hidden. -static Result do_setup_runtime_bionic(const BuiltinArguments& args) { - static bool setup_runtime_bionic_done = false; - if (setup_runtime_bionic_done) { - return Error() << "setup_runtime_bionic was already executed. Cannot be executed again"; - } - if (auto result = bind_mount_bionic(kRuntimeLinkerPath, kRuntimeBionicLibsDir, - kLinkerMountPoint, kBionicLibsMountPointDir, true); - !result) { - return result; - } - if (auto result = bind_mount_bionic(kRuntimeLinkerPath64, kRuntimeBionicLibsDir64, - kLinkerMountPoint64, kBionicLibsMountPointDir64, true); - !result) { - return result; - } - - ServiceList::GetInstance().MarkRuntimeAvailable(); - LOG(INFO) << "setup_runtime_bionic done"; - setup_runtime_bionic_done = true; - return Success(); -} - // Builtin-function-map start const BuiltinFunctionMap::Map& BuiltinFunctionMap::map() const { constexpr std::size_t kMax = std::numeric_limits::max(); @@ -1216,7 +1136,6 @@ const BuiltinFunctionMap::Map& BuiltinFunctionMap::map() const { {"mount_all", {1, kMax, {false, do_mount_all}}}, {"mount", {3, kMax, {false, do_mount}}}, {"parse_apex_configs", {0, 0, {false, do_parse_apex_configs}}}, - {"prepare_bootstrap_bionic",{0, 0, {false, do_prepare_bootstrap_bionic}}}, {"umount", {1, 1, {false, do_umount}}}, {"readahead", {1, 2, {true, do_readahead}}}, {"restart", {1, 1, {false, do_restart}}}, @@ -1225,7 +1144,6 @@ const BuiltinFunctionMap::Map& BuiltinFunctionMap::map() const { {"rm", {1, 1, {true, do_rm}}}, {"rmdir", {1, 1, {true, do_rmdir}}}, {"setprop", {2, 2, {true, do_setprop}}}, - {"setup_runtime_bionic", {0, 0, {false, do_setup_runtime_bionic}}}, {"setrlimit", {3, 3, {false, do_setrlimit}}}, {"start", {1, 1, {false, do_start}}}, {"stop", {1, 1, {false, do_stop}}}, diff --git a/init/service.cpp b/init/service.cpp index c25628e15..638dc5a07 100644 --- a/init/service.cpp +++ b/init/service.cpp @@ -140,43 +140,6 @@ Result Service::SetUpMountNamespace() const { return Success(); } -Result Service::SetUpPreApexdMounts() const { - // If a pre-apexd service is 're' launched after the runtime APEX is - // available, unmount the linker and bionic libs which are currently - // bind mounted to the files in the runtime APEX. This will reveal - // the hidden mount points (targetting the bootstrap ones in the - // system partition) which were setup before the runtime APEX was - // started. Note that these unmounts are done in a separate mount namespace - // for the process. It does not affect other processes including the init. - if (pre_apexd_ && ServiceList::GetInstance().IsRuntimeAvailable()) { - if (access(kLinkerMountPoint, F_OK) == 0) { - if (umount(kLinkerMountPoint) == -1) { - return ErrnoError() << "Could not umount " << kLinkerMountPoint; - } - for (const auto& libname : kBionicLibFileNames) { - std::string mount_point = kBionicLibsMountPointDir + libname; - if (umount(mount_point.c_str()) == -1) { - return ErrnoError() << "Could not umount " << mount_point; - } - } - } - - if (access(kLinkerMountPoint64, F_OK) == 0) { - if (umount(kLinkerMountPoint64) == -1) { - return ErrnoError() << "Could not umount " << kLinkerMountPoint64; - } - for (const auto& libname : kBionicLibFileNames) { - std::string mount_point = kBionicLibsMountPointDir64 + libname; - std::string source = kBootstrapBionicLibsDir64 + libname; - if (umount(mount_point.c_str()) == -1) { - return ErrnoError() << "Could not umount " << mount_point; - } - } - } - } - return Success(); -} - Result Service::SetUpPidNamespace() const { if (prctl(PR_SET_NAME, name_.c_str()) == -1) { return ErrnoError() << "Could not set name"; @@ -966,14 +929,6 @@ Result Service::Start() { scon = *result; } - if (!ServiceList::GetInstance().IsRuntimeAvailable() && !pre_apexd_) { - // If this service is started before the runtime APEX gets available, - // mark it as pre-apexd one. Note that this marking is permanent. So - // for example, if the service is re-launched (e.g., due to crash), - // it is still recognized as pre-apexd... for consistency. - pre_apexd_ = true; - } - LOG(INFO) << "starting service '" << name_ << "'..."; pid_t pid = -1; @@ -990,26 +945,6 @@ Result Service::Start() { LOG(FATAL) << "Service '" << name_ << "' could not enter namespaces: " << result.error(); } - if (pre_apexd_) { - // pre-apexd process gets a private copy of the mount namespace. - // However, this does not mean that mount/unmount events are not - // shared across pre-apexd processes and post-apexd processes. - // *Most* of the events are still shared because the propagation - // type of / is set to 'shared'. (see `mount rootfs rootfs /shared - // rec` in init.rc) - // - // This unsharing is required to not propagate the mount events - // under /system/lib/{libc|libdl|libm}.so and /system/bin/linker(64) - // whose propagation type is set to private. With this, - // bind-mounting the bionic libs and the dynamic linker from the - // runtime APEX to the mount points does not affect pre-apexd - // processes which should use the bootstrap ones. - if (unshare(CLONE_NEWNS) != 0) { - LOG(FATAL) << "Creating a new mount namespace for service" - << " '" << name_ << "' failed: " << strerror(errno); - } - } - if (namespace_flags_ & CLONE_NEWNS) { if (auto result = SetUpMountNamespace(); !result) { LOG(FATAL) << "Service '" << name_ @@ -1017,13 +952,6 @@ Result Service::Start() { } } - if (pre_apexd_ && ServiceList::GetInstance().IsRuntimeAvailable()) { - if (auto result = SetUpPreApexdMounts(); !result) { - LOG(FATAL) << "Pre-apexd service '" << name_ - << "' could not setup the mount points: " << result.error(); - } - } - if (namespace_flags_ & CLONE_NEWPID) { // This will fork again to run an init process inside the PID // namespace. @@ -1396,10 +1324,6 @@ void ServiceList::MarkServicesUpdate() { delayed_service_names_.clear(); } -void ServiceList::MarkRuntimeAvailable() { - runtime_available_ = true; -} - void ServiceList::DelayService(const Service& service) { if (services_update_finished_) { LOG(ERROR) << "Cannot delay the start of service '" << service.name() diff --git a/init/service.h b/init/service.h index 676111fa1..56e75b0bf 100644 --- a/init/service.h +++ b/init/service.h @@ -62,24 +62,6 @@ namespace android { namespace init { -static constexpr const char* kLinkerMountPoint = "/system/bin/linker"; -static constexpr const char* kBootstrapLinkerPath = "/system/bin/linker"; -static constexpr const char* kRuntimeLinkerPath = "/apex/com.android.runtime/bin/linker"; - -static constexpr const char* kBionicLibsMountPointDir = "/system/lib/"; -static constexpr const char* kBootstrapBionicLibsDir = "/system/lib/"; -static constexpr const char* kRuntimeBionicLibsDir = "/apex/com.android.runtime/lib/bionic/"; - -static constexpr const char* kLinkerMountPoint64 = "/system/bin/linker64"; -static constexpr const char* kBootstrapLinkerPath64 = "/system/bin/linker64"; -static constexpr const char* kRuntimeLinkerPath64 = "/apex/com.android.runtime/bin/linker64"; - -static constexpr const char* kBionicLibsMountPointDir64 = "/system/lib64/"; -static constexpr const char* kBootstrapBionicLibsDir64 = "/system/lib64/"; -static constexpr const char* kRuntimeBionicLibsDir64 = "/apex/com.android.runtime/lib64/bionic/"; - -static const std::vector kBionicLibFileNames = {"libc.so", "libm.so", "libdl.so"}; - class Service { public: Service(const std::string& name, Subcontext* subcontext_for_restart_commands, @@ -142,7 +124,6 @@ class Service { std::optional timeout_period() const { return timeout_period_; } const std::vector& args() const { return args_; } bool is_updatable() const { return updatable_; } - bool is_pre_apexd() const { return pre_apexd_; } private: using OptionParser = Result (Service::*)(std::vector&& args); @@ -151,7 +132,6 @@ class Service { Result SetUpMountNamespace() const; Result SetUpPidNamespace() const; Result EnterNamespaces() const; - Result SetUpPreApexdMounts() const; void NotifyStateChange(const std::string& new_state) const; void StopOrReset(int how); void ZapStdio() const; @@ -262,8 +242,6 @@ class Service { std::vector args_; std::vector> reap_callbacks_; - - bool pre_apexd_ = false; }; class ServiceList { @@ -306,16 +284,13 @@ class ServiceList { const std::vector services_in_shutdown_order() const; void MarkServicesUpdate(); - void MarkRuntimeAvailable(); bool IsServicesUpdated() const { return services_update_finished_; } - bool IsRuntimeAvailable() const { return runtime_available_; } void DelayService(const Service& service); private: std::vector> services_; bool services_update_finished_ = false; - bool runtime_available_ = false; std::vector delayed_service_names_; }; diff --git a/rootdir/init.rc b/rootdir/init.rc index 9b2096412..d1a0ffd37 100644 --- a/rootdir/init.rc +++ b/rootdir/init.rc @@ -12,12 +12,6 @@ import /init.usb.configfs.rc import /init.${ro.zygote}.rc on early-init - # Mount shared so changes propagate into child namespaces - # Do this before other processes are started from init. Otherwise, - # processes launched while the propagation type of / is 'private' - # won't get mount events from others. - mount rootfs rootfs / shared rec - # Set init and its forked children's oom_adj. write /proc/1/oom_score_adj -1000 @@ -46,8 +40,6 @@ on early-init # cgroup for system_server and surfaceflinger mkdir /dev/memcg/system 0550 system system - prepare_bootstrap_bionic - start ueventd on init @@ -352,6 +344,8 @@ on post-fs # Once everything is setup, no need to modify /. # The bind+remount combination allows this to work in containers. mount rootfs rootfs / remount bind ro nodev + # Mount shared so changes propagate into child namespaces + mount rootfs rootfs / shared rec # Mount default storage into root namespace mount none /mnt/runtime/default /storage bind rec mount none none /storage slave rec @@ -587,14 +581,6 @@ on post-fs-data # Check any timezone data in /data is newer than the copy in the runtime module, delete if not. exec - system system -- /system/bin/tzdatacheck /apex/com.android.runtime/etc/tz /data/misc/zoneinfo - # Wait for apexd to finish activating APEXes before starting more processes. - # This certainly reduces the parallelism but is required to make as many processes - # as possible to use the bionic libs from the runtime APEX. This takes less than 50ms - # so the impact on the booting time is not significant. - wait_for_prop apexd.status ready - setup_runtime_bionic - parse_apex_configs - # If there is no post-fs-data action in the init..rc file, you # must uncomment this line, otherwise encrypted filesystems # won't work. @@ -816,3 +802,6 @@ on property:ro.debuggable=1 service flash_recovery /system/bin/install-recovery.sh class main oneshot + +on property:apexd.status=ready + parse_apex_configs