From 66d0d96c8909919d464a87944de3c089878ce744 Mon Sep 17 00:00:00 2001 From: Yo Chiang Date: Tue, 27 Oct 2020 19:07:37 +0800 Subject: [PATCH] Refactor fs_mgr_overlayfs_teardown() Right now fs_mgr_overlayfs_teardown() does slightly different things when called from the userspace or recovery. This is accomplished by many runtime checks, runtime assumptions and conditional execution of code. This makes the control flow of the function very hard to follow, and assumptions becomes more fragile as the function becomes more complex. This CL forks fs_mgr_overlayfs_teardown() and removes "recovery" bits from it. A new entry point TeardownAllOverlayForMountPoint() is added for recovery. Recovery (fastbootd) should call TeardownAllOverlayForMountPoint() to teardown overlays of all sources (cache dir, scratch device or DSU). While fs_mgr_overlayfs_teardown() should only be called from the userspace. Also apply some linter suggestions. Bug: 165925766 Test: adb-remount-test.sh Change-Id: I7ff7e3409c910782e1ec207fcd02b967a9762bc1 --- fastboot/device/flashing.cpp | 6 +- fs_mgr/fs_mgr_overlayfs.cpp | 117 ++++++++++++++++++------------ fs_mgr/include/fs_mgr_overlayfs.h | 7 ++ 3 files changed, 80 insertions(+), 50 deletions(-) diff --git a/fastboot/device/flashing.cpp b/fastboot/device/flashing.cpp index 1bf4c9c9c..333ca50fb 100644 --- a/fastboot/device/flashing.cpp +++ b/fastboot/device/flashing.cpp @@ -67,7 +67,7 @@ void WipeOverlayfsForPartition(FastbootDevice* device, const std::string& partit if ((partition + device->GetCurrentSlot()) == partition_name) { mount_metadata.emplace(); - fs_mgr_overlayfs_teardown(entry.mount_point.c_str()); + android::fs_mgr::TeardownAllOverlayForMountPoint(entry.mount_point); } } } @@ -194,7 +194,7 @@ bool UpdateSuper(FastbootDevice* device, const std::string& super_name, bool wip if (!FlashPartitionTable(super_name, *new_metadata.get())) { return device->WriteFail("Unable to flash new partition table"); } - fs_mgr_overlayfs_teardown(); + android::fs_mgr::TeardownAllOverlayForMountPoint(); sync(); return device->WriteOkay("Successfully flashed partition table"); } @@ -234,7 +234,7 @@ bool UpdateSuper(FastbootDevice* device, const std::string& super_name, bool wip if (!UpdateAllPartitionMetadata(device, super_name, *new_metadata.get())) { return device->WriteFail("Unable to write new partition table"); } - fs_mgr_overlayfs_teardown(); + android::fs_mgr::TeardownAllOverlayForMountPoint(); sync(); return device->WriteOkay("Successfully updated partition table"); } diff --git a/fs_mgr/fs_mgr_overlayfs.cpp b/fs_mgr/fs_mgr_overlayfs.cpp index 98f4a80f2..899978f2e 100644 --- a/fs_mgr/fs_mgr_overlayfs.cpp +++ b/fs_mgr/fs_mgr_overlayfs.cpp @@ -132,8 +132,11 @@ bool fs_mgr_overlayfs_is_setup() { namespace android { namespace fs_mgr { -void MapScratchPartitionIfNeeded(Fstab*, - const std::function&)>&) {} +void MapScratchPartitionIfNeeded(Fstab*, const std::function&)>&) { +} + +void TeardownAllOverlayForMountPoint(const std::string&) {} + } // namespace fs_mgr } // namespace android @@ -1376,6 +1379,11 @@ static bool EnsureScratchMapped(std::string* device, bool* mapped) { return true; } + if (!fs_mgr_in_recovery()) { + errno = EINVAL; + return false; + } + auto partition_name = android::base::Basename(kScratchMountPoint); // Check for scratch on /data first, before looking for a modified super @@ -1417,47 +1425,28 @@ static bool EnsureScratchMapped(std::string* device, bool* mapped) { return true; } -static void UnmapScratchDevice() { - // This should only be reachable in recovery, where scratch is not - // automatically mapped and therefore can be unmapped. - DestroyLogicalPartition(android::base::Basename(kScratchMountPoint)); -} - -#if !defined __ANDROID_RECOVERY__ -// Provide stubs for non-recovery variant. -static void fs_mgr_overlayfs_teardown_dsu(const char*) {} -#else -// Note: This should only be called from recovery or fastbootd. -static void fs_mgr_overlayfs_teardown_dsu(const char* mount_point) { +// This should only be reachable in recovery, where DSU scratch is not +// automatically mapped. +static bool MapDsuScratchDevice(std::string* device) { std::string dsu_slot; if (!android::gsi::IsGsiInstalled() || !android::gsi::GetActiveDsu(&dsu_slot) || dsu_slot.empty()) { // Nothing to do if no DSU installation present. - return; + return false; } auto images = IImageManager::Open("dsu/" + dsu_slot, 10s); if (!images || !images->BackingImageExists(android::gsi::kDsuScratch)) { // Nothing to do if DSU scratch device doesn't exist. - return; - } - - std::string scratch_device; - images->UnmapImageDevice(android::gsi::kDsuScratch); - if (!images->MapImageDevice(android::gsi::kDsuScratch, 10s, &scratch_device)) { - return; - } - - fs_mgr_overlayfs_umount_scratch(); - if (fs_mgr_overlayfs_mount_scratch(scratch_device, fs_mgr_overlayfs_scratch_mount_type())) { - fs_mgr_overlayfs_teardown_one(kScratchMountPoint, - mount_point ? fs_mgr_mount_point(mount_point) : "", nullptr); - fs_mgr_overlayfs_umount_scratch(); + return false; } images->UnmapImageDevice(android::gsi::kDsuScratch); + if (!images->MapImageDevice(android::gsi::kDsuScratch, 10s, device)) { + return false; + } + return true; } -#endif // Returns false if teardown not permitted, errno set to last error. // If something is altered, set *change. @@ -1468,10 +1457,9 @@ bool fs_mgr_overlayfs_teardown(const char* mount_point, bool* change) { // If scratch exists, but is not mounted, lets gain access to clean // specific override entries. auto mount_scratch = false; - bool unmap = false; if ((mount_point != nullptr) && !fs_mgr_overlayfs_already_mounted(kScratchMountPoint, false)) { - std::string scratch_device; - if (EnsureScratchMapped(&scratch_device, &unmap)) { + std::string scratch_device = GetBootScratchDevice(); + if (!scratch_device.empty()) { mount_scratch = fs_mgr_overlayfs_mount_scratch(scratch_device, fs_mgr_overlayfs_scratch_mount_type()); } @@ -1485,15 +1473,11 @@ bool fs_mgr_overlayfs_teardown(const char* mount_point, bool* change) { // Do not attempt to destroy DSU scratch if within a DSU system, // because DSU scratch partition is managed by gsid. if (should_destroy_scratch && !fs_mgr_is_dsu_running()) { - // Note: Reaching here in recovery or fastbootd means that a scratch device - // is mounted and cleaned up. Such scratch device mustn't be the DSU scratch, - // because EnsureScratchMapped() is not allowed to map the DSU scratch in - // recovery. In other words, it is safe to destroy the scratch device here. ret &= fs_mgr_overlayfs_teardown_scratch(kScratchMountPoint, change); } if (fs_mgr_overlayfs_valid() == OverlayfsValidResult::kNotSupported) { // After obligatory teardown to make sure everything is clean, but if - // we didn't want overlayfs in the the first place, we do not want to + // we didn't want overlayfs in the first place, we do not want to // waste time on a reboot (or reboot request message). if (change) *change = false; } @@ -1507,15 +1491,6 @@ bool fs_mgr_overlayfs_teardown(const char* mount_point, bool* change) { if (mount_scratch) { fs_mgr_overlayfs_umount_scratch(); } - if (unmap) { - UnmapScratchDevice(); - } - - if (fs_mgr_in_recovery()) { - // Destroy DSU overlay if present. - fs_mgr_overlayfs_teardown_dsu(mount_point); - } - return ret; } @@ -1583,6 +1558,54 @@ void CleanupOldScratchFiles() { } } +void TeardownAllOverlayForMountPoint(const std::string& mount_point) { + if (!fs_mgr_in_recovery()) { + LERROR << __FUNCTION__ << "(): must be called within recovery."; + return; + } + + // Empty string means teardown everything. + const std::string teardown_dir = mount_point.empty() ? "" : fs_mgr_mount_point(mount_point); + constexpr bool* ignore_change = nullptr; + + // Teardown legacy overlay mount points that's not backed by a scratch device. + for (const auto& overlay_mount_point : kOverlayMountPoints) { + if (overlay_mount_point == kScratchMountPoint) { + continue; + } + fs_mgr_overlayfs_teardown_one(overlay_mount_point, teardown_dir, ignore_change); + } + + // Map scratch device, mount kScratchMountPoint and teardown kScratchMountPoint. + bool mapped = false; + std::string scratch_device; + if (EnsureScratchMapped(&scratch_device, &mapped)) { + fs_mgr_overlayfs_umount_scratch(); + if (fs_mgr_overlayfs_mount_scratch(scratch_device, fs_mgr_overlayfs_scratch_mount_type())) { + bool should_destroy_scratch = false; + fs_mgr_overlayfs_teardown_one(kScratchMountPoint, teardown_dir, ignore_change, + &should_destroy_scratch); + if (should_destroy_scratch) { + fs_mgr_overlayfs_teardown_scratch(kScratchMountPoint, nullptr); + } + fs_mgr_overlayfs_umount_scratch(); + } + if (mapped) { + DestroyLogicalPartition(android::base::Basename(kScratchMountPoint)); + } + } + + // Teardown DSU overlay if present. + if (MapDsuScratchDevice(&scratch_device)) { + fs_mgr_overlayfs_umount_scratch(); + if (fs_mgr_overlayfs_mount_scratch(scratch_device, fs_mgr_overlayfs_scratch_mount_type())) { + fs_mgr_overlayfs_teardown_one(kScratchMountPoint, teardown_dir, ignore_change); + fs_mgr_overlayfs_umount_scratch(); + } + DestroyLogicalPartition(android::gsi::kDsuScratch); + } +} + } // namespace fs_mgr } // namespace android diff --git a/fs_mgr/include/fs_mgr_overlayfs.h b/fs_mgr/include/fs_mgr_overlayfs.h index 34aded99e..d45e2de72 100644 --- a/fs_mgr/include/fs_mgr_overlayfs.h +++ b/fs_mgr/include/fs_mgr_overlayfs.h @@ -49,5 +49,12 @@ void MapScratchPartitionIfNeeded(Fstab* fstab, const std::function&)>& init); void CleanupOldScratchFiles(); +// Teardown overlays of all sources (cache dir, scratch device, DSU) for |mount_point|. +// Teardown all overlays if |mount_point| is empty. +// +// Note: This should be called if and only if in recovery or fastbootd to teardown +// overlays if any partition is flashed or updated. +void TeardownAllOverlayForMountPoint(const std::string& mount_point = {}); + } // namespace fs_mgr } // namespace android