From 7a8f82acbd3c24ce5ecc7e57d633576b74af30e9 Mon Sep 17 00:00:00 2001 From: Daniel Zheng Date: Thu, 21 Sep 2023 20:37:47 -0700 Subject: [PATCH] should_flash_in_userspace signature change We should change should_flash_in_userspace to work without $ANDROID_PRODUCT_OUT being set and fall back on $OUT directory when source isn't specified This also fixes the flashing issue of flashing retrofit devices without $ANDROID_PRODUCT_OUT set. If we call fastboot update on a retrofit device, copy_avb_footer will resize buf->sz of secondary partitions to the partition size, causing us to send the unsparsed payload to the device -> "Invalid Size" issue. This is because secondary partitions are considered physical partitions on retrofit devices && should_flash_in_userspace wouldn't work if $ANDROID_PRODUCT_OUT is set, so our first return clause in copy_avb_footer doesn't catch this edge case causing us to resize the buf-> sz incorrectly. Did some testing, and also observed a case where flashing is able to succeed despite $ANDROID_PRODUCT_OUT not being set before, so there might still be some other edge case that we are still missing. Test: fastboot update on sargo without $ANDROID_PRODUCT_OUT Change-Id: I0e4781d96709b712f7d71657ec0d16d99b90214d --- fastboot/fastboot.cpp | 50 ++++++++++++++++++++++++++----------------- fastboot/fastboot.h | 2 +- fastboot/task.cpp | 3 ++- 3 files changed, 33 insertions(+), 22 deletions(-) diff --git a/fastboot/fastboot.cpp b/fastboot/fastboot.cpp index 63f784359..3ce814179 100644 --- a/fastboot/fastboot.cpp +++ b/fastboot/fastboot.cpp @@ -1185,9 +1185,10 @@ static uint64_t get_partition_size(const std::string& partition) { return partition_size; } -static void copy_avb_footer(const std::string& partition, struct fastboot_buffer* buf) { +static void copy_avb_footer(const ImageSource* source, const std::string& partition, + struct fastboot_buffer* buf) { if (buf->sz < AVB_FOOTER_SIZE || is_logical(partition) || - should_flash_in_userspace(partition)) { + should_flash_in_userspace(source, partition)) { return; } // If overflows and negative, it should be < buf->sz. @@ -1244,9 +1245,9 @@ void flash_partition_files(const std::string& partition, const std::vectorResizePartition(pname, std::to_string(buf.image_size)); } std::string flash_pname = repack_ramdisk(pname, &buf, fp->fb); - flash_buf(flash_pname, &buf, apply_vbmeta); + flash_buf(fp->source, flash_pname, &buf, apply_vbmeta); } // Sets slot_override as the active slot. If slot_override is blank, @@ -1807,9 +1808,9 @@ std::vector> FlashAllTool::CollectTasks() { tasks = CollectTasksFromImageList(); } if (fp_->exclude_dynamic_partitions) { - auto is_non_static_flash_task = [](const auto& task) -> bool { + auto is_non_static_flash_task = [&](const auto& task) -> bool { if (auto flash_task = task->AsFlashTask()) { - if (!should_flash_in_userspace(flash_task->GetPartitionAndSlot())) { + if (!should_flash_in_userspace(fp_->source, flash_task->GetPartitionAndSlot())) { return false; } } @@ -1885,7 +1886,7 @@ std::vector> FlashAllTool::CollectTasksFromImageList() { // this, we delete any logical partitions for the "other" slot. if (is_retrofit_device(fp_->source)) { std::string partition_name = image->part_name + "_" + slot; - if (image->IsSecondary() && should_flash_in_userspace(partition_name)) { + if (image->IsSecondary() && should_flash_in_userspace(fp_->source, partition_name)) { tasks.emplace_back(std::make_unique(fp_, partition_name)); } } @@ -2087,7 +2088,8 @@ void fb_perform_format(const std::string& partition, int skip_if_not_supported, if (!load_buf_fd(std::move(fd), &buf, fp)) { die("Cannot read image: %s", strerror(errno)); } - flash_buf(partition, &buf, is_vbmeta_partition(partition)); + + flash_buf(fp->source, partition, &buf, is_vbmeta_partition(partition)); return; failed: @@ -2101,18 +2103,26 @@ failed: } } -bool should_flash_in_userspace(const std::string& partition_name) { - if (!get_android_product_out()) { - return false; - } - auto path = find_item_given_name("super_empty.img"); - if (path.empty() || access(path.c_str(), R_OK)) { - return false; - } - auto metadata = android::fs_mgr::ReadFromImageFile(path); - if (!metadata) { +bool should_flash_in_userspace(const ImageSource* source, const std::string& partition_name) { + if (!source) { + if (!get_android_product_out()) { + return false; + } + auto path = find_item_given_name("super_empty.img"); + if (path.empty() || access(path.c_str(), R_OK)) { + return false; + } + auto metadata = android::fs_mgr::ReadFromImageFile(path); + if (!metadata) { + return false; + } + return should_flash_in_userspace(*metadata.get(), partition_name); + } + std::vector contents; + if (!source->ReadFile("super_empty.img", &contents)) { return false; } + auto metadata = android::fs_mgr::ReadFromImageBlob(contents.data(), contents.size()); return should_flash_in_userspace(*metadata.get(), partition_name); } diff --git a/fastboot/fastboot.h b/fastboot/fastboot.h index eead82c92..4859cebc2 100644 --- a/fastboot/fastboot.h +++ b/fastboot/fastboot.h @@ -148,7 +148,7 @@ class LocalImageSource final : public ImageSource { }; char* get_android_product_out(); -bool should_flash_in_userspace(const std::string& partition_name); +bool should_flash_in_userspace(const ImageSource* source, const std::string& partition_name); bool is_userspace_fastboot(); void do_flash(const char* pname, const char* fname, const bool apply_vbmeta, const FlashingPlan* fp); diff --git a/fastboot/task.cpp b/fastboot/task.cpp index f13dd55b3..4b2b9e316 100644 --- a/fastboot/task.cpp +++ b/fastboot/task.cpp @@ -41,7 +41,8 @@ bool FlashTask::IsDynamicParitition(const ImageSource* source, const FlashTask* void FlashTask::Run() { auto flash = [&](const std::string& partition) { - if (should_flash_in_userspace(partition) && !is_userspace_fastboot() && !fp_->force_flash) { + if (should_flash_in_userspace(fp_->source, partition) && !is_userspace_fastboot() && + !fp_->force_flash) { die("The partition you are trying to flash is dynamic, and " "should be flashed via fastbootd. Please run:\n" "\n"