From c8ac4e7644810275a7ceed2497577199ec7a9426 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Thu, 6 Sep 2018 17:25:03 -0700 Subject: [PATCH] fastbootd: Wait for /dev/block paths when opening logical partitions. Note that in addition to waiting for the path to appear, we must also wait for it to be unlinked. Otherwise, we could accidentally access an older device when opening and closing the same partition twice in a row. Bug: 114198005 Test: fastboot flashall works Change-Id: Iddffc34e1ac8aa066c28e7b1a92b09b6dfd7945c --- fastboot/device/utility.cpp | 5 +++-- fs_mgr/fs_mgr.cpp | 10 +++++++--- fs_mgr/fs_mgr_dm_linear.cpp | 25 ++++++++++++++++++++----- fs_mgr/fs_mgr_priv.h | 8 ++++++-- fs_mgr/include/fs_mgr_dm_linear.h | 11 ++++++++--- 5 files changed, 44 insertions(+), 15 deletions(-) diff --git a/fastboot/device/utility.cpp b/fastboot/device/utility.cpp index d78c80922..261a2026c 100644 --- a/fastboot/device/utility.cpp +++ b/fastboot/device/utility.cpp @@ -28,6 +28,7 @@ #include "fastboot_device.h" using namespace android::fs_mgr; +using namespace std::chrono_literals; using android::base::unique_fd; using android::hardware::boot::V1_0::Slot; @@ -48,11 +49,11 @@ static bool OpenLogicalPartition(const std::string& name, const std::string& slo } uint32_t slot_number = SlotNumberForSlotSuffix(slot); std::string dm_path; - if (!CreateLogicalPartition(path->c_str(), slot_number, name, true, &dm_path)) { + if (!CreateLogicalPartition(path->c_str(), slot_number, name, true, 5s, &dm_path)) { LOG(ERROR) << "Could not map partition: " << name; return false; } - auto closer = [name]() -> void { DestroyLogicalPartition(name); }; + auto closer = [name]() -> void { DestroyLogicalPartition(name, 5s); }; *handle = PartitionHandle(dm_path, std::move(closer)); return true; } diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp index e3c2c2b83..f56043cd5 100644 --- a/fs_mgr/fs_mgr.cpp +++ b/fs_mgr/fs_mgr.cpp @@ -102,12 +102,16 @@ enum FsStatFlags { // TODO: switch to inotify() bool fs_mgr_wait_for_file(const std::string& filename, - const std::chrono::milliseconds relative_timeout) { + const std::chrono::milliseconds relative_timeout, + FileWaitMode file_wait_mode) { auto start_time = std::chrono::steady_clock::now(); while (true) { - if (!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/fs_mgr_dm_linear.cpp b/fs_mgr/fs_mgr_dm_linear.cpp index a91e92eb8..804069a3c 100644 --- a/fs_mgr/fs_mgr_dm_linear.cpp +++ b/fs_mgr/fs_mgr_dm_linear.cpp @@ -81,7 +81,7 @@ static bool CreateDmTable(const std::string& block_device, const LpMetadata& met static bool CreateLogicalPartition(const std::string& block_device, const LpMetadata& metadata, const LpMetadataPartition& partition, bool force_writable, - std::string* path) { + const std::chrono::milliseconds& timeout_ms, std::string* path) { DeviceMapper& dm = DeviceMapper::Instance(); DmTable table; @@ -98,6 +98,13 @@ static bool CreateLogicalPartition(const std::string& block_device, const LpMeta if (!dm.GetDmDevicePathByName(name, path)) { return false; } + if (timeout_ms > std::chrono::milliseconds::zero()) { + if (!fs_mgr_wait_for_file(*path, timeout_ms, FileWaitMode::Exists)) { + DestroyLogicalPartition(name, {}); + LERROR << "Timed out waiting for device path: " << *path; + return false; + } + } LINFO << "Created logical partition " << name << " on device " << *path; return true; } @@ -115,7 +122,7 @@ bool CreateLogicalPartitions(const std::string& block_device) { continue; } std::string path; - if (!CreateLogicalPartition(block_device, *metadata.get(), partition, false, &path)) { + if (!CreateLogicalPartition(block_device, *metadata.get(), partition, false, {}, &path)) { LERROR << "Could not create logical partition: " << GetPartitionName(partition); return false; } @@ -125,7 +132,7 @@ bool CreateLogicalPartitions(const std::string& block_device) { bool CreateLogicalPartition(const std::string& block_device, uint32_t metadata_slot, const std::string& partition_name, bool force_writable, - std::string* path) { + const std::chrono::milliseconds& timeout_ms, std::string* path) { auto metadata = ReadMetadata(block_device.c_str(), metadata_slot); if (!metadata) { LOG(ERROR) << "Could not read partition table."; @@ -134,18 +141,26 @@ bool CreateLogicalPartition(const std::string& block_device, uint32_t metadata_s for (const auto& partition : metadata->partitions) { if (GetPartitionName(partition) == partition_name) { return CreateLogicalPartition(block_device, *metadata.get(), partition, force_writable, - path); + timeout_ms, path); } } LERROR << "Could not find any partition with name: " << partition_name; return false; } -bool DestroyLogicalPartition(const std::string& name) { +bool DestroyLogicalPartition(const std::string& name, const std::chrono::milliseconds& timeout_ms) { DeviceMapper& dm = DeviceMapper::Instance(); + std::string path; + if (timeout_ms > std::chrono::milliseconds::zero()) { + dm.GetDmDevicePathByName(name, &path); + } if (!dm.DeleteDevice(name)) { return false; } + if (!path.empty() && !fs_mgr_wait_for_file(path, timeout_ms, FileWaitMode::DoesNotExist)) { + LERROR << "Timed out waiting for device path to unlink: " << path; + 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 a347faf53..ebc4a0f08 100644 --- a/fs_mgr/fs_mgr_priv.h +++ b/fs_mgr/fs_mgr_priv.h @@ -119,9 +119,13 @@ using namespace std::chrono_literals; -int fs_mgr_set_blk_ro(const char *blockdev); +enum class FileWaitMode { Exists, DoesNotExist }; + bool fs_mgr_wait_for_file(const std::string& filename, - const std::chrono::milliseconds relative_timeout); + const std::chrono::milliseconds relative_timeout, + FileWaitMode wait_mode = FileWaitMode::Exists); + +int fs_mgr_set_blk_ro(const char* blockdev); bool fs_mgr_update_for_slotselect(struct fstab *fstab); bool fs_mgr_is_device_unlocked(); const std::string& get_android_dt_dir(); diff --git a/fs_mgr/include/fs_mgr_dm_linear.h b/fs_mgr/include/fs_mgr_dm_linear.h index f15c45031..08f455474 100644 --- a/fs_mgr/include/fs_mgr_dm_linear.h +++ b/fs_mgr/include/fs_mgr_dm_linear.h @@ -27,6 +27,7 @@ #include +#include #include #include #include @@ -43,12 +44,16 @@ bool CreateLogicalPartitions(const std::string& block_device); // the partition name. On success, a path to the partition's block device is // returned. If |force_writable| is true, the "readonly" flag will be ignored // so the partition can be flashed. +// +// If |timeout_ms| is non-zero, then CreateLogicalPartition will block for the +// given amount of time until the path returned in |path| is available. bool CreateLogicalPartition(const std::string& block_device, uint32_t metadata_slot, const std::string& partition_name, bool force_writable, - std::string* path); + const std::chrono::milliseconds& timeout_ms, std::string* path); -// Destroy the block device for a logical partition, by name. -bool DestroyLogicalPartition(const std::string& name); +// Destroy the block device for a logical partition, by name. If |timeout_ms| +// is non-zero, then this will block until the device path has been unlinked. +bool DestroyLogicalPartition(const std::string& name, const std::chrono::milliseconds& timeout_ms); } // namespace fs_mgr } // namespace android