From 81f13855895c9c0a890520f2da9985cb0aa75e09 Mon Sep 17 00:00:00 2001 From: Yifan Hong Date: Tue, 9 Apr 2019 10:11:34 -0700 Subject: [PATCH] init: add umount_all builtin. umount_all is the cleanup step for mount_all. In particular, the mount_all builtin creates a verity device, 'postinstall-verity', for the following line: system /postinstall ... ... slotselect_other,logical,avb_keys=... cppreopt umounts /postinstall but doesn't destroy the postinstall-verity device, causing OTA to fail (because it cannot destroy the system_[other] device). umount_all also destroy the verity device. Note that mount_all does not map system_[other]; it is mapped by first stage init. Hence, umount_all doesn't destroy it either. The OTA client is reponsible for unmapping the device itself. Bug: 129988285 Test: flash, boot, then check `dmctl list devices`, then OTA Change-Id: Id3ab65b3860b6ea6cfec310ab13652009c81f415 --- fs_mgr/fs_mgr.cpp | 40 ++++++++++++++++ fs_mgr/fs_mgr_dm_linear.cpp | 9 +++- fs_mgr/fs_mgr_priv.h | 8 ++++ fs_mgr/fs_mgr_verity.cpp | 10 ++++ fs_mgr/include/fs_mgr.h | 11 +++++ fs_mgr/libfs_avb/fs_avb.cpp | 23 ++++++++++ fs_mgr/libfs_avb/include/fs_avb/fs_avb.h | 5 ++ fs_mgr/libfs_avb/tests/util_test.cpp | 5 +- fs_mgr/libfs_avb/util.cpp | 11 +++-- fs_mgr/libfs_avb/util.h | 4 +- init/builtins.cpp | 58 +++++++++++++++++++----- 11 files changed, 166 insertions(+), 18 deletions(-) diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp index 045bb48ec..5114f5517 100644 --- a/fs_mgr/fs_mgr.cpp +++ b/fs_mgr/fs_mgr.cpp @@ -1268,6 +1268,46 @@ int fs_mgr_mount_all(Fstab* fstab, int mount_mode) { } } +int fs_mgr_umount_all(android::fs_mgr::Fstab* fstab) { + AvbUniquePtr avb_handle(nullptr); + int ret = FsMgrUmountStatus::SUCCESS; + for (auto& current_entry : *fstab) { + if (!IsMountPointMounted(current_entry.mount_point)) { + continue; + } + + if (umount(current_entry.mount_point.c_str()) == -1) { + PERROR << "Failed to umount " << current_entry.mount_point; + ret |= FsMgrUmountStatus::ERROR_UMOUNT; + continue; + } + + if (current_entry.fs_mgr_flags.logical) { + if (!fs_mgr_update_logical_partition(¤t_entry)) { + LERROR << "Could not get logical partition blk_device, skipping!"; + ret |= FsMgrUmountStatus::ERROR_DEVICE_MAPPER; + continue; + } + } + + if (current_entry.fs_mgr_flags.avb || !current_entry.avb_keys.empty()) { + if (!AvbHandle::TearDownAvbHashtree(¤t_entry, true /* wait */)) { + LERROR << "Failed to tear down AVB on mount point: " << current_entry.mount_point; + ret |= FsMgrUmountStatus::ERROR_VERITY; + continue; + } + } else if ((current_entry.fs_mgr_flags.verify)) { + if (!fs_mgr_teardown_verity(¤t_entry, true /* wait */)) { + LERROR << "Failed to tear down verified partition on mount point: " + << current_entry.mount_point; + ret |= FsMgrUmountStatus::ERROR_VERITY; + continue; + } + } + } + return ret; +} + // wrapper to __mount() and expects a fully prepared fstab_rec, // unlike fs_mgr_do_mount which does more things with avb / verity etc. int fs_mgr_do_mount_one(const FstabEntry& entry, const std::string& mount_point) { diff --git a/fs_mgr/fs_mgr_dm_linear.cpp b/fs_mgr/fs_mgr_dm_linear.cpp index 45cbff3a9..ee6ffdb18 100644 --- a/fs_mgr/fs_mgr_dm_linear.cpp +++ b/fs_mgr/fs_mgr_dm_linear.cpp @@ -193,7 +193,7 @@ bool CreateLogicalPartition(const std::string& block_device, uint32_t metadata_s timeout_ms, path); } -bool DestroyLogicalPartition(const std::string& name, const std::chrono::milliseconds& timeout_ms) { +bool UnmapDevice(const std::string& name, const std::chrono::milliseconds& timeout_ms) { DeviceMapper& dm = DeviceMapper::Instance(); std::string path; if (timeout_ms > std::chrono::milliseconds::zero()) { @@ -206,6 +206,13 @@ bool DestroyLogicalPartition(const std::string& name, const std::chrono::millise LERROR << "Timed out waiting for device path to unlink: " << path; return false; } + return true; +} + +bool DestroyLogicalPartition(const std::string& name, const std::chrono::milliseconds& timeout_ms) { + if (!UnmapDevice(name, timeout_ms)) { + return false; + } LINFO << "Unmapped logical partition " << name; return true; } diff --git a/fs_mgr/fs_mgr_priv.h b/fs_mgr/fs_mgr_priv.h index 11602ea61..70abf5b78 100644 --- a/fs_mgr/fs_mgr_priv.h +++ b/fs_mgr/fs_mgr_priv.h @@ -103,3 +103,11 @@ int load_verity_state(const android::fs_mgr::FstabEntry& entry, int* mode); bool fs_mgr_is_ext4(const std::string& blk_device); bool fs_mgr_is_f2fs(const std::string& blk_device); + +bool fs_mgr_teardown_verity(android::fs_mgr::FstabEntry* fstab, bool wait); + +namespace android { +namespace fs_mgr { +bool UnmapDevice(const std::string& name, const std::chrono::milliseconds& timeout_ms); +} // namespace fs_mgr +} // namespace android diff --git a/fs_mgr/fs_mgr_verity.cpp b/fs_mgr/fs_mgr_verity.cpp index c53e8664f..3f0915780 100644 --- a/fs_mgr/fs_mgr_verity.cpp +++ b/fs_mgr/fs_mgr_verity.cpp @@ -44,6 +44,7 @@ #include "fec/io.h" #include "fs_mgr.h" +#include "fs_mgr_dm_linear.h" #include "fs_mgr_priv.h" // Realistically, this file should be part of the android::fs_mgr namespace; @@ -882,3 +883,12 @@ out: return retval; } + +bool fs_mgr_teardown_verity(FstabEntry* entry, bool wait) { + const std::string mount_point(basename(entry->mount_point.c_str())); + if (!android::fs_mgr::UnmapDevice(mount_point, wait ? 1000ms : 0ms)) { + return false; + } + LINFO << "Unmapped verity device " << mount_point; + return true; +} diff --git a/fs_mgr/include/fs_mgr.h b/fs_mgr/include/fs_mgr.h index 8abe609a5..88b2f8f4f 100644 --- a/fs_mgr/include/fs_mgr.h +++ b/fs_mgr/include/fs_mgr.h @@ -93,3 +93,14 @@ int fs_mgr_setup_verity(android::fs_mgr::FstabEntry* fstab, bool wait_for_verity // specified, the super partition for the corresponding metadata slot will be // returned. Otherwise, it will use the current slot. std::string fs_mgr_get_super_partition_name(int slot = -1); + +enum FsMgrUmountStatus : int { + SUCCESS = 0, + ERROR_UNKNOWN = 1 << 0, + ERROR_UMOUNT = 1 << 1, + ERROR_VERITY = 1 << 2, + ERROR_DEVICE_MAPPER = 1 << 3, +}; +// fs_mgr_umount_all() is the reverse of fs_mgr_mount_all. In particular, +// it destroys verity devices from device mapper after the device is unmounted. +int fs_mgr_umount_all(android::fs_mgr::Fstab* fstab); diff --git a/fs_mgr/libfs_avb/fs_avb.cpp b/fs_mgr/libfs_avb/fs_avb.cpp index f0767dc47..04776edc5 100644 --- a/fs_mgr/libfs_avb/fs_avb.cpp +++ b/fs_mgr/libfs_avb/fs_avb.cpp @@ -449,6 +449,29 @@ AvbHashtreeResult AvbHandle::SetUpAvbHashtree(FstabEntry* fstab_entry, bool wait return AvbHashtreeResult::kSuccess; } +bool AvbHandle::TearDownAvbHashtree(FstabEntry* fstab_entry, bool wait) { + if (!fstab_entry) { + return false; + } + + const std::string device_name(GetVerityDeviceName(*fstab_entry)); + + // TODO: remove duplicated code with UnmapDevice() + android::dm::DeviceMapper& dm = android::dm::DeviceMapper::Instance(); + std::string path; + if (wait) { + dm.GetDmDevicePathByName(device_name, &path); + } + if (!dm.DeleteDevice(device_name)) { + return false; + } + if (!path.empty() && !WaitForFile(path, 1000ms, FileWaitMode::DoesNotExist)) { + return false; + } + + return true; +} + std::string AvbHandle::GetSecurityPatchLevel(const FstabEntry& fstab_entry) const { if (vbmeta_images_.size() < 1) { return ""; diff --git a/fs_mgr/libfs_avb/include/fs_avb/fs_avb.h b/fs_mgr/libfs_avb/include/fs_avb/fs_avb.h index 7127fa6ea..521f2d582 100644 --- a/fs_mgr/libfs_avb/include/fs_avb/fs_avb.h +++ b/fs_mgr/libfs_avb/include/fs_avb/fs_avb.h @@ -110,6 +110,11 @@ class AvbHandle { static AvbHashtreeResult SetUpStandaloneAvbHashtree(FstabEntry* fstab_entry, bool wait_for_verity_dev = true); + // Tear down dm devices created by SetUp[Standalone]AvbHashtree + // The 'wait' parameter makes this function wait for the verity device to get destroyed + // before return. + static bool TearDownAvbHashtree(FstabEntry* fstab_entry, bool wait); + static bool IsDeviceUnlocked(); std::string GetSecurityPatchLevel(const FstabEntry& fstab_entry) const; diff --git a/fs_mgr/libfs_avb/tests/util_test.cpp b/fs_mgr/libfs_avb/tests/util_test.cpp index 9e37d2261..12b5acb8b 100644 --- a/fs_mgr/libfs_avb/tests/util_test.cpp +++ b/fs_mgr/libfs_avb/tests/util_test.cpp @@ -27,6 +27,7 @@ // Target functions to test: using android::fs_mgr::BytesToHex; +using android::fs_mgr::FileWaitMode; using android::fs_mgr::HexToBytes; using android::fs_mgr::NibbleValue; using android::fs_mgr::WaitForFile; @@ -175,7 +176,7 @@ TEST(BasicUtilTest, WaitForFileDeferCreation) { // Waits this path. base::FilePath wait_path = tmp_dir.Append("libfs_avb-test-exist-dir"); ASSERT_TRUE(base::DeleteFile(wait_path, false /* resursive */)); - auto wait_file = std::async(WaitForFile, wait_path.value(), 500ms); + auto wait_file = std::async(WaitForFile, wait_path.value(), 500ms, FileWaitMode::Exists); // Sleeps 100ms before creating the wait_path. std::this_thread::sleep_for(100ms); @@ -196,7 +197,7 @@ TEST(BasicUtilTest, WaitForFileDeferCreationFailure) { // Waits this path. base::FilePath wait_path = tmp_dir.Append("libfs_avb-test-exist-dir"); ASSERT_TRUE(base::DeleteFile(wait_path, false /* resursive */)); - auto wait_file = std::async(WaitForFile, wait_path.value(), 50ms); + auto wait_file = std::async(WaitForFile, wait_path.value(), 50ms, FileWaitMode::Exists); // Sleeps 100ms before creating the wait_path. std::this_thread::sleep_for(100ms); diff --git a/fs_mgr/libfs_avb/util.cpp b/fs_mgr/libfs_avb/util.cpp index 9d4f05f57..d214b5bdb 100644 --- a/fs_mgr/libfs_avb/util.cpp +++ b/fs_mgr/libfs_avb/util.cpp @@ -82,12 +82,17 @@ std::string BytesToHex(const uint8_t* bytes, size_t bytes_len) { return hex; } -bool WaitForFile(const std::string& filename, const std::chrono::milliseconds relative_timeout) { +// TODO: remove duplicate code with fs_mgr_wait_for_file +bool WaitForFile(const std::string& filename, const std::chrono::milliseconds relative_timeout, + FileWaitMode file_wait_mode) { auto start_time = std::chrono::steady_clock::now(); while (true) { - if (0 == access(filename.c_str(), F_OK) || errno != ENOENT) { - return true; + int rv = access(filename.c_str(), F_OK); + if (file_wait_mode == FileWaitMode::Exists) { + if (!rv || errno != ENOENT) return true; + } else if (file_wait_mode == FileWaitMode::DoesNotExist) { + if (rv && errno == ENOENT) return true; } std::this_thread::sleep_for(50ms); diff --git a/fs_mgr/libfs_avb/util.h b/fs_mgr/libfs_avb/util.h index cb861f466..7763da502 100644 --- a/fs_mgr/libfs_avb/util.h +++ b/fs_mgr/libfs_avb/util.h @@ -52,7 +52,9 @@ bool HexToBytes(uint8_t* bytes, size_t bytes_len, const std::string& hex); std::string BytesToHex(const uint8_t* bytes, size_t bytes_len); -bool WaitForFile(const std::string& filename, const std::chrono::milliseconds relative_timeout); +enum class FileWaitMode { Exists, DoesNotExist }; +bool WaitForFile(const std::string& filename, const std::chrono::milliseconds relative_timeout, + FileWaitMode wait_mode = FileWaitMode::Exists); bool IsDeviceUnlocked(); diff --git a/init/builtins.cpp b/init/builtins.cpp index 8437e3790..fc75072b6 100644 --- a/init/builtins.cpp +++ b/init/builtins.cpp @@ -451,13 +451,13 @@ static void import_late(const std::vector& args, size_t start_index if (false) DumpState(); } -/* mount_fstab +/* handle_fstab * - * Call fs_mgr_mount_all() to mount the given fstab + * Read the given fstab file and execute func on it. */ -static Result mount_fstab(const char* fstabfile, int mount_mode) { +static Result handle_fstab(const std::string& fstabfile, std::function func) { /* - * Call fs_mgr_mount_all() to mount all filesystems. We fork(2) and + * Call fs_mgr_[u]mount_all() to [u]mount all filesystems. We fork(2) and * do the call in the child to provide protection to the main init * process if anything goes wrong (crash or memory leak), and wait for * the child to finish in the parent. @@ -478,25 +478,51 @@ static Result mount_fstab(const char* fstabfile, int mount_mode) { return Error() << "child aborted"; } } else if (pid == 0) { - /* child, call fs_mgr_mount_all() */ + /* child, call fs_mgr_[u]mount_all() */ - // So we can always see what fs_mgr_mount_all() does. + // So we can always see what fs_mgr_[u]mount_all() does. // Only needed if someone explicitly changes the default log level in their init.rc. android::base::ScopedLogSeverity info(android::base::INFO); Fstab fstab; ReadFstabFromFile(fstabfile, &fstab); - int child_ret = fs_mgr_mount_all(&fstab, mount_mode); - if (child_ret == -1) { - PLOG(ERROR) << "fs_mgr_mount_all returned an error"; - } + int child_ret = func(&fstab); + _exit(child_ret); } else { return Error() << "fork() failed"; } } +/* mount_fstab + * + * Call fs_mgr_mount_all() to mount the given fstab + */ +static Result mount_fstab(const std::string& fstabfile, int mount_mode) { + return handle_fstab(fstabfile, [mount_mode](Fstab* fstab) { + int ret = fs_mgr_mount_all(fstab, mount_mode); + if (ret == -1) { + PLOG(ERROR) << "fs_mgr_mount_all returned an error"; + } + return ret; + }); +} + +/* umount_fstab + * + * Call fs_mgr_umount_all() to umount the given fstab + */ +static Result umount_fstab(const std::string& fstabfile) { + return handle_fstab(fstabfile, [](Fstab* fstab) { + int ret = fs_mgr_umount_all(fstab); + if (ret != 0) { + PLOG(ERROR) << "fs_mgr_umount_all returned " << ret; + } + return ret; + }); +} + /* Queue event based on fs_mgr return code. * * code: return code of fs_mgr_mount_all @@ -583,7 +609,7 @@ static Result do_mount_all(const BuiltinArguments& args) { bool import_rc = true; bool queue_event = true; int mount_mode = MOUNT_MODE_DEFAULT; - const char* fstabfile = args[1].c_str(); + const auto& fstabfile = args[1]; std::size_t path_arg_end = args.size(); const char* prop_post_fix = "default"; @@ -626,6 +652,15 @@ static Result do_mount_all(const BuiltinArguments& args) { return Success(); } +/* umount_all */ +static Result do_umount_all(const BuiltinArguments& args) { + auto umount_fstab_return_code = umount_fstab(args[1]); + if (!umount_fstab_return_code) { + return Error() << "umount_fstab() failed " << umount_fstab_return_code.error(); + } + return Success(); +} + static Result do_swapon_all(const BuiltinArguments& args) { Fstab fstab; if (!ReadFstabFromFile(args[1], &fstab)) { @@ -1165,6 +1200,7 @@ const BuiltinFunctionMap::Map& BuiltinFunctionMap::map() const { {"mount", {3, kMax, {false, do_mount}}}, {"parse_apex_configs", {0, 0, {false, do_parse_apex_configs}}}, {"umount", {1, 1, {false, do_umount}}}, + {"umount_all", {1, 1, {false, do_umount_all}}}, {"readahead", {1, 2, {true, do_readahead}}}, {"restart", {1, 1, {false, do_restart}}}, {"restorecon", {1, kMax, {true, do_restorecon}}},