From 9856460763f0d62e91fe31363a58c0b64b98f0da Mon Sep 17 00:00:00 2001 From: Tom Cherry Date: Tue, 13 Nov 2018 11:33:42 -0800 Subject: [PATCH] fs_mgr: remove by_name_symlinks_map_ from AvpOps With the addition of the /dev/block/by-name/ symlinks created for the boot_device, we no longer need to use a map to track the symlinks for the partitions AVB needs to access. This will help us in removing the requirement to specify which partitions contain AVB metadata. Bug: 117933812 Test: boot blueline_mainline with AVB Change-Id: I1d46dba5b2fc16b2a14f861b34225ac0f2995b60 --- fs_mgr/fs_mgr.cpp | 4 ++-- fs_mgr/fs_mgr_avb.cpp | 19 +++------------- fs_mgr/fs_mgr_avb_ops.cpp | 27 ++++------------------- fs_mgr/fs_mgr_priv_avb_ops.h | 5 +---- fs_mgr/include/fs_mgr_avb.h | 11 +--------- init/first_stage_mount.cpp | 42 +++--------------------------------- 6 files changed, 14 insertions(+), 94 deletions(-) diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp index c321fe3eb..8e57e1ed0 100644 --- a/fs_mgr/fs_mgr.cpp +++ b/fs_mgr/fs_mgr.cpp @@ -1031,7 +1031,7 @@ int fs_mgr_mount_all(fstab* fstab, int mount_mode) { if (fstab->recs[i].fs_mgr_flags & MF_AVB) { if (!avb_handle) { - avb_handle = FsManagerAvbHandle::Open(*fstab); + avb_handle = FsManagerAvbHandle::Open(); if (!avb_handle) { LERROR << "Failed to open FsManagerAvbHandle"; return FS_MGR_MNTALL_FAIL; @@ -1275,7 +1275,7 @@ static int fs_mgr_do_mount_helper(fstab* fstab, const char* n_name, char* n_blk_ if (fstab->recs[i].fs_mgr_flags & MF_AVB) { if (!avb_handle) { - avb_handle = FsManagerAvbHandle::Open(*fstab); + avb_handle = FsManagerAvbHandle::Open(); if (!avb_handle) { LERROR << "Failed to open FsManagerAvbHandle"; return FS_MGR_DOMNT_FAILED; diff --git a/fs_mgr/fs_mgr_avb.cpp b/fs_mgr/fs_mgr_avb.cpp index 31affbe7d..6f94d454c 100644 --- a/fs_mgr/fs_mgr_avb.cpp +++ b/fs_mgr/fs_mgr_avb.cpp @@ -361,21 +361,7 @@ static bool get_hashtree_descriptor(const std::string& partition_name, return true; } -FsManagerAvbUniquePtr FsManagerAvbHandle::Open(const fstab& fstab) { - FsManagerAvbOps avb_ops(fstab); - return DoOpen(&avb_ops); -} - -FsManagerAvbUniquePtr FsManagerAvbHandle::Open(ByNameSymlinkMap&& by_name_symlink_map) { - if (by_name_symlink_map.empty()) { - LERROR << "Empty by_name_symlink_map when opening FsManagerAvbHandle"; - return nullptr; - } - FsManagerAvbOps avb_ops(std::move(by_name_symlink_map)); - return DoOpen(&avb_ops); -} - -FsManagerAvbUniquePtr FsManagerAvbHandle::DoOpen(FsManagerAvbOps* avb_ops) { +FsManagerAvbUniquePtr FsManagerAvbHandle::Open() { bool is_device_unlocked = fs_mgr_is_device_unlocked(); FsManagerAvbUniquePtr avb_handle(new FsManagerAvbHandle()); @@ -384,10 +370,11 @@ FsManagerAvbUniquePtr FsManagerAvbHandle::DoOpen(FsManagerAvbOps* avb_ops) { return nullptr; } + FsManagerAvbOps avb_ops; AvbSlotVerifyFlags flags = is_device_unlocked ? AVB_SLOT_VERIFY_FLAGS_ALLOW_VERIFICATION_ERROR : AVB_SLOT_VERIFY_FLAGS_NONE; AvbSlotVerifyResult verify_result = - avb_ops->AvbSlotVerify(fs_mgr_get_slot_suffix(), flags, &avb_handle->avb_slot_data_); + avb_ops.AvbSlotVerify(fs_mgr_get_slot_suffix(), flags, &avb_handle->avb_slot_data_); // Only allow two verify results: // - AVB_SLOT_VERIFY_RESULT_OK. diff --git a/fs_mgr/fs_mgr_avb_ops.cpp b/fs_mgr/fs_mgr_avb_ops.cpp index 43879fe56..18efa2266 100644 --- a/fs_mgr/fs_mgr_avb_ops.cpp +++ b/fs_mgr/fs_mgr_avb_ops.cpp @@ -40,6 +40,8 @@ #include "fs_mgr.h" #include "fs_mgr_priv.h" +using namespace std::literals; + static AvbIOResult read_from_partition(AvbOps* ops, const char* partition, int64_t offset, size_t num_bytes, void* buffer, size_t* out_num_read) { return FsManagerAvbOps::GetInstanceFromAvbOps(ops)->ReadFromPartition( @@ -98,7 +100,7 @@ static AvbIOResult dummy_get_size_of_partition(AvbOps* ops ATTRIBUTE_UNUSED, return AVB_IO_RESULT_OK; } -void FsManagerAvbOps::InitializeAvbOps() { +FsManagerAvbOps::FsManagerAvbOps() { // We only need to provide the implementation of read_from_partition() // operation since that's all what is being used by the avb_slot_verify(). // Other I/O operations are only required in bootloader but not in @@ -116,31 +118,10 @@ void FsManagerAvbOps::InitializeAvbOps() { avb_ops_.user_data = this; } -FsManagerAvbOps::FsManagerAvbOps(std::map&& by_name_symlink_map) - : by_name_symlink_map_(std::move(by_name_symlink_map)) { - InitializeAvbOps(); -} - -FsManagerAvbOps::FsManagerAvbOps(const fstab& fstab) { - // Constructs the by-name symlink map for each fstab record. - // /dev/block/platform/soc.0/7824900.sdhci/by-name/system_a => - // by_name_symlink_map_["system_a"] = "/dev/block/platform/soc.0/7824900.sdhci/by-name/system_a" - for (int i = 0; i < fstab.num_entries; i++) { - std::string partition_name = basename(fstab.recs[i].blk_device); - by_name_symlink_map_[partition_name] = fstab.recs[i].blk_device; - } - InitializeAvbOps(); -} - AvbIOResult FsManagerAvbOps::ReadFromPartition(const char* partition, int64_t offset, size_t num_bytes, void* buffer, size_t* out_num_read) { - const auto iter = by_name_symlink_map_.find(partition); - if (iter == by_name_symlink_map_.end()) { - LERROR << "by-name symlink not found for partition: '" << partition << "'"; - return AVB_IO_RESULT_ERROR_NO_SUCH_PARTITION; - } - std::string path = iter->second; + const std::string path = "/dev/block/by-name/"s + partition; // Ensures the device path (a symlink created by init) is ready to access. if (!fs_mgr_wait_for_file(path, 1s)) { diff --git a/fs_mgr/fs_mgr_priv_avb_ops.h b/fs_mgr/fs_mgr_priv_avb_ops.h index d1ef2e91d..44eb1da43 100644 --- a/fs_mgr/fs_mgr_priv_avb_ops.h +++ b/fs_mgr/fs_mgr_priv_avb_ops.h @@ -46,8 +46,7 @@ // class FsManagerAvbOps { public: - FsManagerAvbOps(const fstab& fstab); - FsManagerAvbOps(std::map&& by_name_symlink_map); + FsManagerAvbOps(); static FsManagerAvbOps* GetInstanceFromAvbOps(AvbOps* ops) { return reinterpret_cast(ops->user_data); @@ -60,8 +59,6 @@ class FsManagerAvbOps { AvbSlotVerifyData** out_data); private: - void InitializeAvbOps(); - AvbOps avb_ops_; std::map by_name_symlink_map_; }; diff --git a/fs_mgr/include/fs_mgr_avb.h b/fs_mgr/include/fs_mgr_avb.h index 73a22c863..bb55a14b0 100644 --- a/fs_mgr/include/fs_mgr_avb.h +++ b/fs_mgr/include/fs_mgr_avb.h @@ -53,13 +53,6 @@ class FsManagerAvbHandle { // A typical usage will be: // - FsManagerAvbUniquePtr handle = FsManagerAvbHandle::Open(); // - // There are two overloaded Open() functions with a single parameter. - // The argument can be a ByNameSymlinkMap describing the mapping from partition - // name to by-name symlink, or a fstab file to which the ByNameSymlinkMap is - // constructed from. e.g., - // - /dev/block/platform/soc.0/7824900.sdhci/by-name/system_a -> - // - ByNameSymlinkMap["system_a"] = "/dev/block/platform/soc.0/7824900.sdhci/by-name/system_a" - // // Possible return values: // - nullptr: any error when reading and verifying the metadata, // e.g., I/O error, digest value mismatch, size mismatch, etc. @@ -82,8 +75,7 @@ class FsManagerAvbHandle { // - a valid unique_ptr with status kAvbHandleSuccess: the metadata // is verified and can be trusted. // - static FsManagerAvbUniquePtr Open(const fstab& fstab); - static FsManagerAvbUniquePtr Open(ByNameSymlinkMap&& by_name_symlink_map); + static FsManagerAvbUniquePtr Open(); // Sets up dm-verity on the given fstab entry. // The 'wait_for_verity_dev' parameter makes this function wait for the @@ -121,7 +113,6 @@ class FsManagerAvbHandle { }; FsManagerAvbHandle() : avb_slot_data_(nullptr), status_(kAvbHandleUninitialized) {} - static FsManagerAvbUniquePtr DoOpen(FsManagerAvbOps* avb_ops); AvbSlotVerifyData* avb_slot_data_; AvbHandleStatus status_; diff --git a/init/first_stage_mount.cpp b/init/first_stage_mount.cpp index 1e434d240..43b2ebf8c 100644 --- a/init/first_stage_mount.cpp +++ b/init/first_stage_mount.cpp @@ -73,7 +73,7 @@ class FirstStageMount { bool GetDmLinearMetadataDevice(); bool InitDmLinearBackingDevices(const android::fs_mgr::LpMetadata& metadata); - virtual ListenerAction UeventCallback(const Uevent& uevent); + ListenerAction UeventCallback(const Uevent& uevent); // Pure virtual functions. virtual bool GetDmVerityDevices() = 0; @@ -108,14 +108,12 @@ class FirstStageMountVBootV2 : public FirstStageMount { ~FirstStageMountVBootV2() override = default; protected: - ListenerAction UeventCallback(const Uevent& uevent) override; bool GetDmVerityDevices() override; bool SetUpDmVerity(fstab_rec* fstab_rec) override; bool InitAvbHandle(); std::string device_tree_vbmeta_parts_; FsManagerAvbUniquePtr avb_handle_; - ByNameSymlinkMap by_name_symlink_map_; }; // Static Functions @@ -544,33 +542,6 @@ bool FirstStageMountVBootV2::GetDmVerityDevices() { return true; } -ListenerAction FirstStageMountVBootV2::UeventCallback(const Uevent& uevent) { - // Check if this uevent corresponds to one of the required partitions and store its symlinks if - // so, in order to create FsManagerAvbHandle later. - // Note that the parent callback removes partitions from the list of required partitions - // as it finds them, so this must happen first. - if (!uevent.partition_name.empty() && - required_devices_partition_names_.find(uevent.partition_name) != - required_devices_partition_names_.end()) { - // GetBlockDeviceSymlinks() will return three symlinks at most, depending on - // the content of uevent. by-name symlink will be at [0] if uevent->partition_name - // is not empty. e.g., - // - /dev/block/platform/soc.0/f9824900.sdhci/by-name/modem - // - /dev/block/platform/soc.0/f9824900.sdhci/mmcblk0p1 - std::vector links = device_handler_->GetBlockDeviceSymlinks(uevent); - if (!links.empty()) { - auto [it, inserted] = by_name_symlink_map_.emplace(uevent.partition_name, links[0]); - if (!inserted && (links[0] != it->second)) { - LOG(ERROR) << "Partition '" << uevent.partition_name - << "' already existed in the by-name symlink map with a value of '" - << it->second << "', new value '" << links[0] << "' will be ignored."; - } - } - } - - return FirstStageMount::UeventCallback(uevent); -} - bool FirstStageMountVBootV2::SetUpDmVerity(fstab_rec* fstab_rec) { if (fs_mgr_is_avb(fstab_rec)) { if (!InitAvbHandle()) return false; @@ -594,13 +565,7 @@ bool FirstStageMountVBootV2::SetUpDmVerity(fstab_rec* fstab_rec) { bool FirstStageMountVBootV2::InitAvbHandle() { if (avb_handle_) return true; // Returns true if the handle is already initialized. - if (by_name_symlink_map_.empty()) { - LOG(ERROR) << "by_name_symlink_map_ is empty"; - return false; - } - - avb_handle_ = FsManagerAvbHandle::Open(std::move(by_name_symlink_map_)); - by_name_symlink_map_.clear(); // Removes all elements after the above std::move(). + avb_handle_ = FsManagerAvbHandle::Open(); if (!avb_handle_) { PLOG(ERROR) << "Failed to open FsManagerAvbHandle"; @@ -651,8 +616,7 @@ void SetInitAvbVersionInRecovery() { return; } - FsManagerAvbUniquePtr avb_handle = - FsManagerAvbHandle::Open(std::move(avb_first_mount.by_name_symlink_map_)); + FsManagerAvbUniquePtr avb_handle = FsManagerAvbHandle::Open(); if (!avb_handle) { PLOG(ERROR) << "Failed to open FsManagerAvbHandle for INIT_AVB_VERSION"; return;