From 91a98487752b21269dd459623a29c0d48bd7bced Mon Sep 17 00:00:00 2001 From: Nikita Ioffe Date: Wed, 10 Jun 2020 00:53:59 +0100 Subject: [PATCH] Reboot sequence: Unmount active apexes before unmounting /data Having mounted apexes with loop back devices backing files on /data partition will prevent clean unmount of it. Unmounting them and tearing down loop devices should minimize the risk of that. Note that it won't fix the issue completely, as there are a few (~2-3) processes that keep restarting even after SIGKILL is sent. Which means that they can still hold references to apexes on /data partition. But in practice probability of this is quite low. Test: adb reboot Test: put tzdata apex in /data/apex/active && adb reboot Bug: 158152940 Change-Id: I4624567b3d0f304dba4c6e37b77abd89e57411de --- init/reboot.cpp | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/init/reboot.cpp b/init/reboot.cpp index ec9236140..19e83cc7a 100644 --- a/init/reboot.cpp +++ b/init/reboot.cpp @@ -544,6 +544,18 @@ static int StopServicesAndLogViolations(const std::vector& services, return still_running; } +static Result UnmountAllApexes() { + const char* args[] = {"/system/bin/apexd", "--unmount-all"}; + int status; + if (logwrap_fork_execvp(arraysize(args), args, &status, false, LOG_KLOG, true, nullptr) != 0) { + return ErrnoError() << "Failed to call '/system/bin/apexd --unmount-all'"; + } + if (WIFEXITED(status) && WEXITSTATUS(status) == 0) { + return {}; + } + return Error() << "'/system/bin/apexd --unmount-all' failed : " << status; +} + //* Reboot / shutdown the system. // cmd ANDROID_RB_* as defined in android_reboot.h // reason Reason string like "reboot", "shutdown,userrequested" @@ -698,6 +710,11 @@ static void DoReboot(unsigned int cmd, const std::string& reason, const std::str // 5. drop caches and disable zram backing device, if exist KillZramBackingDevice(); + LOG(INFO) << "Ready to unmount apexes. So far shutdown sequence took " << t; + // 6. unmount active apexes, otherwise they might prevent clean unmount of /data. + if (auto ret = UnmountAllApexes(); !ret.ok()) { + LOG(ERROR) << ret.error(); + } UmountStat stat = TryUmountAndFsck(cmd, run_fsck, shutdown_timeout - t.duration(), &reboot_semaphore); // Follow what linux shutdown is doing: one more sync with little bit delay @@ -736,18 +753,6 @@ static void LeaveShutdown() { StartSendingMessages(); } -static Result UnmountAllApexes() { - const char* args[] = {"/system/bin/apexd", "--unmount-all"}; - int status; - if (logwrap_fork_execvp(arraysize(args), args, &status, false, LOG_KLOG, true, nullptr) != 0) { - return ErrnoError() << "Failed to call '/system/bin/apexd --unmount-all'"; - } - if (WIFEXITED(status) && WEXITSTATUS(status) == 0) { - return {}; - } - return Error() << "'/system/bin/apexd --unmount-all' failed : " << status; -} - static std::chrono::milliseconds GetMillisProperty(const std::string& name, std::chrono::milliseconds default_value) { auto value = GetUintProperty(name, static_cast(default_value.count()));