From 7627b549104ce2dda9c42002181c6a5db56f6939 Mon Sep 17 00:00:00 2001 From: Daniel Zheng Date: Thu, 13 Apr 2023 13:43:17 -0700 Subject: [PATCH 01/16] Moving logging include Header file shouldn't have include of logging Test: tested compilation Change-Id: I02cccb66b6ee5811e5d4f2b9b72508fee640e828 --- fastboot/fastboot_driver.h | 1 - fastboot/task.cpp | 7 +++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/fastboot/fastboot_driver.h b/fastboot/fastboot_driver.h index f60c9f1c3..64e3c2b86 100644 --- a/fastboot/fastboot_driver.h +++ b/fastboot/fastboot_driver.h @@ -33,7 +33,6 @@ #include #include -#include #include #include #include diff --git a/fastboot/task.cpp b/fastboot/task.cpp index de48a16de..d43fd8084 100644 --- a/fastboot/task.cpp +++ b/fastboot/task.cpp @@ -14,13 +14,16 @@ // limitations under the License. // #include "task.h" + #include + +#include +#include + #include "fastboot.h" #include "filesystem.h" #include "super_flash_helper.h" -#include - using namespace std::string_literals; FlashTask::FlashTask(const std::string& _slot, const std::string& _pname, const std::string& _fname, const bool apply_vbmeta) From 0897526557e66dbb74523d67c314c4f72affb7f8 Mon Sep 17 00:00:00 2001 From: Daniel Zheng Date: Mon, 17 Apr 2023 11:34:00 -0700 Subject: [PATCH 02/16] Fixing circular dependency Test: tested compilation Change-Id: Iae25cade1efb2372e5fc219fa69cbb2381b4e05c --- fastboot/task.h | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/fastboot/task.h b/fastboot/task.h index e80f88de2..801a0f6bf 100644 --- a/fastboot/task.h +++ b/fastboot/task.h @@ -18,11 +18,14 @@ #include #include -#include "fastboot.h" #include "fastboot_driver.h" #include "super_flash_helper.h" #include "util.h" +struct FlashingPlan; +struct Image; +using ImageEntry = std::pair; + class Task { public: Task() = default; From 93c9dfcd335e3c06ac52b5a8e9ed25260efe5e45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Maciej=20=C5=BBenczykowski?= Date: Tue, 18 Apr 2023 21:08:12 +0000 Subject: [PATCH 03/16] fix clatd permissions try 3 - this time for GSI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit where stuff is apparently under /system_ext/apex/... instead of /system/apex/... Bug: 277646103 Test: TreeHugger Signed-off-by: Maciej Żenczykowski Change-Id: I947e44af334628d82ca633546f3328319c2bac60 --- libcutils/fs_config.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/libcutils/fs_config.cpp b/libcutils/fs_config.cpp index e46774ba4..79d79dd44 100644 --- a/libcutils/fs_config.cpp +++ b/libcutils/fs_config.cpp @@ -90,6 +90,8 @@ static const struct fs_path_config android_dirs[] = { { 00755, AID_ROOT, AID_SHELL, 0, "system/vendor" }, { 00750, AID_ROOT, AID_SHELL, 0, "system/xbin" }, { 00751, AID_ROOT, AID_SHELL, 0, "system/apex/*/bin" }, + { 00750, AID_ROOT, AID_SYSTEM, 0, "system_ext/apex/com.android.tethering/bin/for-system" }, + { 00750, AID_ROOT, AID_SYSTEM, 0, "system_ext/apex/com.android.tethering.inprocess/bin/for-system" }, { 00751, AID_ROOT, AID_SHELL, 0, "system_ext/bin" }, { 00751, AID_ROOT, AID_SHELL, 0, "system_ext/apex/*/bin" }, { 00751, AID_ROOT, AID_SHELL, 0, "vendor/bin" }, @@ -198,6 +200,8 @@ static const struct fs_path_config android_files[] = { // in user builds. { 06755, AID_CLAT, AID_CLAT, 0, "system/apex/com.android.tethering/bin/for-system/clatd" }, { 06755, AID_CLAT, AID_CLAT, 0, "system/apex/com.android.tethering.inprocess/bin/for-system/clatd" }, + { 06755, AID_CLAT, AID_CLAT, 0, "system_ext/apex/com.android.tethering/bin/for-system/clatd" }, + { 06755, AID_CLAT, AID_CLAT, 0, "system_ext/apex/com.android.tethering.inprocess/bin/for-system/clatd" }, { 00700, AID_SYSTEM, AID_SHELL, CAP_MASK_LONG(CAP_BLOCK_SUSPEND), "system/bin/inputflinger" }, { 00750, AID_ROOT, AID_SHELL, CAP_MASK_LONG(CAP_SETUID) | From 29a211c19402c4c972a3afed9f302ef893bff0e9 Mon Sep 17 00:00:00 2001 From: Daniel Zheng Date: Fri, 10 Mar 2023 07:23:49 +0000 Subject: [PATCH 04/16] Changing flashall to parse fastboot-info.txt Test: tested on Raven Change-Id: I9f26bf6025a0a6318f84be929091f4e82bd87556 --- fastboot/fastboot.cpp | 292 +++++++++++++++++++++++++++++++++++++----- fastboot/fastboot.h | 4 +- fastboot/task.cpp | 88 ++++++++++++- fastboot/task.h | 24 +++- 4 files changed, 375 insertions(+), 33 deletions(-) diff --git a/fastboot/fastboot.cpp b/fastboot/fastboot.cpp index d7d2091bb..6b0df523e 100644 --- a/fastboot/fastboot.cpp +++ b/fastboot/fastboot.cpp @@ -1284,7 +1284,7 @@ static void flash_buf(const std::string& partition, struct fastboot_buffer* buf, } } -static std::string get_current_slot() { +std::string get_current_slot() { std::string current_slot; if (fb->GetVar("current-slot", ¤t_slot) != fastboot::SUCCESS) return ""; if (current_slot[0] == '_') current_slot.erase(0, 1); @@ -1560,7 +1560,7 @@ static void CancelSnapshotIfNeeded() { } } -std::string GetPartitionName(const ImageEntry& entry, std::string& current_slot) { +std::string GetPartitionName(const ImageEntry& entry, const std::string& current_slot) { auto slot = entry.second; if (slot.empty()) { slot = current_slot; @@ -1574,6 +1574,216 @@ std::string GetPartitionName(const ImageEntry& entry, std::string& current_slot) return entry.first->part_name + "_" + slot; } +std::unique_ptr ParseFlashCommand(const FlashingPlan* fp, + const std::vector& parts) { + bool apply_vbmeta = false; + std::string slot = fp->slot_override; + std::string partition; + std::string img_name; + for (auto& part : parts) { + if (part == "--apply-vbmeta") { + apply_vbmeta = true; + } else if (part == "--slot-other") { + slot = fp->secondary_slot; + } else if (partition.empty()) { + partition = part; + } else if (img_name.empty()) { + img_name = part; + } else { + LOG(ERROR) << "unknown argument" << part + << " in fastboot-info.txt. parts: " << android::base::Join(parts, " "); + return nullptr; + } + } + if (partition.empty()) { + LOG(ERROR) << "partition name not found when parsing fastboot-info.txt. parts: " + << android::base::Join(parts, " "); + return nullptr; + } + if (img_name.empty()) { + img_name = partition + ".img"; + } + return std::make_unique(slot, partition, img_name, apply_vbmeta); +} + +std::unique_ptr ParseRebootCommand(const FlashingPlan* fp, + const std::vector& parts) { + if (parts.empty()) return std::make_unique(fp); + if (parts.size() > 1) { + LOG(ERROR) << "unknown arguments in reboot {target} in fastboot-info.txt: " + << android::base::Join(parts, " "); + return nullptr; + } + return std::make_unique(fp, parts[0]); +} + +std::unique_ptr ParseWipeCommand(const FlashingPlan* fp, + const std::vector& parts) { + if (parts.size() != 1) { + LOG(ERROR) << "unknown arguments in erase {partition} in fastboot-info.txt: " + << android::base::Join(parts, " "); + return nullptr; + } + return std::make_unique(fp, parts[0]); +} + +std::unique_ptr ParseFastbootInfoLine(const FlashingPlan* fp, + const std::vector& command) { + if (command.size() == 0) { + return nullptr; + } + std::unique_ptr task; + + if (command[0] == "flash") { + task = ParseFlashCommand(fp, std::vector{command.begin() + 1, command.end()}); + } else if (command[0] == "reboot") { + task = ParseRebootCommand(fp, std::vector{command.begin() + 1, command.end()}); + } else if (command[0] == "update-super" && command.size() == 1) { + task = std::make_unique(fp); + } else if (command[0] == "erase" && command.size() == 2) { + task = ParseWipeCommand(fp, std::vector{command.begin() + 1, command.end()}); + } + if (!task) { + LOG(ERROR) << "unknown command parsing fastboot-info.txt line: " + << android::base::Join(command, " "); + } + return task; +} + +void AddResizeTasks(const FlashingPlan* fp, std::vector>* tasks) { + // expands "resize-partitions" into individual commands : resize {os_partition_1}, resize + // {os_partition_2}, etc. + std::vector> resize_tasks; + std::optional loc; + for (size_t i = 0; i < tasks->size(); i++) { + if (auto flash_task = tasks->at(i)->AsFlashTask()) { + if (should_flash_in_userspace(flash_task->GetPartitionAndSlot())) { + if (!loc) { + loc = i; + } + resize_tasks.emplace_back(std::make_unique( + fp, flash_task->GetPartition(), "0", fp->slot_override)); + } + } + } + // if no logical partitions (although should never happen since system will always need to be + // flashed) + if (!loc) { + return; + } + tasks->insert(tasks->begin() + loc.value(), std::make_move_iterator(resize_tasks.begin()), + std::make_move_iterator(resize_tasks.end())); + return; +} + +static bool IsNumber(const std::string& s) { + bool period = false; + for (size_t i = 0; i < s.length(); i++) { + if (!isdigit(s[i])) { + if (!period && s[i] == '.' && i != 0 && i != s.length() - 1) { + period = true; + } else { + return false; + } + } + } + return true; +} + +static bool IsIgnore(const std::vector& command) { + if (command[0][0] == '#') { + return true; + } + return false; +} + +bool CheckFastbootInfoRequirements(const std::vector& command) { + if (command.size() != 2) { + LOG(ERROR) << "unknown characters in version info in fastboot-info.txt -> " + << android::base::Join(command, " "); + return false; + } + if (command[0] != "version") { + LOG(ERROR) << "unknown characters in version info in fastboot-info.txt -> " + << android::base::Join(command, " "); + return false; + } + + if (!IsNumber(command[1])) { + LOG(ERROR) << "version number contains non-numeric values in fastboot-info.txt -> " + << android::base::Join(command, " "); + return false; + } + + LOG(VERBOSE) << "Checking 'fastboot-info.txt version'"; + if (command[1] < PLATFORM_TOOLS_VERSION) { + return true; + } + LOG(ERROR) << "fasboot-info.txt version: " << command[1] + << " not compatible with host tool version --> " << PLATFORM_TOOLS_VERSION; + return false; +} + +std::vector> ParseFastbootInfo(const FlashingPlan* fp, + const std::vector& file) { + std::vector> tasks; + // Get os_partitions that need to be resized + for (auto& text : file) { + std::vector command = android::base::Tokenize(text, " "); + if (IsIgnore(command)) { + continue; + } + if (command.size() > 1 && command[0] == "version") { + if (!CheckFastbootInfoRequirements(command)) { + return {}; + } + continue; + } else if (command.size() >= 2 && command[0] == "if-wipe") { + if (!fp->wants_wipe) { + continue; + } + command.erase(command.begin()); + } + auto task = ParseFastbootInfoLine(fp, command); + if (!task) { + LOG(ERROR) << "Error when parsing fastboot-info.txt, falling back on Hardcoded list: " + << text; + return {}; + } + tasks.emplace_back(std::move(task)); + } + if (auto flash_super_task = FlashSuperLayoutTask::InitializeFromTasks(fp, tasks)) { + auto it = tasks.begin(); + for (size_t i = 0; i < tasks.size(); i++) { + if (auto flash_task = tasks[i]->AsFlashTask()) { + if (should_flash_in_userspace(flash_task->GetPartitionAndSlot())) { + break; + } + } + if (auto wipe_task = tasks[i]->AsWipeTask()) { + break; + } + it++; + } + tasks.insert(it, std::move(flash_super_task)); + } else { + AddResizeTasks(fp, &tasks); + } + return tasks; +} + +std::vector> ParseFastbootInfo(const FlashingPlan* fp, std::ifstream& fs) { + if (!fs || fs.eof()) return {}; + + std::string text; + std::vector file; + // Get os_partitions that need to be resized + while (std::getline(fs, text)) { + file.emplace_back(text); + } + return ParseFastbootInfo(fp, file); +} + class FlashAllTool { public: FlashAllTool(FlashingPlan* fp); @@ -1586,6 +1796,7 @@ class FlashAllTool { void CollectImages(); void FlashImages(const std::vector>& images); void FlashImage(const Image& image, const std::string& slot, fastboot_buffer* buf); + void HardcodedFlash(); std::vector boot_images_; std::vector os_images_; @@ -1611,38 +1822,23 @@ void FlashAllTool::Flash() { CancelSnapshotIfNeeded(); - // First flash boot partitions. We allow this to happen either in userspace - // or in bootloader fastboot. - FlashImages(boot_images_); - - std::vector> tasks; - - if (auto flash_super_task = FlashSuperLayoutTask::Initialize(fp_, os_images_)) { - tasks.emplace_back(std::move(flash_super_task)); - } else { - // Sync the super partition. This will reboot to userspace fastboot if needed. - tasks.emplace_back(std::make_unique(fp_)); - // Resize any logical partition to 0, so each partition is reset to 0 - // extents, and will achieve more optimal allocation. - for (const auto& [image, slot] : os_images_) { - // Retrofit devices have two super partitions, named super_a and super_b. - // On these devices, secondary slots must be flashed as physical - // partitions (otherwise they would not mount on first boot). To enforce - // this, we delete any logical partitions for the "other" slot. - if (is_retrofit_device()) { - std::string partition_name = image->part_name + "_"s + slot; - if (image->IsSecondary() && is_logical(partition_name)) { - fp_->fb->DeletePartition(partition_name); - } - tasks.emplace_back(std::make_unique(fp_, partition_name)); - } - tasks.emplace_back(std::make_unique(fp_, image->part_name, "0", slot)); - } + std::string path = find_item_given_name("fastboot-info.txt"); + std::ifstream stream(path); + std::vector> tasks = ParseFastbootInfo(fp_, stream); + if (tasks.empty()) { + LOG(VERBOSE) << "Flashing from hardcoded images. fastboot-info.txt is empty or does not " + "exist"; + HardcodedFlash(); + return; } + LOG(VERBOSE) << "Flashing from fastboot-info.txt"; for (auto& task : tasks) { task->Run(); } - FlashImages(os_images_); + if (fp_->wants_wipe) { + // avoid adding duplicate wipe tasks in fastboot main code. + fp_->wants_wipe = false; + } } void FlashAllTool::CheckRequirements() { @@ -1693,6 +1889,42 @@ void FlashAllTool::CollectImages() { } } +void FlashAllTool::HardcodedFlash() { + CollectImages(); + // First flash boot partitions. We allow this to happen either in userspace + // or in bootloader fastboot. + FlashImages(boot_images_); + + std::vector> tasks; + + if (auto flash_super_task = FlashSuperLayoutTask::Initialize(fp_, os_images_)) { + tasks.emplace_back(std::move(flash_super_task)); + } else { + // Sync the super partition. This will reboot to userspace fastboot if needed. + tasks.emplace_back(std::make_unique(fp_)); + // Resize any logical partition to 0, so each partition is reset to 0 + // extents, and will achieve more optimal allocation. + for (const auto& [image, slot] : os_images_) { + // Retrofit devices have two super partitions, named super_a and super_b. + // On these devices, secondary slots must be flashed as physical + // partitions (otherwise they would not mount on first boot). To enforce + // this, we delete any logical partitions for the "other" slot. + if (is_retrofit_device()) { + std::string partition_name = image->part_name + "_"s + slot; + if (image->IsSecondary() && should_flash_in_userspace(partition_name)) { + fp_->fb->DeletePartition(partition_name); + } + tasks.emplace_back(std::make_unique(fp_, partition_name)); + } + tasks.emplace_back(std::make_unique(fp_, image->part_name, "0", slot)); + } + } + for (auto& i : tasks) { + i->Run(); + } + FlashImages(os_images_); +} + void FlashAllTool::FlashImages(const std::vector>& images) { for (const auto& [image, slot] : images) { fastboot_buffer buf; diff --git a/fastboot/fastboot.h b/fastboot/fastboot.h index 6462a4fed..7c44bd86b 100644 --- a/fastboot/fastboot.h +++ b/fastboot/fastboot.h @@ -83,6 +83,7 @@ struct FlashingPlan { std::string slot_override; std::string current_slot; std::string secondary_slot; + fastboot::FastBootDriver* fb; }; @@ -94,6 +95,7 @@ void do_for_partitions(const std::string& part, const std::string& slot, std::string find_item(const std::string& item); void reboot_to_userspace_fastboot(); void syntax_error(const char* fmt, ...); +std::string get_current_slot(); struct NetworkSerial { Socket::Protocol protocol; @@ -103,7 +105,7 @@ struct NetworkSerial { Result ParseNetworkSerial(const std::string& serial); bool supports_AB(); -std::string GetPartitionName(const ImageEntry& entry, std::string& current_slot_); +std::string GetPartitionName(const ImageEntry& entry, const std::string& current_slot_); void flash_partition_files(const std::string& partition, const std::vector& files); int64_t get_sparse_limit(int64_t size); std::vector resparse_file(sparse_file* s, int64_t max_size); diff --git a/fastboot/task.cpp b/fastboot/task.cpp index d43fd8084..054c1edba 100644 --- a/fastboot/task.cpp +++ b/fastboot/task.cpp @@ -23,6 +23,7 @@ #include "fastboot.h" #include "filesystem.h" #include "super_flash_helper.h" +#include "util.h" using namespace std::string_literals; FlashTask::FlashTask(const std::string& _slot, const std::string& _pname, const std::string& _fname, @@ -45,6 +46,20 @@ void FlashTask::Run() { do_for_partitions(pname_, slot_, flash, true); } +std::string FlashTask::GetPartitionAndSlot() { + auto slot = slot_; + if (slot.empty()) { + slot = get_current_slot(); + } + if (slot.empty()) { + return pname_; + } + if (slot == "all") { + LOG(FATAL) << "Cannot retrieve a singular name when using all slots"; + } + return pname_ + "_" + slot; +} + RebootTask::RebootTask(const FlashingPlan* fp) : fp_(fp){}; RebootTask::RebootTask(const FlashingPlan* fp, const std::string& reboot_target) : reboot_target_(reboot_target), fp_(fp){}; @@ -93,7 +108,7 @@ void FlashSuperLayoutTask::Run() { } std::unique_ptr FlashSuperLayoutTask::Initialize( - FlashingPlan* fp, std::vector& os_images) { + const FlashingPlan* fp, std::vector& os_images) { if (!supports_AB()) { LOG(VERBOSE) << "Cannot optimize flashing super on non-AB device"; return nullptr; @@ -156,6 +171,77 @@ std::unique_ptr FlashSuperLayoutTask::Initialize( partition_size); } +std::unique_ptr FlashSuperLayoutTask::InitializeFromTasks( + const FlashingPlan* fp, std::vector>& tasks) { + if (!supports_AB()) { + LOG(VERBOSE) << "Cannot optimize flashing super on non-AB device"; + return nullptr; + } + if (fp->slot_override == "all") { + LOG(VERBOSE) << "Cannot optimize flashing super for all slots"; + return nullptr; + } + + // Does this device use dynamic partitions at all? + unique_fd fd = fp->source->OpenFile("super_empty.img"); + + if (fd < 0) { + LOG(VERBOSE) << "could not open super_empty.img"; + return nullptr; + } + + std::string super_name; + // Try to find whether there is a super partition. + if (fp->fb->GetVar("super-partition-name", &super_name) != fastboot::SUCCESS) { + super_name = "super"; + } + uint64_t partition_size; + std::string partition_size_str; + if (fp->fb->GetVar("partition-size:" + super_name, &partition_size_str) != fastboot::SUCCESS) { + LOG(VERBOSE) << "Cannot optimize super flashing: could not determine super partition"; + return nullptr; + } + partition_size_str = fb_fix_numeric_var(partition_size_str); + if (!android::base::ParseUint(partition_size_str, &partition_size)) { + LOG(VERBOSE) << "Could not parse " << super_name << " size: " << partition_size_str; + return nullptr; + } + + std::unique_ptr helper = std::make_unique(*fp->source); + if (!helper->Open(fd)) { + return nullptr; + } + + for (const auto& task : tasks) { + if (auto flash_task = task->AsFlashTask()) { + if (should_flash_in_userspace(flash_task->GetPartitionAndSlot())) { + auto partition = flash_task->GetPartitionAndSlot(); + if (!helper->AddPartition(partition, flash_task->GetImageName(), false)) { + return nullptr; + } + } + } + } + + auto s = helper->GetSparseLayout(); + if (!s) return nullptr; + // Remove images that we already flashed, just in case we have non-dynamic OS images. + auto remove_if_callback = [&](const auto& task) -> bool { + if (auto flash_task = task->AsFlashTask()) { + return helper->WillFlash(flash_task->GetPartitionAndSlot()); + } else if (auto update_super_task = task->AsUpdateSuperTask()) { + return true; + } else if (auto reboot_task = task->AsRebootTask()) { + return true; + } + return false; + }; + tasks.erase(std::remove_if(tasks.begin(), tasks.end(), remove_if_callback), tasks.end()); + + return std::make_unique(super_name, std::move(helper), std::move(s), + partition_size); +} + UpdateSuperTask::UpdateSuperTask(const FlashingPlan* fp) : fp_(fp) {} void UpdateSuperTask::Run() { diff --git a/fastboot/task.h b/fastboot/task.h index 801a0f6bf..34e3e92aa 100644 --- a/fastboot/task.h +++ b/fastboot/task.h @@ -26,10 +26,20 @@ struct FlashingPlan; struct Image; using ImageEntry = std::pair; +class FlashTask; +class RebootTask; +class UpdateSuperTask; +class WipeTask; + class Task { public: Task() = default; virtual void Run() = 0; + virtual FlashTask* AsFlashTask() { return nullptr; } + virtual RebootTask* AsRebootTask() { return nullptr; } + virtual UpdateSuperTask* AsUpdateSuperTask() { return nullptr; } + virtual WipeTask* AsWipeTask() { return nullptr; } + virtual ~Task() = default; }; @@ -37,7 +47,12 @@ class FlashTask : public Task { public: FlashTask(const std::string& slot, const std::string& pname, const std::string& fname, const bool apply_vbmeta); + virtual FlashTask* AsFlashTask() override { return this; } + std::string GetPartition() { return pname_; } + std::string GetImageName() { return fname_; } + std::string GetPartitionAndSlot(); + std::string GetSlot() { return slot_; } void Run() override; private: @@ -51,6 +66,7 @@ class RebootTask : public Task { public: RebootTask(const FlashingPlan* fp); RebootTask(const FlashingPlan* fp, const std::string& reboot_target); + virtual RebootTask* AsRebootTask() override { return this; } void Run() override; private: @@ -62,8 +78,10 @@ class FlashSuperLayoutTask : public Task { public: FlashSuperLayoutTask(const std::string& super_name, std::unique_ptr helper, SparsePtr sparse_layout, uint64_t super_size); - static std::unique_ptr Initialize(FlashingPlan* fp, + static std::unique_ptr Initialize(const FlashingPlan* fp, std::vector& os_images); + static std::unique_ptr InitializeFromTasks( + const FlashingPlan* fp, std::vector>& tasks); using ImageEntry = std::pair; void Run() override; @@ -77,6 +95,8 @@ class FlashSuperLayoutTask : public Task { class UpdateSuperTask : public Task { public: UpdateSuperTask(const FlashingPlan* fp); + virtual UpdateSuperTask* AsUpdateSuperTask() override { return this; } + void Run() override; private: @@ -109,6 +129,8 @@ class DeleteTask : public Task { class WipeTask : public Task { public: WipeTask(const FlashingPlan* fp, const std::string& pname); + virtual WipeTask* AsWipeTask() override { return this; } + void Run() override; private: From 65eb246aa26dd9dd8710ccd995b16b7f92a13060 Mon Sep 17 00:00:00 2001 From: Daniel Zheng Date: Thu, 30 Mar 2023 14:54:13 -0700 Subject: [PATCH 05/16] Moving FlashallTool Definition moved flashalltool definition to header file Test: flashall on raven Bug: 194686221 Change-Id: I5f726492ee5b3ae42755214e3de0269257b62ba8 --- fastboot/fastboot.cpp | 32 -------------------------------- fastboot/fastboot.h | 33 +++++++++++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 32 deletions(-) diff --git a/fastboot/fastboot.cpp b/fastboot/fastboot.cpp index 6b0df523e..0c75e49ab 100644 --- a/fastboot/fastboot.cpp +++ b/fastboot/fastboot.cpp @@ -115,19 +115,6 @@ static bool g_disable_verification = false; fastboot::FastBootDriver* fb = nullptr; -enum fb_buffer_type { - FB_BUFFER_FD, - FB_BUFFER_SPARSE, -}; - -struct fastboot_buffer { - enum fb_buffer_type type; - std::vector files; - int64_t sz; - unique_fd fd; - int64_t image_size; -}; - static std::vector images = { // clang-format off { "boot", "boot.img", "boot.sig", "boot", false, ImageType::BootCritical }, @@ -1784,25 +1771,6 @@ std::vector> ParseFastbootInfo(const FlashingPlan* fp, std return ParseFastbootInfo(fp, file); } -class FlashAllTool { - public: - FlashAllTool(FlashingPlan* fp); - - void Flash(); - - private: - void CheckRequirements(); - void DetermineSlot(); - void CollectImages(); - void FlashImages(const std::vector>& images); - void FlashImage(const Image& image, const std::string& slot, fastboot_buffer* buf); - void HardcodedFlash(); - - std::vector boot_images_; - std::vector os_images_; - FlashingPlan* fp_; -}; - FlashAllTool::FlashAllTool(FlashingPlan* fp) : fp_(fp) {} void FlashAllTool::Flash() { diff --git a/fastboot/fastboot.h b/fastboot/fastboot.h index 7c44bd86b..b5033ffd4 100644 --- a/fastboot/fastboot.h +++ b/fastboot/fastboot.h @@ -29,6 +29,7 @@ #include #include "fastboot_driver.h" +#include "filesystem.h" #include "super_flash_helper.h" #include "util.h" @@ -47,6 +48,19 @@ class FastBootTool { unsigned ParseFsOption(const char*); }; +enum fb_buffer_type { + FB_BUFFER_FD, + FB_BUFFER_SPARSE, +}; + +struct fastboot_buffer { + enum fb_buffer_type type; + std::vector files; + int64_t sz; + unique_fd fd; + int64_t image_size; +}; + enum class ImageType { // Must be flashed for device to boot into the kernel. BootCritical, @@ -87,6 +101,25 @@ struct FlashingPlan { fastboot::FastBootDriver* fb; }; +class FlashAllTool { + public: + FlashAllTool(FlashingPlan* fp); + + void Flash(); + + private: + void CheckRequirements(); + void DetermineSlot(); + void CollectImages(); + void FlashImages(const std::vector>& images); + void FlashImage(const Image& image, const std::string& slot, fastboot_buffer* buf); + void HardcodedFlash(); + + std::vector boot_images_; + std::vector os_images_; + FlashingPlan* fp_; +}; + bool should_flash_in_userspace(const std::string& partition_name); bool is_userspace_fastboot(); void do_flash(const char* pname, const char* fname, const bool apply_vbmeta); From c9e869bd9e2c3f663af2cb8750d2a7003ff1620c Mon Sep 17 00:00:00 2001 From: Daniel Zheng Date: Thu, 30 Mar 2023 14:59:48 -0700 Subject: [PATCH 06/16] Adding fastboot_driver interface + mock Adding mock fastboot_driver to be used for testing Test: tested fastboot_test Bug: 194686221 Change-Id: I6948310d44b75c0f389cbfd194747649aaccb0b8 --- fastboot/fastboot.h | 1 + fastboot/fastboot_driver.h | 32 ++++++--------- fastboot/fastboot_driver_interface.h | 59 ++++++++++++++++++++++++++++ fastboot/fastboot_driver_mock.h | 51 ++++++++++++++++++++++++ 4 files changed, 124 insertions(+), 19 deletions(-) create mode 100644 fastboot/fastboot_driver_interface.h create mode 100644 fastboot/fastboot_driver_mock.h diff --git a/fastboot/fastboot.h b/fastboot/fastboot.h index b5033ffd4..2d79e42a0 100644 --- a/fastboot/fastboot.h +++ b/fastboot/fastboot.h @@ -29,6 +29,7 @@ #include #include "fastboot_driver.h" +#include "fastboot_driver_interface.h" #include "filesystem.h" #include "super_flash_helper.h" #include "util.h" diff --git a/fastboot/fastboot_driver.h b/fastboot/fastboot_driver.h index 64e3c2b86..3d6c7b017 100644 --- a/fastboot/fastboot_driver.h +++ b/fastboot/fastboot_driver.h @@ -40,21 +40,13 @@ #include #include "constants.h" +#include "fastboot_driver_interface.h" #include "transport.h" class Transport; namespace fastboot { -enum RetCode : int { - SUCCESS = 0, - BAD_ARG, - IO_ERROR, - BAD_DEV_RESP, - DEVICE_FAIL, - TIMEOUT, -}; - struct DriverCallbacks { std::function prolog = [](const std::string&) {}; std::function epilog = [](int) {}; @@ -62,7 +54,7 @@ struct DriverCallbacks { std::function text = [](const std::string&) {}; }; -class FastBootDriver { +class FastBootDriver : public IFastBootDriver { friend class FastBootTest; public: @@ -77,9 +69,10 @@ class FastBootDriver { RetCode Boot(std::string* response = nullptr, std::vector* info = nullptr); RetCode Continue(std::string* response = nullptr, std::vector* info = nullptr); RetCode CreatePartition(const std::string& partition, const std::string& size); - RetCode DeletePartition(const std::string& partition); + RetCode DeletePartition(const std::string& partition) override; RetCode Download(const std::string& name, android::base::borrowed_fd fd, size_t size, - std::string* response = nullptr, std::vector* info = nullptr); + std::string* response = nullptr, + std::vector* info = nullptr) override; RetCode Download(android::base::borrowed_fd fd, size_t size, std::string* response = nullptr, std::vector* info = nullptr); RetCode Download(const std::string& name, const std::vector& buf, @@ -92,16 +85,17 @@ class FastBootDriver { RetCode Download(sparse_file* s, bool use_crc = false, std::string* response = nullptr, std::vector* info = nullptr); RetCode Erase(const std::string& partition, std::string* response = nullptr, - std::vector* info = nullptr); + std::vector* info = nullptr) override; RetCode Flash(const std::string& partition, std::string* response = nullptr, std::vector* info = nullptr); RetCode GetVar(const std::string& key, std::string* val, - std::vector* info = nullptr); + std::vector* info = nullptr) override; RetCode GetVarAll(std::vector* response); - RetCode Reboot(std::string* response = nullptr, std::vector* info = nullptr); + RetCode Reboot(std::string* response = nullptr, + std::vector* info = nullptr) override; RetCode RebootTo(std::string target, std::string* response = nullptr, - std::vector* info = nullptr); - RetCode ResizePartition(const std::string& partition, const std::string& size); + std::vector* info = nullptr) override; + RetCode ResizePartition(const std::string& partition, const std::string& size) override; RetCode SetActive(const std::string& slot, std::string* response = nullptr, std::vector* info = nullptr); RetCode Upload(const std::string& outfile, std::string* response = nullptr, @@ -115,7 +109,7 @@ class FastBootDriver { /* HIGHER LEVEL COMMANDS -- Composed of the commands above */ RetCode FlashPartition(const std::string& partition, const std::vector& data); RetCode FlashPartition(const std::string& partition, android::base::borrowed_fd fd, - uint32_t sz); + uint32_t sz) override; RetCode FlashPartition(const std::string& partition, sparse_file* s, uint32_t sz, size_t current, size_t total); @@ -127,7 +121,7 @@ class FastBootDriver { void SetInfoCallback(std::function info); static const std::string RCString(RetCode rc); std::string Error(); - RetCode WaitForDisconnect(); + RetCode WaitForDisconnect() override; // Note: set_transport will return the previous transport. Transport* set_transport(Transport* transport); diff --git a/fastboot/fastboot_driver_interface.h b/fastboot/fastboot_driver_interface.h new file mode 100644 index 000000000..795938fb4 --- /dev/null +++ b/fastboot/fastboot_driver_interface.h @@ -0,0 +1,59 @@ +// +// Copyright (C) 2023 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +#pragma once + +#include + +#include "android-base/unique_fd.h" + +class Transport; + +namespace fastboot { + +enum RetCode : int { + SUCCESS = 0, + BAD_ARG, + IO_ERROR, + BAD_DEV_RESP, + DEVICE_FAIL, + TIMEOUT, +}; + +class IFastBootDriver { + public: + RetCode virtual FlashPartition(const std::string& partition, android::base::borrowed_fd fd, + uint32_t sz) = 0; + RetCode virtual DeletePartition(const std::string& partition) = 0; + RetCode virtual WaitForDisconnect() = 0; + RetCode virtual Reboot(std::string* response = nullptr, + std::vector* info = nullptr) = 0; + + RetCode virtual RebootTo(std::string target, std::string* response = nullptr, + std::vector* info = nullptr) = 0; + RetCode virtual GetVar(const std::string& key, std::string* val, + std::vector* info = nullptr) = 0; + RetCode virtual Download(const std::string& name, android::base::borrowed_fd fd, size_t size, + std::string* response = nullptr, + std::vector* info = nullptr) = 0; + RetCode virtual RawCommand(const std::string& cmd, const std::string& message, + std::string* response = nullptr, + std::vector* info = nullptr, int* dsize = nullptr) = 0; + RetCode virtual ResizePartition(const std::string& partition, const std::string& size) = 0; + RetCode virtual Erase(const std::string& partition, std::string* response = nullptr, + std::vector* info = nullptr) = 0; + virtual ~IFastBootDriver() = default; +}; +} // namespace fastboot \ No newline at end of file diff --git a/fastboot/fastboot_driver_mock.h b/fastboot/fastboot_driver_mock.h new file mode 100644 index 000000000..62a570849 --- /dev/null +++ b/fastboot/fastboot_driver_mock.h @@ -0,0 +1,51 @@ +// +// Copyright (C) 2023 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// +#pragma once + +#include +#include "fastboot_driver_interface.h" + +namespace fastboot { + +class MockFastbootDriver : public IFastBootDriver { + public: + MOCK_METHOD(RetCode, FlashPartition, + (const std::string& partition, android::base::borrowed_fd fd, uint32_t sz), + (override)); + MOCK_METHOD(RetCode, DeletePartition, (const std::string&), (override)); + MOCK_METHOD(RetCode, Reboot, (std::string*, std::vector*), (override)); + MOCK_METHOD(RetCode, RebootTo, (std::string, std::string*, std::vector*), + (override)); + + MOCK_METHOD(RetCode, GetVar, (const std::string&, std::string*, std::vector*), + (override)); + + MOCK_METHOD(RetCode, Download, + (const std::string&, android::base::borrowed_fd, size_t, std::string*, + std::vector*), + (override)); + + MOCK_METHOD(RetCode, RawCommand, + (const std::string&, const std::string&, std::string*, std::vector*, + int*), + (override)); + MOCK_METHOD(RetCode, ResizePartition, (const std::string&, const std::string&), (override)); + MOCK_METHOD(RetCode, Erase, (const std::string&, std::string*, std::vector*), + (override)); + MOCK_METHOD(RetCode, WaitForDisconnect, (), (override)); +}; + +} // namespace fastboot \ No newline at end of file From bf09c61d9502057d8bc25317c20537557ad9501c Mon Sep 17 00:00:00 2001 From: Daniel Zheng Date: Tue, 11 Apr 2023 09:33:47 -0700 Subject: [PATCH 07/16] Pointed FlashingPlan to Interface Using interface for testing purposes Bug: 194686221 Test: tested compilation Change-Id: Ia9465c1f5673c249cdbc08a1a2432e8a603bd9cc --- fastboot/fastboot.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fastboot/fastboot.h b/fastboot/fastboot.h index 2d79e42a0..009b3a11e 100644 --- a/fastboot/fastboot.h +++ b/fastboot/fastboot.h @@ -99,7 +99,7 @@ struct FlashingPlan { std::string current_slot; std::string secondary_slot; - fastboot::FastBootDriver* fb; + fastboot::IFastBootDriver* fb; }; class FlashAllTool { From efca333730842f0abd7f08dcac81ae9e95ee19f6 Mon Sep 17 00:00:00 2001 From: Daniel Zheng Date: Thu, 30 Mar 2023 15:39:14 -0700 Subject: [PATCH 08/16] Added forward declaration for compilation Test: tested compilation Change-Id: I7165e323574ef8f3f90d1540c4a2b6a991803b40 --- fastboot/fastboot.h | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/fastboot/fastboot.h b/fastboot/fastboot.h index 009b3a11e..b3dc67dc2 100644 --- a/fastboot/fastboot.h +++ b/fastboot/fastboot.h @@ -32,6 +32,7 @@ #include "fastboot_driver_interface.h" #include "filesystem.h" #include "super_flash_helper.h" +#include "task.h" #include "util.h" #include @@ -131,6 +132,19 @@ void reboot_to_userspace_fastboot(); void syntax_error(const char* fmt, ...); std::string get_current_slot(); +// Code for Parsing fastboot-info.txt +std::unique_ptr ParseFlashCommand(const FlashingPlan* fp, + const std::vector& parts); +std::unique_ptr ParseRebootCommand(const FlashingPlan* fp, + const std::vector& parts); +std::unique_ptr ParseWipeCommand(const FlashingPlan* fp, + const std::vector& parts); +std::unique_ptr ParseFastbootInfoLine(const FlashingPlan* fp, + const std::vector& command); +void AddResizeTasks(const FlashingPlan* fp, std::vector>& tasks); +std::vector> ParseFastbootInfo(const FlashingPlan* fp, + const std::vector& file); + struct NetworkSerial { Socket::Protocol protocol; std::string address; From 76aa257850e511a49bde24588d3500d4cccf4d07 Mon Sep 17 00:00:00 2001 From: Daniel Zheng Date: Thu, 30 Mar 2023 15:48:23 -0700 Subject: [PATCH 09/16] Adding Test for Parsing Flash Task Test: fastboot_test Bug: 194686221 Change-Id: Ie15b2d0423366fb9d1debb9be52de3b6d3a891af --- fastboot/Android.bp | 1 + fastboot/task_test.cpp | 82 ++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 83 insertions(+) create mode 100644 fastboot/task_test.cpp diff --git a/fastboot/Android.bp b/fastboot/Android.bp index 3b786e875..7794c4b46 100644 --- a/fastboot/Android.bp +++ b/fastboot/Android.bp @@ -381,6 +381,7 @@ cc_test_host { "socket_mock.cpp", "socket_test.cpp", "super_flash_helper_test.cpp", + "task_test.cpp", "tcp_test.cpp", "udp_test.cpp", ], diff --git a/fastboot/task_test.cpp b/fastboot/task_test.cpp new file mode 100644 index 000000000..400e27f74 --- /dev/null +++ b/fastboot/task_test.cpp @@ -0,0 +1,82 @@ +// +// Copyright (C) 2023 The Android Open Source Project +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. +// + +#include "task.h" +#include "fastboot.h" + +#include +#include +#include +#include +#include +#include "android-base/strings.h" +using android::base::Split; + +class ParseTest : public ::testing ::Test { + protected: + void SetUp() override { + fp = std::make_unique(); + fp->slot_override = "b"; + fp->secondary_slot = "a"; + fp->wants_wipe = false; + } + void TearDown() override {} + + std::unique_ptr fp; + + private: +}; + +static std::vector> collectTasks(FlashingPlan* fp, + const std::vector& commands) { + std::vector> vec_commands; + for (auto& command : commands) { + vec_commands.emplace_back(android::base::Split(command, " ")); + } + std::vector> tasks; + for (auto& command : vec_commands) { + tasks.emplace_back(ParseFastbootInfoLine(fp, command)); + } + return tasks; +} + +TEST_F(ParseTest, CORRECT_FlASH_TASK_FORMED) { + std::vector commands = {"flash dtbo", "flash --slot-other system system_other.img", + "flash system", "flash --apply-vbmeta vbmeta"}; + + std::vector> tasks = collectTasks(fp.get(), commands); + + std::vector> expected_values{ + {"dtbo", "dtbo_b", "b", "dtbo.img"}, + {"system", "system_a", "a", "system_other.img"}, + {"system", "system_b", "b", "system.img"}, + {"vbmeta", "vbmeta_b", "b", "vbmeta.img"} + + }; + + for (auto& task : tasks) { + ASSERT_TRUE(task != nullptr); + } + + for (size_t i = 0; i < tasks.size(); i++) { + auto task = tasks[i]->AsFlashTask(); + ASSERT_TRUE(task != nullptr); + ASSERT_EQ(task->GetPartition(), expected_values[i][0]); + ASSERT_EQ(task->GetPartitionAndSlot(), expected_values[i][1]); + ASSERT_EQ(task->GetSlot(), expected_values[i][2]); + ASSERT_EQ(task->GetImageName(), expected_values[i][3]); + } +} From 68b84df9949983d8a0e55183749f1f6d2a0bb670 Mon Sep 17 00:00:00 2001 From: Daniel Zheng Date: Thu, 13 Apr 2023 11:39:26 -0700 Subject: [PATCH 10/16] Added test for version check Test: fastboot_test Bug: 194686221 Change-Id: I886c0a91be3b4f4bf7f2b8c76704aa30a352ee5b --- fastboot/fastboot.h | 1 + fastboot/task_test.cpp | 18 ++++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/fastboot/fastboot.h b/fastboot/fastboot.h index b3dc67dc2..d6afb9ed8 100644 --- a/fastboot/fastboot.h +++ b/fastboot/fastboot.h @@ -133,6 +133,7 @@ void syntax_error(const char* fmt, ...); std::string get_current_slot(); // Code for Parsing fastboot-info.txt +bool CheckFastbootInfoRequirements(const std::vector& command); std::unique_ptr ParseFlashCommand(const FlashingPlan* fp, const std::vector& parts); std::unique_ptr ParseRebootCommand(const FlashingPlan* fp, diff --git a/fastboot/task_test.cpp b/fastboot/task_test.cpp index 400e27f74..52d1fc391 100644 --- a/fastboot/task_test.cpp +++ b/fastboot/task_test.cpp @@ -80,3 +80,21 @@ TEST_F(ParseTest, CORRECT_FlASH_TASK_FORMED) { ASSERT_EQ(task->GetImageName(), expected_values[i][3]); } } + +TEST_F(ParseTest, VERSION_CHECK_CORRRECT) { + std::vector correct_versions = { + "version 1.0", + "version 22.00", + }; + + std::vector bad_versions = {"version", "version .01", "version x1", + "version 1.0.1", "version 1.", "s 1.0", + "version 1.0 2.0"}; + + for (auto& version : correct_versions) { + ASSERT_TRUE(CheckFastbootInfoRequirements(android::base::Split(version, " "))) << version; + } + for (auto& version : bad_versions) { + ASSERT_FALSE(CheckFastbootInfoRequirements(android::base::Split(version, " "))) << version; + } +} \ No newline at end of file From 8a20643f7ffe9e175db11455accb8a0f49fbdb21 Mon Sep 17 00:00:00 2001 From: Bart Van Assche Date: Wed, 19 Apr 2023 13:49:12 -0700 Subject: [PATCH 11/16] task_profiles_test: Skip this test if cgroups is read-only GKE provides an unusual environment: the cgroupv2 filesystem is mounted read-only. Skip the task_profiles_test on the host if the cgroup2 filesystem is mounted read-only to prevent that a test fails as follows: Failed to write '-1' to /sys/fs/cgroup/cgroup.procs: Read-only file system. Bug: 278899193 Change-Id: I8c5a0c0848a47a395ae87f2fc31ba0ccda7d7f31 Signed-off-by: Bart Van Assche --- libprocessgroup/task_profiles_test.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/libprocessgroup/task_profiles_test.cpp b/libprocessgroup/task_profiles_test.cpp index 842189d43..6a5b48bf3 100644 --- a/libprocessgroup/task_profiles_test.cpp +++ b/libprocessgroup/task_profiles_test.cpp @@ -16,6 +16,7 @@ #include "task_profiles.h" #include +#include #include #include #include @@ -29,13 +30,14 @@ using ::android::base::LogFunction; using ::android::base::LogId; using ::android::base::LogSeverity; using ::android::base::SetLogger; +using ::android::base::Split; using ::android::base::VERBOSE; using ::testing::TestWithParam; using ::testing::Values; namespace { -bool IsCgroupV2Mounted() { +bool IsCgroupV2MountedRw() { std::unique_ptr mnts(setmntent("/proc/mounts", "re"), endmntent); if (!mnts) { LOG(ERROR) << "Failed to open /proc/mounts"; @@ -43,9 +45,11 @@ bool IsCgroupV2Mounted() { } struct mntent* mnt; while ((mnt = getmntent(mnts.get()))) { - if (strcmp(mnt->mnt_type, "cgroup2") == 0) { - return true; + if (strcmp(mnt->mnt_type, "cgroup2") != 0) { + continue; } + const std::vector options = Split(mnt->mnt_opts, ","); + return std::count(options.begin(), options.end(), "ro") == 0; } return false; } @@ -145,8 +149,9 @@ class SetAttributeFixture : public TestWithParam { }; TEST_P(SetAttributeFixture, SetAttribute) { - // Treehugger runs host tests inside a container without cgroupv2 support. - if (!IsCgroupV2Mounted()) { + // Treehugger runs host tests inside a container either without cgroupv2 + // support or with the cgroup filesystem mounted read-only. + if (!IsCgroupV2MountedRw()) { GTEST_SKIP(); return; } From 935ee1f9718438763a075398897c8ee7c1905bf8 Mon Sep 17 00:00:00 2001 From: Daniel Zheng Date: Thu, 13 Apr 2023 11:03:34 -0700 Subject: [PATCH 12/16] Added test for parsing bad input Test: fastboot_test Change-Id: I04fc24741987bc88a490b2084189dfd8f3714582 --- fastboot/task_test.cpp | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/fastboot/task_test.cpp b/fastboot/task_test.cpp index 52d1fc391..145b4d77d 100644 --- a/fastboot/task_test.cpp +++ b/fastboot/task_test.cpp @@ -53,6 +53,11 @@ static std::vector> collectTasks(FlashingPlan* fp, return tasks; } +std::unique_ptr ParseCommand(FlashingPlan* fp, std::string command) { + std::vector vec_command = android::base::Split(command, " "); + return ParseFastbootInfoLine(fp, vec_command); +} + TEST_F(ParseTest, CORRECT_FlASH_TASK_FORMED) { std::vector commands = {"flash dtbo", "flash --slot-other system system_other.img", "flash system", "flash --apply-vbmeta vbmeta"}; @@ -97,4 +102,22 @@ TEST_F(ParseTest, VERSION_CHECK_CORRRECT) { for (auto& version : bad_versions) { ASSERT_FALSE(CheckFastbootInfoRequirements(android::base::Split(version, " "))) << version; } -} \ No newline at end of file +} + +TEST_F(ParseTest, BAD_FASTBOOT_INFO_INPUT) { + ASSERT_EQ(ParseCommand(fp.get(), "flash"), nullptr); + ASSERT_EQ(ParseCommand(fp.get(), "flash --slot-other --apply-vbmeta"), nullptr); + ASSERT_EQ(ParseCommand(fp.get(), "flash --apply-vbmeta"), nullptr); + ASSERT_EQ(ParseCommand(fp.get(), "if-wipe"), nullptr); + ASSERT_EQ(ParseCommand(fp.get(), "if-wipe flash"), nullptr); + ASSERT_EQ(ParseCommand(fp.get(), "wipe dtbo"), nullptr); + ASSERT_EQ(ParseCommand(fp.get(), "update-super dtbo"), nullptr); + ASSERT_EQ(ParseCommand(fp.get(), "flash system system.img system"), nullptr); + ASSERT_EQ(ParseCommand(fp.get(), "reboot bootloader fastboot"), nullptr); + ASSERT_EQ(ParseCommand(fp.get(), + "flash --slot-other --apply-vbmeta system system_other.img system"), + nullptr); + ASSERT_EQ(ParseCommand(fp.get(), "erase"), nullptr); + ASSERT_EQ(ParseCommand(fp.get(), "erase dtbo dtbo"), nullptr); + ASSERT_EQ(ParseCommand(fp.get(), "wipe this"), nullptr); +} From 3a81dda861c85b15b1ea614b6c15b2274105d113 Mon Sep 17 00:00:00 2001 From: Christopher Ferris Date: Mon, 24 Apr 2023 18:14:53 -0700 Subject: [PATCH 13/16] Re-add code to skip gettings logs on logd crashes. Also add new unit tests to verify this behavior. Bug: 276934420 Test: New unit tests pass. Test: Ran new unit tests without pthread_setname_np call and verified Test: the tests fail. Test: Force crash logd and verify log messages are not gathered. Test: Force crash a logd thread and verify log messages are not gathered. Change-Id: If8effef68f629432923cdc89e57d28ef5b8b4ce2 Merged-In: If8effef68f629432923cdc89e57d28ef5b8b4ce2 (cherry picked from commit bda106416096d51d872bffacfd251e586f982004) --- debuggerd/debuggerd_test.cpp | 56 ++++++++++++++++++++++ debuggerd/libdebuggerd/tombstone_proto.cpp | 9 +++- 2 files changed, 64 insertions(+), 1 deletion(-) diff --git a/debuggerd/debuggerd_test.cpp b/debuggerd/debuggerd_test.cpp index a00a2026f..39d7fff1c 100644 --- a/debuggerd/debuggerd_test.cpp +++ b/debuggerd/debuggerd_test.cpp @@ -20,6 +20,7 @@ #include #include #include +#include #include #include #include @@ -2703,3 +2704,58 @@ TEST_F(CrasherTest, verify_build_id) { } ASSERT_TRUE(found_valid_elf) << "Did not find any elf files with valid BuildIDs to check."; } + +const char kLogMessage[] = "Should not see this log message."; + +// Verify that the logd process does not read the log. +TEST_F(CrasherTest, logd_skips_reading_logs) { + StartProcess([]() { + pthread_setname_np(pthread_self(), "logd"); + LOG(INFO) << kLogMessage; + abort(); + }); + + unique_fd output_fd; + StartIntercept(&output_fd); + FinishCrasher(); + AssertDeath(SIGABRT); + int intercept_result; + FinishIntercept(&intercept_result); + ASSERT_EQ(1, intercept_result) << "tombstoned reported failure"; + + std::string result; + ConsumeFd(std::move(output_fd), &result); + // logd should not contain our log message. + ASSERT_NOT_MATCH(result, kLogMessage); +} + +// Verify that the logd process does not read the log when the non-main +// thread crashes. +TEST_F(CrasherTest, logd_skips_reading_logs_not_main_thread) { + StartProcess([]() { + pthread_setname_np(pthread_self(), "logd"); + LOG(INFO) << kLogMessage; + + std::thread thread([]() { + pthread_setname_np(pthread_self(), "not_logd_thread"); + // Raise the signal on the side thread. + raise_debugger_signal(kDebuggerdTombstone); + }); + thread.join(); + _exit(0); + }); + + unique_fd output_fd; + StartIntercept(&output_fd, kDebuggerdTombstone); + FinishCrasher(); + AssertDeath(0); + + int intercept_result; + FinishIntercept(&intercept_result); + ASSERT_EQ(1, intercept_result) << "tombstoned reported failure"; + + std::string result; + ConsumeFd(std::move(output_fd), &result); + ASSERT_BACKTRACE_FRAME(result, "raise_debugger_signal"); + ASSERT_NOT_MATCH(result, kLogMessage); +} diff --git a/debuggerd/libdebuggerd/tombstone_proto.cpp b/debuggerd/libdebuggerd/tombstone_proto.cpp index 9a565deb6..acd814efa 100644 --- a/debuggerd/libdebuggerd/tombstone_proto.cpp +++ b/debuggerd/libdebuggerd/tombstone_proto.cpp @@ -690,7 +690,14 @@ void engrave_tombstone_proto(Tombstone* tombstone, unwindstack::AndroidUnwinder* // Only dump logs on debuggable devices. if (android::base::GetBoolProperty("ro.debuggable", false)) { - dump_logcat(&result, main_thread.pid); + // Get the thread that corresponds to the main pid of the process. + const ThreadInfo& thread = threads.at(main_thread.pid); + + // Do not attempt to dump logs of the logd process because the gathering + // of logs can hang until a timeout occurs. + if (thread.thread_name != "logd") { + dump_logcat(&result, main_thread.pid); + } } dump_open_fds(&result, open_files); From 6727d5840b94d94a3ca9b9dca41a37cc3a5efe49 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Tue, 25 Apr 2023 16:46:21 -0700 Subject: [PATCH 14/16] libsnapshot: Fix test failures on certain configurations. Due to how CF is built and tested, VABC is enabled even when not supported by the kernel. To work around this add some logic in libsnapshot and the test harness to recognize this situation and silently flip off the VABC flag. This also fixes the -force_mode option to vts_libsnapshot_test, so that it will skip tests that aren't supported by the device. Bug: 264279496 Test: vts_libsnapshot_test on android12-gsi with 11-5.4 kernel Change-Id: I9279d8d400cac5cd504a7ae91f254aae57fa856d --- fs_mgr/libsnapshot/snapshot.cpp | 3 ++- fs_mgr/libsnapshot/snapshot_test.cpp | 34 ++++++++++++++++------------ fs_mgr/libsnapshot/utility.cpp | 7 ++++++ fs_mgr/libsnapshot/utility.h | 1 + 4 files changed, 29 insertions(+), 16 deletions(-) diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index 0e36da151..2773be79b 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -2784,7 +2784,8 @@ Return SnapshotManager::CreateUpdateSnapshots(const DeltaArchiveManifest& manife << " writer.GetCowVersion(): " << writer.GetCowVersion(); bool use_compression = IsCompressionEnabled() && dap_metadata.vabc_enabled() && - !device_->IsRecovery() && cow_format_support; + !device_->IsRecovery() && cow_format_support && + KernelSupportsCompressedSnapshots(); std::string compression_algorithm; if (use_compression) { diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp index 60186434a..57a28546f 100644 --- a/fs_mgr/libsnapshot/snapshot_test.cpp +++ b/fs_mgr/libsnapshot/snapshot_test.cpp @@ -125,6 +125,10 @@ class SnapshotTest : public ::testing::Test { sm->set_use_first_stage_snapuserd(false); } + bool DeviceSupportsCompression() { + return IsCompressionEnabled() && KernelSupportsCompressedSnapshots(); + } + void CleanupTestArtifacts() { // Normally cancelling inside a merge is not allowed. Since these // are tests, we don't care, destroy everything that might exist. @@ -327,7 +331,7 @@ class SnapshotTest : public ::testing::Test { DeltaArchiveManifest manifest; auto dynamic_partition_metadata = manifest.mutable_dynamic_partition_metadata(); - dynamic_partition_metadata->set_vabc_enabled(IsCompressionEnabled()); + dynamic_partition_metadata->set_vabc_enabled(DeviceSupportsCompression()); dynamic_partition_metadata->set_cow_version(android::snapshot::kCowVersionMajor); auto group = dynamic_partition_metadata->add_groups(); @@ -366,7 +370,7 @@ class SnapshotTest : public ::testing::Test { if (!res) { return res; } - } else if (!IsCompressionEnabled()) { + } else if (!DeviceSupportsCompression()) { std::string ignore; if (!MapUpdateSnapshot("test_partition_b", &ignore)) { return AssertionFailure() << "Failed to map test_partition_b"; @@ -425,7 +429,7 @@ TEST_F(SnapshotTest, CreateSnapshot) { ASSERT_TRUE(AcquireLock()); PartitionCowCreator cow_creator; - cow_creator.compression_enabled = IsCompressionEnabled(); + cow_creator.compression_enabled = DeviceSupportsCompression(); if (cow_creator.compression_enabled) { cow_creator.compression_algorithm = "gz"; } else { @@ -466,7 +470,7 @@ TEST_F(SnapshotTest, MapSnapshot) { ASSERT_TRUE(AcquireLock()); PartitionCowCreator cow_creator; - cow_creator.compression_enabled = IsCompressionEnabled(); + cow_creator.compression_enabled = DeviceSupportsCompression(); static const uint64_t kDeviceSize = 1024 * 1024; SnapshotStatus status; @@ -585,7 +589,7 @@ TEST_F(SnapshotTest, FirstStageMountAndMerge) { SnapshotStatus status; ASSERT_TRUE(init->ReadSnapshotStatus(lock_.get(), "test_partition_b", &status)); ASSERT_EQ(status.state(), SnapshotState::CREATED); - if (IsCompressionEnabled()) { + if (DeviceSupportsCompression()) { ASSERT_EQ(status.compression_algorithm(), "gz"); } else { ASSERT_EQ(status.compression_algorithm(), "none"); @@ -855,7 +859,7 @@ class SnapshotUpdateTest : public SnapshotTest { opener_ = std::make_unique(fake_super); auto dynamic_partition_metadata = manifest_.mutable_dynamic_partition_metadata(); - dynamic_partition_metadata->set_vabc_enabled(IsCompressionEnabled()); + dynamic_partition_metadata->set_vabc_enabled(DeviceSupportsCompression()); dynamic_partition_metadata->set_cow_version(android::snapshot::kCowVersionMajor); // Create a fake update package metadata. @@ -975,7 +979,7 @@ class SnapshotUpdateTest : public SnapshotTest { } AssertionResult MapOneUpdateSnapshot(const std::string& name) { - if (IsCompressionEnabled()) { + if (DeviceSupportsCompression()) { std::unique_ptr writer; return MapUpdateSnapshot(name, &writer); } else { @@ -985,7 +989,7 @@ class SnapshotUpdateTest : public SnapshotTest { } AssertionResult WriteSnapshotAndHash(const std::string& name) { - if (IsCompressionEnabled()) { + if (DeviceSupportsCompression()) { std::unique_ptr writer; auto res = MapUpdateSnapshot(name, &writer); if (!res) { @@ -1158,7 +1162,7 @@ TEST_F(SnapshotUpdateTest, FullUpdateFlow) { // Initiate the merge and wait for it to be completed. ASSERT_TRUE(init->InitiateMerge()); - ASSERT_EQ(init->IsSnapuserdRequired(), IsCompressionEnabled()); + ASSERT_EQ(init->IsSnapuserdRequired(), DeviceSupportsCompression()); { // We should have started in SECOND_PHASE since nothing shrinks. ASSERT_TRUE(AcquireLock()); @@ -1187,7 +1191,7 @@ TEST_F(SnapshotUpdateTest, FullUpdateFlow) { // Test that shrinking and growing partitions at the same time is handled // correctly in VABC. TEST_F(SnapshotUpdateTest, SpaceSwapUpdate) { - if (!IsCompressionEnabled()) { + if (!DeviceSupportsCompression()) { // b/179111359 GTEST_SKIP() << "Skipping Virtual A/B Compression test"; } @@ -1255,7 +1259,7 @@ TEST_F(SnapshotUpdateTest, SpaceSwapUpdate) { // Initiate the merge and wait for it to be completed. ASSERT_TRUE(init->InitiateMerge()); - ASSERT_EQ(init->IsSnapuserdRequired(), IsCompressionEnabled()); + ASSERT_EQ(init->IsSnapuserdRequired(), DeviceSupportsCompression()); { // Check that the merge phase is FIRST_PHASE until at least one call // to ProcessUpdateState() occurs. @@ -1879,8 +1883,8 @@ TEST_F(SnapshotUpdateTest, DataWipeWithStaleSnapshots) { ASSERT_TRUE(AcquireLock()); PartitionCowCreator cow_creator = { - .compression_enabled = IsCompressionEnabled(), - .compression_algorithm = IsCompressionEnabled() ? "gz" : "none", + .compression_enabled = DeviceSupportsCompression(), + .compression_algorithm = DeviceSupportsCompression() ? "gz" : "none", }; SnapshotStatus status; status.set_name("sys_a"); @@ -1974,7 +1978,7 @@ TEST_F(SnapshotUpdateTest, Hashtree) { // Test for overflow bit after update TEST_F(SnapshotUpdateTest, Overflow) { - if (IsCompressionEnabled()) { + if (DeviceSupportsCompression()) { GTEST_SKIP() << "No overflow bit set for userspace COWs"; } @@ -2040,7 +2044,7 @@ class AutoKill final { }; TEST_F(SnapshotUpdateTest, DaemonTransition) { - if (!IsCompressionEnabled()) { + if (!DeviceSupportsCompression()) { GTEST_SKIP() << "Skipping Virtual A/B Compression test"; } diff --git a/fs_mgr/libsnapshot/utility.cpp b/fs_mgr/libsnapshot/utility.cpp index 4a2af1c10..e46bffbde 100644 --- a/fs_mgr/libsnapshot/utility.cpp +++ b/fs_mgr/libsnapshot/utility.cpp @@ -25,7 +25,9 @@ #include #include #include +#include +using android::dm::DeviceMapper; using android::dm::kSectorSize; using android::fiemap::FiemapStatus; using android::fs_mgr::EnsurePathMounted; @@ -195,5 +197,10 @@ std::string GetOtherPartitionName(const std::string& name) { return name.substr(0, name.size() - suffix.size()) + other_suffix; } +bool KernelSupportsCompressedSnapshots() { + auto& dm = DeviceMapper::Instance(); + return dm.GetTargetByName("user", nullptr); +} + } // namespace snapshot } // namespace android diff --git a/fs_mgr/libsnapshot/utility.h b/fs_mgr/libsnapshot/utility.h index 671de9dbc..b0e565cfe 100644 --- a/fs_mgr/libsnapshot/utility.h +++ b/fs_mgr/libsnapshot/utility.h @@ -129,6 +129,7 @@ std::ostream& operator<<(std::ostream& os, const Now&); void AppendExtent(google::protobuf::RepeatedPtrField* extents, uint64_t start_block, uint64_t num_blocks); +bool KernelSupportsCompressedSnapshots(); bool IsCompressionEnabled(); // Swap the suffix of a partition name. From 4a8c1461ff2135e29bb00bcd32989567108e776a Mon Sep 17 00:00:00 2001 From: Suren Baghdasaryan Date: Wed, 12 Apr 2023 01:24:23 +0000 Subject: [PATCH 15/16] libprocessgroup: implement task profile validity checks Provide profile validity check functions for cases when user wants to check whether a profile can be successfully applied before actually applying it. Add test cases to cover new APIs. Also add a wrapper function for framework code to call it. Bug: 277233783 Test: atest task_profiles_test Test: manually verify freezer with outdated cgroup configuration Signed-off-by: Suren Baghdasaryan Signed-off-by: Li Li Change-Id: Iefb321dead27adbe67721972f164efea213c06cb Merged-In: Iefb321dead27adbe67721972f164efea213c06cb --- .../include/processgroup/processgroup.h | 4 + libprocessgroup/processgroup.cpp | 10 ++ libprocessgroup/task_profiles.cpp | 121 ++++++++++++++++++ libprocessgroup/task_profiles.h | 20 ++- libprocessgroup/task_profiles_test.cpp | 50 ++++++++ 5 files changed, 202 insertions(+), 3 deletions(-) diff --git a/libprocessgroup/include/processgroup/processgroup.h b/libprocessgroup/include/processgroup/processgroup.h index 48bc0b7f3..dbaeb9397 100644 --- a/libprocessgroup/include/processgroup/processgroup.h +++ b/libprocessgroup/include/processgroup/processgroup.h @@ -96,6 +96,10 @@ void removeAllEmptyProcessGroups(void); // Returns false in case of error, true in case of success bool getAttributePathForTask(const std::string& attr_name, int tid, std::string* path); +// Check if a profile can be applied without failing. +// Returns true if it can be applied without failing, false otherwise +bool isProfileValidForProcess(const std::string& profile_name, int uid, int pid); + #endif // __ANDROID_VNDK__ __END_DECLS diff --git a/libprocessgroup/processgroup.cpp b/libprocessgroup/processgroup.cpp index a02159401..234b793d4 100644 --- a/libprocessgroup/processgroup.cpp +++ b/libprocessgroup/processgroup.cpp @@ -657,3 +657,13 @@ bool setProcessGroupLimit(uid_t, int pid, int64_t limit_in_bytes) { bool getAttributePathForTask(const std::string& attr_name, int tid, std::string* path) { return CgroupGetAttributePathForTask(attr_name, tid, path); } + +bool isProfileValidForProcess(const std::string& profile_name, int uid, int pid) { + const TaskProfile* tp = TaskProfiles::GetInstance().GetProfile(profile_name); + + if (tp == nullptr) { + return false; + } + + return tp->IsValidForProcess(uid, pid); +} \ No newline at end of file diff --git a/libprocessgroup/task_profiles.cpp b/libprocessgroup/task_profiles.cpp index 17318289f..44dba2a16 100644 --- a/libprocessgroup/task_profiles.cpp +++ b/libprocessgroup/task_profiles.cpp @@ -259,6 +259,31 @@ bool SetAttributeAction::ExecuteForUID(uid_t uid) const { return true; } +bool SetAttributeAction::IsValidForProcess(uid_t, pid_t pid) const { + return IsValidForTask(pid); +} + +bool SetAttributeAction::IsValidForTask(int tid) const { + std::string path; + + if (!attribute_->GetPathForTask(tid, &path)) { + return false; + } + + if (!access(path.c_str(), W_OK)) { + // operation will succeed + return true; + } + + if (!access(path.c_str(), F_OK)) { + // file exists but not writable + return false; + } + + // file does not exist, ignore if optional + return optional_; +} + SetCgroupAction::SetCgroupAction(const CgroupController& c, const std::string& p) : controller_(c), path_(p) { FdCacheHelper::Init(controller_.GetTasksFilePath(path_), fd_[ProfileAction::RCT_TASK]); @@ -396,6 +421,39 @@ void SetCgroupAction::DropResourceCaching(ResourceCacheType cache_type) { FdCacheHelper::Drop(fd_[cache_type]); } +bool SetCgroupAction::IsValidForProcess(uid_t uid, pid_t pid) const { + std::lock_guard lock(fd_mutex_); + if (FdCacheHelper::IsCached(fd_[ProfileAction::RCT_PROCESS])) { + return true; + } + + if (fd_[ProfileAction::RCT_PROCESS] == FdCacheHelper::FDS_INACCESSIBLE) { + return false; + } + + std::string procs_path = controller()->GetProcsFilePath(path_, uid, pid); + return access(procs_path.c_str(), W_OK) == 0; +} + +bool SetCgroupAction::IsValidForTask(int) const { + std::lock_guard lock(fd_mutex_); + if (FdCacheHelper::IsCached(fd_[ProfileAction::RCT_TASK])) { + return true; + } + + if (fd_[ProfileAction::RCT_TASK] == FdCacheHelper::FDS_INACCESSIBLE) { + return false; + } + + if (fd_[ProfileAction::RCT_TASK] == FdCacheHelper::FDS_APP_DEPENDENT) { + // application-dependent path can't be used with tid + return false; + } + + std::string tasks_path = controller()->GetTasksFilePath(path_); + return access(tasks_path.c_str(), W_OK) == 0; +} + WriteFileAction::WriteFileAction(const std::string& task_path, const std::string& proc_path, const std::string& value, bool logfailures) : task_path_(task_path), proc_path_(proc_path), value_(value), logfailures_(logfailures) { @@ -532,6 +590,37 @@ void WriteFileAction::DropResourceCaching(ResourceCacheType cache_type) { FdCacheHelper::Drop(fd_[cache_type]); } +bool WriteFileAction::IsValidForProcess(uid_t, pid_t) const { + std::lock_guard lock(fd_mutex_); + if (FdCacheHelper::IsCached(fd_[ProfileAction::RCT_PROCESS])) { + return true; + } + + if (fd_[ProfileAction::RCT_PROCESS] == FdCacheHelper::FDS_INACCESSIBLE) { + return false; + } + + return access(proc_path_.empty() ? task_path_.c_str() : proc_path_.c_str(), W_OK) == 0; +} + +bool WriteFileAction::IsValidForTask(int) const { + std::lock_guard lock(fd_mutex_); + if (FdCacheHelper::IsCached(fd_[ProfileAction::RCT_TASK])) { + return true; + } + + if (fd_[ProfileAction::RCT_TASK] == FdCacheHelper::FDS_INACCESSIBLE) { + return false; + } + + if (fd_[ProfileAction::RCT_TASK] == FdCacheHelper::FDS_APP_DEPENDENT) { + // application-dependent path can't be used with tid + return false; + } + + return access(task_path_.c_str(), W_OK) == 0; +} + bool ApplyProfileAction::ExecuteForProcess(uid_t uid, pid_t pid) const { for (const auto& profile : profiles_) { profile->ExecuteForProcess(uid, pid); @@ -558,6 +647,24 @@ void ApplyProfileAction::DropResourceCaching(ResourceCacheType cache_type) { } } +bool ApplyProfileAction::IsValidForProcess(uid_t uid, pid_t pid) const { + for (const auto& profile : profiles_) { + if (!profile->IsValidForProcess(uid, pid)) { + return false; + } + } + return true; +} + +bool ApplyProfileAction::IsValidForTask(int tid) const { + for (const auto& profile : profiles_) { + if (!profile->IsValidForTask(tid)) { + return false; + } + } + return true; +} + void TaskProfile::MoveTo(TaskProfile* profile) { profile->elements_ = std::move(elements_); profile->res_cached_ = res_cached_; @@ -620,6 +727,20 @@ void TaskProfile::DropResourceCaching(ProfileAction::ResourceCacheType cache_typ res_cached_ = false; } +bool TaskProfile::IsValidForProcess(uid_t uid, pid_t pid) const { + for (const auto& element : elements_) { + if (!element->IsValidForProcess(uid, pid)) return false; + } + return true; +} + +bool TaskProfile::IsValidForTask(int tid) const { + for (const auto& element : elements_) { + if (!element->IsValidForTask(tid)) return false; + } + return true; +} + void TaskProfiles::DropResourceCaching(ProfileAction::ResourceCacheType cache_type) const { for (auto& iter : profiles_) { iter.second->DropResourceCaching(cache_type); diff --git a/libprocessgroup/task_profiles.h b/libprocessgroup/task_profiles.h index a8ecb873d..a62c5b0a9 100644 --- a/libprocessgroup/task_profiles.h +++ b/libprocessgroup/task_profiles.h @@ -72,12 +72,14 @@ class ProfileAction { virtual const char* Name() const = 0; // Default implementations will fail - virtual bool ExecuteForProcess(uid_t, pid_t) const { return false; }; - virtual bool ExecuteForTask(int) const { return false; }; - virtual bool ExecuteForUID(uid_t) const { return false; }; + virtual bool ExecuteForProcess(uid_t, pid_t) const { return false; } + virtual bool ExecuteForTask(int) const { return false; } + virtual bool ExecuteForUID(uid_t) const { return false; } virtual void EnableResourceCaching(ResourceCacheType) {} virtual void DropResourceCaching(ResourceCacheType) {} + virtual bool IsValidForProcess(uid_t uid, pid_t pid) const { return false; } + virtual bool IsValidForTask(int tid) const { return false; } protected: enum CacheUseResult { SUCCESS, FAIL, UNUSED }; @@ -103,6 +105,8 @@ class SetTimerSlackAction : public ProfileAction { const char* Name() const override { return "SetTimerSlack"; } bool ExecuteForTask(int tid) const override; + bool IsValidForProcess(uid_t uid, pid_t pid) const override { return true; } + bool IsValidForTask(int tid) const override { return true; } private: unsigned long slack_; @@ -120,6 +124,8 @@ class SetAttributeAction : public ProfileAction { bool ExecuteForProcess(uid_t uid, pid_t pid) const override; bool ExecuteForTask(int tid) const override; bool ExecuteForUID(uid_t uid) const override; + bool IsValidForProcess(uid_t uid, pid_t pid) const override; + bool IsValidForTask(int tid) const override; private: const IProfileAttribute* attribute_; @@ -137,6 +143,8 @@ class SetCgroupAction : public ProfileAction { bool ExecuteForTask(int tid) const override; void EnableResourceCaching(ResourceCacheType cache_type) override; void DropResourceCaching(ResourceCacheType cache_type) override; + bool IsValidForProcess(uid_t uid, pid_t pid) const override; + bool IsValidForTask(int tid) const override; const CgroupController* controller() const { return &controller_; } @@ -161,6 +169,8 @@ class WriteFileAction : public ProfileAction { bool ExecuteForTask(int tid) const override; void EnableResourceCaching(ResourceCacheType cache_type) override; void DropResourceCaching(ResourceCacheType cache_type) override; + bool IsValidForProcess(uid_t uid, pid_t pid) const override; + bool IsValidForTask(int tid) const override; private: std::string task_path_, proc_path_, value_; @@ -186,6 +196,8 @@ class TaskProfile { bool ExecuteForUID(uid_t uid) const; void EnableResourceCaching(ProfileAction::ResourceCacheType cache_type); void DropResourceCaching(ProfileAction::ResourceCacheType cache_type); + bool IsValidForProcess(uid_t uid, pid_t pid) const; + bool IsValidForTask(int tid) const; private: const std::string name_; @@ -204,6 +216,8 @@ class ApplyProfileAction : public ProfileAction { bool ExecuteForTask(int tid) const override; void EnableResourceCaching(ProfileAction::ResourceCacheType cache_type) override; void DropResourceCaching(ProfileAction::ResourceCacheType cache_type) override; + bool IsValidForProcess(uid_t uid, pid_t pid) const override; + bool IsValidForTask(int tid) const override; private: std::vector> profiles_; diff --git a/libprocessgroup/task_profiles_test.cpp b/libprocessgroup/task_profiles_test.cpp index 6a5b48bf3..eadbe7697 100644 --- a/libprocessgroup/task_profiles_test.cpp +++ b/libprocessgroup/task_profiles_test.cpp @@ -175,6 +175,32 @@ TEST_P(SetAttributeFixture, SetAttribute) { } } +class TaskProfileFixture : public TestWithParam { + public: + ~TaskProfileFixture() = default; +}; + +TEST_P(TaskProfileFixture, TaskProfile) { + // Treehugger runs host tests inside a container without cgroupv2 support. + if (!IsCgroupV2MountedRw()) { + GTEST_SKIP(); + return; + } + const TestParam params = GetParam(); + ProfileAttributeMock pa(params.attr_name); + // Test simple profile with one action + std::shared_ptr tp = std::make_shared("test_profile"); + tp->Add(std::make_unique(&pa, params.attr_value, params.optional_attr)); + EXPECT_EQ(tp->IsValidForProcess(getuid(), getpid()), params.result); + EXPECT_EQ(tp->IsValidForTask(getpid()), params.result); + // Test aggregate profile + TaskProfile tp2("meta_profile"); + std::vector> profiles = {tp}; + tp2.Add(std::make_unique(profiles)); + EXPECT_EQ(tp2.IsValidForProcess(getuid(), getpid()), params.result); + EXPECT_EQ(tp2.IsValidForTask(getpid()), params.result); +} + // Test the four combinations of optional_attr {false, true} and cgroup attribute { does not exist, // exists }. INSTANTIATE_TEST_SUITE_P( @@ -215,4 +241,28 @@ INSTANTIATE_TEST_SUITE_P( .log_prefix = "Failed to write", .log_suffix = geteuid() == 0 ? "Invalid argument" : "Permission denied"})); +// Test TaskProfile IsValid calls. +INSTANTIATE_TEST_SUITE_P( + TaskProfileTestSuite, TaskProfileFixture, + Values( + // Test operating on non-existing cgroup attribute fails. + TestParam{.attr_name = "no-such-attribute", + .attr_value = ".", + .optional_attr = false, + .result = false}, + // Test operating on optional non-existing cgroup attribute succeeds. + TestParam{.attr_name = "no-such-attribute", + .attr_value = ".", + .optional_attr = true, + .result = true}, + // Test operating on existing cgroup attribute succeeds. + TestParam{.attr_name = "cgroup.procs", + .attr_value = ".", + .optional_attr = false, + .result = true}, + // Test operating on optional existing cgroup attribute succeeds. + TestParam{.attr_name = "cgroup.procs", + .attr_value = ".", + .optional_attr = true, + .result = true})); } // namespace From e80a6b6dd403ef0cabb35dfefa798a6e72d775d3 Mon Sep 17 00:00:00 2001 From: Vincent Donnefort Date: Fri, 28 Apr 2023 09:30:23 +0100 Subject: [PATCH 16/16] ramdisk_node_list: Add urandom node Bionic requires random numbers to init the shadow call stack. Those numbers are obtained via the syscall getrandom (non-blocking) and will fallback to /dev/urandom if the former fails. When loading pKVM modules, we are so early in the boot process that the only source of entropy for the linux RNG are the architecture random number generators... which might be available on some platforms. Without any source of entropy, the only way of generating a random number is to try to generate some, which is what the bionic fallback expects via urandom. As a consequence, add the urandom node to the initramfs. Bug: 274876849 Merged-In: I111e2db53fabd63d070b8e9ab9c52faebf484ab3 Change-Id: I34a0e3f7c72de7344512366d4a96183b445edc2e --- rootdir/ramdisk_node_list | 1 + 1 file changed, 1 insertion(+) diff --git a/rootdir/ramdisk_node_list b/rootdir/ramdisk_node_list index d3ab8a66e..4f45faaec 100644 --- a/rootdir/ramdisk_node_list +++ b/rootdir/ramdisk_node_list @@ -1,3 +1,4 @@ dir dev 0755 0 0 nod dev/null 0600 0 0 c 1 3 nod dev/console 0600 0 0 c 5 1 +nod dev/urandom 0600 0 0 c 1 9