From 3ee2b9db5a5c9977c6bd726d0e5d12f5ac07d5cf Mon Sep 17 00:00:00 2001 From: Tianjie Xu Date: Mon, 27 Mar 2017 14:12:26 -0700 Subject: Log temperature during OTA update Log the maximum temperature as well as the start/end temperature of an update to last_install. Check the temperature at the end of each block_image_update(verify). To get the maximum temp, we iterate through /sys/class/thermal/thermal_zone*/temp and find the maximum value present. Bug: 32518487 Test: temperature logs in last_install Change-Id: Iaf22a9fbc5b18611bbc5320ffea995417872e514 --- install.cpp | 52 +++++++++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 47 insertions(+), 5 deletions(-) (limited to 'install.cpp') diff --git a/install.cpp b/install.cpp index 7cef44a37..0a2fa3ca4 100644 --- a/install.cpp +++ b/install.cpp @@ -26,11 +26,15 @@ #include #include +#include #include +#include #include #include #include +#include #include +#include #include #include @@ -46,10 +50,13 @@ #include "error_code.h" #include "minui/minui.h" #include "otautil/SysUtil.h" +#include "otautil/ThermalUtil.h" #include "roots.h" #include "ui.h" #include "verifier.h" +using namespace std::chrono_literals; + #define ASSUMED_UPDATE_BINARY_NAME "META-INF/com/google/android/update-binary" static constexpr const char* AB_OTA_PAYLOAD_PROPERTIES = "payload_properties.txt"; static constexpr const char* AB_OTA_PAYLOAD = "payload.bin"; @@ -63,6 +70,8 @@ static constexpr float VERIFICATION_PROGRESS_FRACTION = 0.25; static constexpr float DEFAULT_FILES_PROGRESS_FRACTION = 0.4; static constexpr float DEFAULT_IMAGE_PROGRESS_FRACTION = 0.1; +static std::condition_variable finish_log_temperature; + // This function parses and returns the build.version.incremental static int parse_build_number(const std::string& str) { size_t pos = str.find('='); @@ -299,9 +308,19 @@ update_binary_command(const char* path, ZipArchiveHandle zip, int retry_count, } #endif // !AB_OTA_UPDATER +static void log_max_temperature(int* max_temperature) { + CHECK(max_temperature != nullptr); + std::mutex mtx; + std::unique_lock lck(mtx); + while (finish_log_temperature.wait_for(lck, 20s) == std::cv_status::timeout) { + *max_temperature = std::max(*max_temperature, GetMaxValueFromThermalZone()); + } +} + // If the package contains an update binary, extract it and run it. static int try_update_binary(const char* path, ZipArchiveHandle zip, bool* wipe_cache, - std::vector& log_buffer, int retry_count) { + std::vector& log_buffer, int retry_count, + int* max_temperature) { read_source_target_build(zip, log_buffer); int pipefd[2]; @@ -392,6 +411,8 @@ static int try_update_binary(const char* path, ZipArchiveHandle zip, bool* wipe_ } close(pipefd[1]); + std::thread temperature_logger(log_max_temperature, max_temperature); + *wipe_cache = false; bool retry_update = false; @@ -453,6 +474,10 @@ static int try_update_binary(const char* path, ZipArchiveHandle zip, bool* wipe_ int status; waitpid(pid, &status, 0); + + finish_log_temperature.notify_one(); + temperature_logger.join(); + if (retry_update) { return INSTALL_RETRY; } @@ -466,7 +491,7 @@ static int try_update_binary(const char* path, ZipArchiveHandle zip, bool* wipe_ static int really_install_package(const char *path, bool* wipe_cache, bool needs_mount, - std::vector& log_buffer, int retry_count) + std::vector& log_buffer, int retry_count, int* max_temperature) { ui->SetBackground(RecoveryUI::INSTALLING_UPDATE); ui->Print("Finding update package...\n"); @@ -517,7 +542,7 @@ really_install_package(const char *path, bool* wipe_cache, bool needs_mount, ui->Print("Retry attempt: %d\n", retry_count); } ui->SetEnableReboot(false); - int result = try_update_binary(path, zip, wipe_cache, log_buffer, retry_count); + int result = try_update_binary(path, zip, wipe_cache, log_buffer, retry_count, max_temperature); ui->SetEnableReboot(true); ui->Print("\n"); @@ -533,13 +558,17 @@ install_package(const char* path, bool* wipe_cache, const char* install_file, modified_flash = true; auto start = std::chrono::system_clock::now(); + int start_temperature = GetMaxValueFromThermalZone(); + int max_temperature = start_temperature; + int result; std::vector log_buffer; if (setup_install_mounts() != 0) { LOG(ERROR) << "failed to set up expected mounts for install; aborting"; result = INSTALL_ERROR; } else { - result = really_install_package(path, wipe_cache, needs_mount, log_buffer, retry_count); + result = really_install_package(path, wipe_cache, needs_mount, log_buffer, retry_count, + &max_temperature); } // Measure the time spent to apply OTA update in seconds. @@ -570,8 +599,21 @@ install_package(const char* path, bool* wipe_cache, const char* install_file, "time_total: " + std::to_string(time_total), "retry: " + std::to_string(retry_count), }; + + int end_temperature = GetMaxValueFromThermalZone(); + max_temperature = std::max(end_temperature, max_temperature); + if (start_temperature > 0) { + log_buffer.push_back("temperature_start: " + std::to_string(start_temperature)); + } + if (end_temperature > 0) { + log_buffer.push_back("temperature_end: " + std::to_string(end_temperature)); + } + if (max_temperature > 0) { + log_buffer.push_back("temperature_max: " + std::to_string(max_temperature)); + } + std::string log_content = android::base::Join(log_header, "\n") + "\n" + - android::base::Join(log_buffer, "\n"); + android::base::Join(log_buffer, "\n") + "\n"; if (!android::base::WriteStringToFile(log_content, install_file)) { PLOG(ERROR) << "failed to write " << install_file; } -- cgit v1.2.3 From 1d866050eba7614109a1edec42529d4d80b0998f Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Mon, 10 Apr 2017 16:55:57 -0700 Subject: Verify the package compatibility with libvintf. verify_package_compatibility() is added to parse the compatibility entry (compatibility.zip) in a given OTA package. If entry is present, the information is sent to libvintf to check the compatibility. This CL doesn't actually call libvintf, since the API there is not available yet. Bug: 36597505 Test: Doesn't break the install with existing packages (i.e. w/o the compatibility entry). Test: recovery_component_test Change-Id: I3903ffa5f6ba33a5c0d761602ade6290c6752596 (cherry picked from commit 62e0bc7586077b3bde82759fb34b51b982cea20f) --- install.cpp | 73 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) (limited to 'install.cpp') diff --git a/install.cpp b/install.cpp index 0a2fa3ca4..b4b869b98 100644 --- a/install.cpp +++ b/install.cpp @@ -489,6 +489,70 @@ static int try_update_binary(const char* path, ZipArchiveHandle zip, bool* wipe_ return INSTALL_SUCCESS; } +// Verifes the compatibility info in a Treble-compatible package. Returns true directly if the +// entry doesn't exist. Note that the compatibility info is packed in a zip file inside the OTA +// package. +bool verify_package_compatibility(ZipArchiveHandle package_zip) { + LOG(INFO) << "Verifying package compatibility..."; + + static constexpr const char* COMPATIBILITY_ZIP_ENTRY = "compatibility.zip"; + ZipString compatibility_entry_name(COMPATIBILITY_ZIP_ENTRY); + ZipEntry compatibility_entry; + if (FindEntry(package_zip, compatibility_entry_name, &compatibility_entry) != 0) { + LOG(INFO) << "Package doesn't contain " << COMPATIBILITY_ZIP_ENTRY << " entry"; + return true; + } + + std::string zip_content(compatibility_entry.uncompressed_length, '\0'); + int32_t ret; + if ((ret = ExtractToMemory(package_zip, &compatibility_entry, + reinterpret_cast(&zip_content[0]), + compatibility_entry.uncompressed_length)) != 0) { + LOG(ERROR) << "Failed to read " << COMPATIBILITY_ZIP_ENTRY << ": " << ErrorCodeString(ret); + return false; + } + + ZipArchiveHandle zip_handle; + ret = OpenArchiveFromMemory(static_cast(const_cast(zip_content.data())), + zip_content.size(), COMPATIBILITY_ZIP_ENTRY, &zip_handle); + if (ret != 0) { + LOG(ERROR) << "Failed to OpenArchiveFromMemory: " << ErrorCodeString(ret); + return false; + } + + // Iterate all the entries inside COMPATIBILITY_ZIP_ENTRY and read the contents. + void* cookie; + ret = StartIteration(zip_handle, &cookie, nullptr, nullptr); + if (ret != 0) { + LOG(ERROR) << "Failed to start iterating zip entries: " << ErrorCodeString(ret); + CloseArchive(zip_handle); + return false; + } + std::unique_ptr guard(cookie, EndIteration); + + std::vector compatibility_info; + ZipEntry info_entry; + ZipString info_name; + while (Next(cookie, &info_entry, &info_name) == 0) { + std::string content(info_entry.uncompressed_length, '\0'); + int32_t ret = ExtractToMemory(zip_handle, &info_entry, reinterpret_cast(&content[0]), + info_entry.uncompressed_length); + if (ret != 0) { + LOG(ERROR) << "Failed to read " << info_name.name << ": " << ErrorCodeString(ret); + CloseArchive(zip_handle); + return false; + } + compatibility_info.emplace_back(std::move(content)); + } + EndIteration(cookie); + CloseArchive(zip_handle); + + // TODO(b/36814503): Enable the actual verification when VintfObject::CheckCompatibility() lands. + // VintfObject::CheckCompatibility returns zero on success. + // return (android::vintf::VintfObject::CheckCompatibility(compatibility_info, true) == 0); + return true; +} + static int really_install_package(const char *path, bool* wipe_cache, bool needs_mount, std::vector& log_buffer, int retry_count, int* max_temperature) @@ -536,6 +600,15 @@ really_install_package(const char *path, bool* wipe_cache, bool needs_mount, return INSTALL_CORRUPT; } + // Additionally verify the compatibility of the package. + if (!verify_package_compatibility(zip)) { + LOG(ERROR) << "Failed to verify package compatibility"; + log_buffer.push_back(android::base::StringPrintf("error: %d", kPackageCompatibilityFailure)); + sysReleaseMap(&map); + CloseArchive(zip); + return INSTALL_CORRUPT; + } + // Verify and install the contents of the package. ui->Print("Installing update...\n"); if (retry_count > 0) { -- cgit v1.2.3 From 99c549db74ea697136820cf4bdca8a486747f569 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Mon, 17 Apr 2017 10:44:00 -0700 Subject: Fix the double free in verify_package_compatibility(). """ void* cookie; std::unique_ptr guard(cookie, EndIteration); ... EndIteration(cookie); """ The above pattern is buggy that frees 'cookie' twice. Bug: 37413730 Test: Build new recovery and adb sideload a previously crashed package that contains 'compatibility.zip'. Change-Id: I183c33827fb28a438ebaedda446e84cabe7cb92d (cherry picked from commit f978278995d02a58e311fe017bdbb2c3702dd3bc) --- install.cpp | 1 - 1 file changed, 1 deletion(-) (limited to 'install.cpp') diff --git a/install.cpp b/install.cpp index b4b869b98..6dcd3565e 100644 --- a/install.cpp +++ b/install.cpp @@ -544,7 +544,6 @@ bool verify_package_compatibility(ZipArchiveHandle package_zip) { } compatibility_info.emplace_back(std::move(content)); } - EndIteration(cookie); CloseArchive(zip_handle); // TODO(b/36814503): Enable the actual verification when VintfObject::CheckCompatibility() lands. -- cgit v1.2.3 From bc4b1fe4c4305ebf0fbfc891b9b508c14b5c8ef8 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Mon, 17 Apr 2017 16:46:05 -0700 Subject: Add tests for update_binary_command(). Expose update_binary_command() through private/install.h for testing purpose. Also make minor clean-ups to install.cpp: a) adding more verbose logging on ExtractToMemory failures; b) update_binary_command() taking std::string instead of const char*; c) moving a few macro and global constants into update_binary_command(). Bug: 37300957 Test: recovery_component_test on marlin Test: Build new recovery and adb sideload on angler and sailfish. Change-Id: Ib2d9068af3fee038f01c90940ccaeb0a7da374fc --- install.cpp | 162 ++++++++++++++++++++++++++++++------------------------------ 1 file changed, 80 insertions(+), 82 deletions(-) (limited to 'install.cpp') diff --git a/install.cpp b/install.cpp index 6dcd3565e..4fb809996 100644 --- a/install.cpp +++ b/install.cpp @@ -57,9 +57,6 @@ using namespace std::chrono_literals; -#define ASSUMED_UPDATE_BINARY_NAME "META-INF/com/google/android/update-binary" -static constexpr const char* AB_OTA_PAYLOAD_PROPERTIES = "payload_properties.txt"; -static constexpr const char* AB_OTA_PAYLOAD = "payload.bin"; #define PUBLIC_KEYS_FILE "/res/keys" static constexpr const char* METADATA_PATH = "META-INF/com/android/metadata"; static constexpr const char* UNCRYPT_STATUS = "/cache/recovery/uncrypt_status"; @@ -136,13 +133,11 @@ static void read_source_target_build(ZipArchiveHandle zip, std::vector* cmd); +// Extract the update binary from the open zip archive |zip| located at |path| and store into |cmd| +// the command line that should be called. The |status_fd| is the file descriptor the child process +// should use to report back the progress of the update. +int update_binary_command(const std::string& path, ZipArchiveHandle zip, int retry_count, + int status_fd, std::vector* cmd); #ifdef AB_OTA_UPDATER @@ -224,87 +219,90 @@ static int check_newer_ab_build(ZipArchiveHandle zip) { return 0; } -static int -update_binary_command(const char* path, ZipArchiveHandle zip, int retry_count, - int status_fd, std::vector* cmd) -{ - int ret = check_newer_ab_build(zip); - if (ret) { - return ret; - } +int update_binary_command(const std::string& path, ZipArchiveHandle zip, int retry_count, + int status_fd, std::vector* cmd) { + CHECK(cmd != nullptr); + int ret = check_newer_ab_build(zip); + if (ret != 0) { + return ret; + } - // For A/B updates we extract the payload properties to a buffer and obtain - // the RAW payload offset in the zip file. - ZipString property_name(AB_OTA_PAYLOAD_PROPERTIES); - ZipEntry properties_entry; - if (FindEntry(zip, property_name, &properties_entry) != 0) { - LOG(ERROR) << "Can't find " << AB_OTA_PAYLOAD_PROPERTIES; - return INSTALL_CORRUPT; - } - std::vector payload_properties( - properties_entry.uncompressed_length); - if (ExtractToMemory(zip, &properties_entry, payload_properties.data(), - properties_entry.uncompressed_length) != 0) { - LOG(ERROR) << "Can't extract " << AB_OTA_PAYLOAD_PROPERTIES; - return INSTALL_CORRUPT; - } + // For A/B updates we extract the payload properties to a buffer and obtain the RAW payload offset + // in the zip file. + static constexpr const char* AB_OTA_PAYLOAD_PROPERTIES = "payload_properties.txt"; + ZipString property_name(AB_OTA_PAYLOAD_PROPERTIES); + ZipEntry properties_entry; + if (FindEntry(zip, property_name, &properties_entry) != 0) { + LOG(ERROR) << "Failed to find " << AB_OTA_PAYLOAD_PROPERTIES; + return INSTALL_CORRUPT; + } + uint32_t properties_entry_length = properties_entry.uncompressed_length; + std::vector payload_properties(properties_entry_length); + int32_t err = + ExtractToMemory(zip, &properties_entry, payload_properties.data(), properties_entry_length); + if (err != 0) { + LOG(ERROR) << "Failed to extract " << AB_OTA_PAYLOAD_PROPERTIES << ": " << ErrorCodeString(err); + return INSTALL_CORRUPT; + } - ZipString payload_name(AB_OTA_PAYLOAD); - ZipEntry payload_entry; - if (FindEntry(zip, payload_name, &payload_entry) != 0) { - LOG(ERROR) << "Can't find " << AB_OTA_PAYLOAD; - return INSTALL_CORRUPT; - } - long payload_offset = payload_entry.offset; - *cmd = { - "/sbin/update_engine_sideload", - android::base::StringPrintf("--payload=file://%s", path), - android::base::StringPrintf("--offset=%ld", payload_offset), - "--headers=" + std::string(payload_properties.begin(), - payload_properties.end()), - android::base::StringPrintf("--status_fd=%d", status_fd), - }; - return 0; + static constexpr const char* AB_OTA_PAYLOAD = "payload.bin"; + ZipString payload_name(AB_OTA_PAYLOAD); + ZipEntry payload_entry; + if (FindEntry(zip, payload_name, &payload_entry) != 0) { + LOG(ERROR) << "Failed to find " << AB_OTA_PAYLOAD; + return INSTALL_CORRUPT; + } + long payload_offset = payload_entry.offset; + *cmd = { + "/sbin/update_engine_sideload", + "--payload=file://" + path, + android::base::StringPrintf("--offset=%ld", payload_offset), + "--headers=" + std::string(payload_properties.begin(), payload_properties.end()), + android::base::StringPrintf("--status_fd=%d", status_fd), + }; + return 0; } #else // !AB_OTA_UPDATER -static int -update_binary_command(const char* path, ZipArchiveHandle zip, int retry_count, - int status_fd, std::vector* cmd) -{ - // On traditional updates we extract the update binary from the package. - ZipString binary_name(ASSUMED_UPDATE_BINARY_NAME); - ZipEntry binary_entry; - if (FindEntry(zip, binary_name, &binary_entry) != 0) { - return INSTALL_CORRUPT; - } +int update_binary_command(const std::string& path, ZipArchiveHandle zip, int retry_count, + int status_fd, std::vector* cmd) { + CHECK(cmd != nullptr); - const char* binary = "/tmp/update_binary"; - unlink(binary); - int fd = creat(binary, 0755); - if (fd < 0) { - PLOG(ERROR) << "Can't make " << binary; - return INSTALL_ERROR; - } - int error = ExtractEntryToFile(zip, &binary_entry, fd); - close(fd); + // On traditional updates we extract the update binary from the package. + static constexpr const char* UPDATE_BINARY_NAME = "META-INF/com/google/android/update-binary"; + ZipString binary_name(UPDATE_BINARY_NAME); + ZipEntry binary_entry; + if (FindEntry(zip, binary_name, &binary_entry) != 0) { + LOG(ERROR) << "Failed to find update binary " << UPDATE_BINARY_NAME; + return INSTALL_CORRUPT; + } - if (error != 0) { - LOG(ERROR) << "Can't copy " << ASSUMED_UPDATE_BINARY_NAME - << " : " << ErrorCodeString(error); - return INSTALL_ERROR; - } + const char* binary = "/tmp/update_binary"; + unlink(binary); + int fd = creat(binary, 0755); + if (fd == -1) { + PLOG(ERROR) << "Failed to create " << binary; + return INSTALL_ERROR; + } - *cmd = { - binary, - EXPAND(RECOVERY_API_VERSION), // defined in Android.mk - std::to_string(status_fd), - path, - }; - if (retry_count > 0) - cmd->push_back("retry"); - return 0; + int32_t error = ExtractEntryToFile(zip, &binary_entry, fd); + close(fd); + if (error != 0) { + LOG(ERROR) << "Failed to extract " << UPDATE_BINARY_NAME << ": " << ErrorCodeString(error); + return INSTALL_ERROR; + } + + *cmd = { + binary, + EXPAND(RECOVERY_API_VERSION), // defined in Android.mk + std::to_string(status_fd), + path, + }; + if (retry_count > 0) { + cmd->push_back("retry"); + } + return 0; } #endif // !AB_OTA_UPDATER -- cgit v1.2.3 From f8119fbafba0a25c93d9b2ba82b12f4668e7d9ca Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Tue, 18 Apr 2017 21:35:12 -0700 Subject: Minor clean up to install.cpp. - Move some macros / constants into functions; - Remove unneeded #include "minui/minui.h"; - Remove two dead constants (DEFAULT_{FILES,IMAGES}_PROGRESS_FRACTION). Test: mmma bootable/recovery Change-Id: Ib808f14b7569e06e23a8a7cc9b2d4e9aa5469de1 --- install.cpp | 127 +++++++++++++++++++++++++++++------------------------------- 1 file changed, 61 insertions(+), 66 deletions(-) (limited to 'install.cpp') diff --git a/install.cpp b/install.cpp index 4fb809996..95794ce07 100644 --- a/install.cpp +++ b/install.cpp @@ -48,7 +48,6 @@ #include "common.h" #include "error_code.h" -#include "minui/minui.h" #include "otautil/SysUtil.h" #include "otautil/ThermalUtil.h" #include "roots.h" @@ -57,15 +56,11 @@ using namespace std::chrono_literals; -#define PUBLIC_KEYS_FILE "/res/keys" static constexpr const char* METADATA_PATH = "META-INF/com/android/metadata"; -static constexpr const char* UNCRYPT_STATUS = "/cache/recovery/uncrypt_status"; // Default allocation of progress bar segments to operations static constexpr int VERIFICATION_PROGRESS_TIME = 60; static constexpr float VERIFICATION_PROGRESS_FRACTION = 0.25; -static constexpr float DEFAULT_FILES_PROGRESS_FRACTION = 0.4; -static constexpr float DEFAULT_IMAGE_PROGRESS_FRACTION = 0.1; static std::condition_variable finish_log_temperature; @@ -621,80 +616,80 @@ really_install_package(const char *path, bool* wipe_cache, bool needs_mount, return result; } -int -install_package(const char* path, bool* wipe_cache, const char* install_file, - bool needs_mount, int retry_count) -{ - modified_flash = true; - auto start = std::chrono::system_clock::now(); +int install_package(const char* path, bool* wipe_cache, const char* install_file, bool needs_mount, + int retry_count) { + modified_flash = true; + auto start = std::chrono::system_clock::now(); + + int start_temperature = GetMaxValueFromThermalZone(); + int max_temperature = start_temperature; + + int result; + std::vector log_buffer; + if (setup_install_mounts() != 0) { + LOG(ERROR) << "failed to set up expected mounts for install; aborting"; + result = INSTALL_ERROR; + } else { + result = really_install_package(path, wipe_cache, needs_mount, log_buffer, retry_count, + &max_temperature); + } - int start_temperature = GetMaxValueFromThermalZone(); - int max_temperature = start_temperature; + // Measure the time spent to apply OTA update in seconds. + std::chrono::duration duration = std::chrono::system_clock::now() - start; + int time_total = static_cast(duration.count()); - int result; - std::vector log_buffer; - if (setup_install_mounts() != 0) { - LOG(ERROR) << "failed to set up expected mounts for install; aborting"; - result = INSTALL_ERROR; + bool has_cache = volume_for_path("/cache") != nullptr; + // Skip logging the uncrypt_status on devices without /cache. + if (has_cache) { + static constexpr const char* UNCRYPT_STATUS = "/cache/recovery/uncrypt_status"; + if (ensure_path_mounted(UNCRYPT_STATUS) != 0) { + LOG(WARNING) << "Can't mount " << UNCRYPT_STATUS; } else { - result = really_install_package(path, wipe_cache, needs_mount, log_buffer, retry_count, - &max_temperature); - } - - // Measure the time spent to apply OTA update in seconds. - std::chrono::duration duration = std::chrono::system_clock::now() - start; - int time_total = static_cast(duration.count()); - - bool has_cache = volume_for_path("/cache") != nullptr; - // Skip logging the uncrypt_status on devices without /cache. - if (has_cache) { - if (ensure_path_mounted(UNCRYPT_STATUS) != 0) { - LOG(WARNING) << "Can't mount " << UNCRYPT_STATUS; + std::string uncrypt_status; + if (!android::base::ReadFileToString(UNCRYPT_STATUS, &uncrypt_status)) { + PLOG(WARNING) << "failed to read uncrypt status"; + } else if (!android::base::StartsWith(uncrypt_status, "uncrypt_")) { + LOG(WARNING) << "corrupted uncrypt_status: " << uncrypt_status; } else { - std::string uncrypt_status; - if (!android::base::ReadFileToString(UNCRYPT_STATUS, &uncrypt_status)) { - PLOG(WARNING) << "failed to read uncrypt status"; - } else if (!android::base::StartsWith(uncrypt_status, "uncrypt_")) { - LOG(WARNING) << "corrupted uncrypt_status: " << uncrypt_status; - } else { - log_buffer.push_back(android::base::Trim(uncrypt_status)); - } + log_buffer.push_back(android::base::Trim(uncrypt_status)); } } + } - // The first two lines need to be the package name and install result. - std::vector log_header = { - path, - result == INSTALL_SUCCESS ? "1" : "0", - "time_total: " + std::to_string(time_total), - "retry: " + std::to_string(retry_count), - }; - - int end_temperature = GetMaxValueFromThermalZone(); - max_temperature = std::max(end_temperature, max_temperature); - if (start_temperature > 0) { - log_buffer.push_back("temperature_start: " + std::to_string(start_temperature)); - } - if (end_temperature > 0) { - log_buffer.push_back("temperature_end: " + std::to_string(end_temperature)); - } - if (max_temperature > 0) { - log_buffer.push_back("temperature_max: " + std::to_string(max_temperature)); - } + // The first two lines need to be the package name and install result. + std::vector log_header = { + path, + result == INSTALL_SUCCESS ? "1" : "0", + "time_total: " + std::to_string(time_total), + "retry: " + std::to_string(retry_count), + }; - std::string log_content = android::base::Join(log_header, "\n") + "\n" + - android::base::Join(log_buffer, "\n") + "\n"; - if (!android::base::WriteStringToFile(log_content, install_file)) { - PLOG(ERROR) << "failed to write " << install_file; - } + int end_temperature = GetMaxValueFromThermalZone(); + max_temperature = std::max(end_temperature, max_temperature); + if (start_temperature > 0) { + log_buffer.push_back("temperature_start: " + std::to_string(start_temperature)); + } + if (end_temperature > 0) { + log_buffer.push_back("temperature_end: " + std::to_string(end_temperature)); + } + if (max_temperature > 0) { + log_buffer.push_back("temperature_max: " + std::to_string(max_temperature)); + } - // Write a copy into last_log. - LOG(INFO) << log_content; + std::string log_content = + android::base::Join(log_header, "\n") + "\n" + android::base::Join(log_buffer, "\n") + "\n"; + if (!android::base::WriteStringToFile(log_content, install_file)) { + PLOG(ERROR) << "failed to write " << install_file; + } - return result; + // Write a copy into last_log. + LOG(INFO) << log_content; + + return result; } bool verify_package(const unsigned char* package_data, size_t package_size) { + static constexpr const char* PUBLIC_KEYS_FILE = "/res/keys"; std::vector loadedKeys; if (!load_keys(PUBLIC_KEYS_FILE, loadedKeys)) { LOG(ERROR) << "Failed to load keys"; -- cgit v1.2.3 From 8a7afcc6ed5467dd2e9800244d10a88cd359a0a3 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Tue, 18 Apr 2017 22:05:50 -0700 Subject: Add tests for read_metadata_from_package(). Test: recovery_component_test Change-Id: I672a6a4f101c72e82b9f25f165dccd1c9520627b --- install.cpp | 82 +++++++++++++++++++++++++++++-------------------------------- 1 file changed, 39 insertions(+), 43 deletions(-) (limited to 'install.cpp') diff --git a/install.cpp b/install.cpp index 95794ce07..73ddf5e92 100644 --- a/install.cpp +++ b/install.cpp @@ -56,8 +56,6 @@ using namespace std::chrono_literals; -static constexpr const char* METADATA_PATH = "META-INF/com/android/metadata"; - // Default allocation of progress bar segments to operations static constexpr int VERIFICATION_PROGRESS_TIME = 60; static constexpr float VERIFICATION_PROGRESS_FRACTION = 0.25; @@ -79,53 +77,51 @@ static int parse_build_number(const std::string& str) { return -1; } -bool read_metadata_from_package(ZipArchiveHandle zip, std::string* meta_data) { - ZipString metadata_path(METADATA_PATH); - ZipEntry meta_entry; - if (meta_data == nullptr) { - LOG(ERROR) << "string* meta_data can't be nullptr"; - return false; - } - if (FindEntry(zip, metadata_path, &meta_entry) != 0) { - LOG(ERROR) << "Failed to find " << METADATA_PATH << " in update package"; - return false; - } +bool read_metadata_from_package(ZipArchiveHandle zip, std::string* metadata) { + CHECK(metadata != nullptr); - meta_data->resize(meta_entry.uncompressed_length, '\0'); - if (ExtractToMemory(zip, &meta_entry, reinterpret_cast(&(*meta_data)[0]), - meta_entry.uncompressed_length) != 0) { - LOG(ERROR) << "Failed to read metadata in update package"; - return false; - } - return true; + static constexpr const char* METADATA_PATH = "META-INF/com/android/metadata"; + ZipString path(METADATA_PATH); + ZipEntry entry; + if (FindEntry(zip, path, &entry) != 0) { + LOG(ERROR) << "Failed to find " << METADATA_PATH; + return false; + } + + uint32_t length = entry.uncompressed_length; + metadata->resize(length, '\0'); + int32_t err = ExtractToMemory(zip, &entry, reinterpret_cast(&(*metadata)[0]), length); + if (err != 0) { + LOG(ERROR) << "Failed to extract " << METADATA_PATH << ": " << ErrorCodeString(err); + return false; + } + return true; } // Read the build.version.incremental of src/tgt from the metadata and log it to last_install. static void read_source_target_build(ZipArchiveHandle zip, std::vector& log_buffer) { - std::string meta_data; - if (!read_metadata_from_package(zip, &meta_data)) { - return; - } - // Examples of the pre-build and post-build strings in metadata: - // pre-build-incremental=2943039 - // post-build-incremental=2951741 - std::vector lines = android::base::Split(meta_data, "\n"); - for (const std::string& line : lines) { - std::string str = android::base::Trim(line); - if (android::base::StartsWith(str, "pre-build-incremental")){ - int source_build = parse_build_number(str); - if (source_build != -1) { - log_buffer.push_back(android::base::StringPrintf("source_build: %d", - source_build)); - } - } else if (android::base::StartsWith(str, "post-build-incremental")) { - int target_build = parse_build_number(str); - if (target_build != -1) { - log_buffer.push_back(android::base::StringPrintf("target_build: %d", - target_build)); - } - } + std::string metadata; + if (!read_metadata_from_package(zip, &metadata)) { + return; + } + // Examples of the pre-build and post-build strings in metadata: + // pre-build-incremental=2943039 + // post-build-incremental=2951741 + std::vector lines = android::base::Split(metadata, "\n"); + for (const std::string& line : lines) { + std::string str = android::base::Trim(line); + if (android::base::StartsWith(str, "pre-build-incremental")) { + int source_build = parse_build_number(str); + if (source_build != -1) { + log_buffer.push_back(android::base::StringPrintf("source_build: %d", source_build)); + } + } else if (android::base::StartsWith(str, "post-build-incremental")) { + int target_build = parse_build_number(str); + if (target_build != -1) { + log_buffer.push_back(android::base::StringPrintf("target_build: %d", target_build)); + } } + } } // Extract the update binary from the open zip archive |zip| located at |path| and store into |cmd| -- cgit v1.2.3 From 919d2c9a5341d123fa1c97ffad3549c20e1dd021 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Mon, 10 Apr 2017 16:55:57 -0700 Subject: Call libvintf to verify package compatibility. The libvintf API has landed. Hook up to do the actual verification. Bug: 36597505 Test: recovery_component_test Test: m recoveryimage; adb sideload on angler and sailfish, with packages that contain dummy compatibility entries. Test: m recoveryimage; adb sideload on angler and sailfish, with packages that don't contain any compatibility entries. Change-Id: Idbd6f5aaef605ca51b20e667505d686de5ac781f (cherry picked from commit da320ac6ab53395ddff3cc08b88a61f977ed939a) --- install.cpp | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) (limited to 'install.cpp') diff --git a/install.cpp b/install.cpp index 95794ce07..2a647b1a7 100644 --- a/install.cpp +++ b/install.cpp @@ -44,6 +44,7 @@ #include #include #include +#include #include #include "common.h" @@ -539,10 +540,15 @@ bool verify_package_compatibility(ZipArchiveHandle package_zip) { } CloseArchive(zip_handle); - // TODO(b/36814503): Enable the actual verification when VintfObject::CheckCompatibility() lands. - // VintfObject::CheckCompatibility returns zero on success. - // return (android::vintf::VintfObject::CheckCompatibility(compatibility_info, true) == 0); - return true; + // VintfObjectRecovery::CheckCompatibility returns zero on success. + std::string err; + int result = android::vintf::VintfObjectRecovery::CheckCompatibility(compatibility_info, &err); + if (result == 0) { + return true; + } + + LOG(ERROR) << "Failed to verify package compatibility (result " << result << "): " << err; + return false; } static int @@ -594,7 +600,6 @@ really_install_package(const char *path, bool* wipe_cache, bool needs_mount, // Additionally verify the compatibility of the package. if (!verify_package_compatibility(zip)) { - LOG(ERROR) << "Failed to verify package compatibility"; log_buffer.push_back(android::base::StringPrintf("error: %d", kPackageCompatibilityFailure)); sysReleaseMap(&map); CloseArchive(zip); -- cgit v1.2.3 From b24510cd60c899f2967d3f0875ee4a744aa569c2 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Thu, 20 Apr 2017 17:54:27 -0700 Subject: librecovery: Remove -Wno-unused-parameter and add -Wall. Test: mmma bootable/recovery Change-Id: I5598d32bebb9dbda4a183a1502e0b7dc4918392e --- install.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'install.cpp') diff --git a/install.cpp b/install.cpp index e5a59b832..e945d13ab 100644 --- a/install.cpp +++ b/install.cpp @@ -211,7 +211,7 @@ static int check_newer_ab_build(ZipArchiveHandle zip) { return 0; } -int update_binary_command(const std::string& path, ZipArchiveHandle zip, int retry_count, +int update_binary_command(const std::string& path, ZipArchiveHandle zip, int /* retry_count */, int status_fd, std::vector* cmd) { CHECK(cmd != nullptr); int ret = check_newer_ab_build(zip); -- cgit v1.2.3 From 29ee69bf27cc7ad7ef7e604684110b3f562ed42d Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Mon, 1 May 2017 12:23:17 -0700 Subject: recovery: Change install_package() to take std::string. Also change the parameter type for log_buffer from reference to pointer, so the styles for parameters look consistent. Test: mmma bootable/recovery Test: sideload a package with the new recovery image Change-Id: I8f25580ccf22977624648b3e2181cca44dd67c1b --- install.cpp | 141 +++++++++++++++++++++++++++++++----------------------------- 1 file changed, 72 insertions(+), 69 deletions(-) (limited to 'install.cpp') diff --git a/install.cpp b/install.cpp index e945d13ab..d86f46caa 100644 --- a/install.cpp +++ b/install.cpp @@ -100,7 +100,7 @@ bool read_metadata_from_package(ZipArchiveHandle zip, std::string* metadata) { } // Read the build.version.incremental of src/tgt from the metadata and log it to last_install. -static void read_source_target_build(ZipArchiveHandle zip, std::vector& log_buffer) { +static void read_source_target_build(ZipArchiveHandle zip, std::vector* log_buffer) { std::string metadata; if (!read_metadata_from_package(zip, &metadata)) { return; @@ -114,12 +114,12 @@ static void read_source_target_build(ZipArchiveHandle zip, std::vectorpush_back(android::base::StringPrintf("source_build: %d", source_build)); } } else if (android::base::StartsWith(str, "post-build-incremental")) { int target_build = parse_build_number(str); if (target_build != -1) { - log_buffer.push_back(android::base::StringPrintf("target_build: %d", target_build)); + log_buffer->push_back(android::base::StringPrintf("target_build: %d", target_build)); } } } @@ -308,8 +308,8 @@ static void log_max_temperature(int* max_temperature) { } // If the package contains an update binary, extract it and run it. -static int try_update_binary(const char* path, ZipArchiveHandle zip, bool* wipe_cache, - std::vector& log_buffer, int retry_count, +static int try_update_binary(const std::string& path, ZipArchiveHandle zip, bool* wipe_cache, + std::vector* log_buffer, int retry_count, int* max_temperature) { read_source_target_build(zip, log_buffer); @@ -452,7 +452,7 @@ static int try_update_binary(const char* path, ZipArchiveHandle zip, bool* wipe_ } else if (command == "log") { if (!args.empty()) { // Save the logging request from updater and write to last_install later. - log_buffer.push_back(args); + log_buffer->push_back(args); } else { LOG(ERROR) << "invalid \"log\" parameters: " << line; } @@ -547,78 +547,81 @@ bool verify_package_compatibility(ZipArchiveHandle package_zip) { return false; } -static int -really_install_package(const char *path, bool* wipe_cache, bool needs_mount, - std::vector& log_buffer, int retry_count, int* max_temperature) -{ - ui->SetBackground(RecoveryUI::INSTALLING_UPDATE); - ui->Print("Finding update package...\n"); - // Give verification half the progress bar... - ui->SetProgressType(RecoveryUI::DETERMINATE); - ui->ShowProgress(VERIFICATION_PROGRESS_FRACTION, VERIFICATION_PROGRESS_TIME); - LOG(INFO) << "Update location: " << path; - - // Map the update package into memory. - ui->Print("Opening update package...\n"); - - if (path && needs_mount) { - if (path[0] == '@') { - ensure_path_mounted(path+1); - } else { - ensure_path_mounted(path); - } - } - - MemMapping map; - if (sysMapFile(path, &map) != 0) { - LOG(ERROR) << "failed to map file"; - return INSTALL_CORRUPT; - } - - // Verify package. - if (!verify_package(map.addr, map.length)) { - log_buffer.push_back(android::base::StringPrintf("error: %d", kZipVerificationFailure)); - sysReleaseMap(&map); - return INSTALL_CORRUPT; +static int really_install_package(const std::string& path, bool* wipe_cache, bool needs_mount, + std::vector* log_buffer, int retry_count, + int* max_temperature) { + ui->SetBackground(RecoveryUI::INSTALLING_UPDATE); + ui->Print("Finding update package...\n"); + // Give verification half the progress bar... + ui->SetProgressType(RecoveryUI::DETERMINATE); + ui->ShowProgress(VERIFICATION_PROGRESS_FRACTION, VERIFICATION_PROGRESS_TIME); + LOG(INFO) << "Update location: " << path; + + // Map the update package into memory. + ui->Print("Opening update package...\n"); + + if (needs_mount) { + if (path[0] == '@') { + ensure_path_mounted(path.substr(1).c_str()); + } else { + ensure_path_mounted(path.c_str()); } + } - // Try to open the package. - ZipArchiveHandle zip; - int err = OpenArchiveFromMemory(map.addr, map.length, path, &zip); - if (err != 0) { - LOG(ERROR) << "Can't open " << path << " : " << ErrorCodeString(err); - log_buffer.push_back(android::base::StringPrintf("error: %d", kZipOpenFailure)); + MemMapping map; + if (sysMapFile(path.c_str(), &map) != 0) { + LOG(ERROR) << "failed to map file"; + return INSTALL_CORRUPT; + } - sysReleaseMap(&map); - CloseArchive(zip); - return INSTALL_CORRUPT; - } + // Verify package. + if (!verify_package(map.addr, map.length)) { + log_buffer->push_back(android::base::StringPrintf("error: %d", kZipVerificationFailure)); + sysReleaseMap(&map); + return INSTALL_CORRUPT; + } - // Additionally verify the compatibility of the package. - if (!verify_package_compatibility(zip)) { - log_buffer.push_back(android::base::StringPrintf("error: %d", kPackageCompatibilityFailure)); - sysReleaseMap(&map); - CloseArchive(zip); - return INSTALL_CORRUPT; - } + // Try to open the package. + ZipArchiveHandle zip; + int err = OpenArchiveFromMemory(map.addr, map.length, path.c_str(), &zip); + if (err != 0) { + LOG(ERROR) << "Can't open " << path << " : " << ErrorCodeString(err); + log_buffer->push_back(android::base::StringPrintf("error: %d", kZipOpenFailure)); - // Verify and install the contents of the package. - ui->Print("Installing update...\n"); - if (retry_count > 0) { - ui->Print("Retry attempt: %d\n", retry_count); - } - ui->SetEnableReboot(false); - int result = try_update_binary(path, zip, wipe_cache, log_buffer, retry_count, max_temperature); - ui->SetEnableReboot(true); - ui->Print("\n"); + sysReleaseMap(&map); + CloseArchive(zip); + return INSTALL_CORRUPT; + } + // Additionally verify the compatibility of the package. + if (!verify_package_compatibility(zip)) { + log_buffer->push_back(android::base::StringPrintf("error: %d", kPackageCompatibilityFailure)); sysReleaseMap(&map); CloseArchive(zip); - return result; + return INSTALL_CORRUPT; + } + + // Verify and install the contents of the package. + ui->Print("Installing update...\n"); + if (retry_count > 0) { + ui->Print("Retry attempt: %d\n", retry_count); + } + ui->SetEnableReboot(false); + int result = try_update_binary(path, zip, wipe_cache, log_buffer, retry_count, max_temperature); + ui->SetEnableReboot(true); + ui->Print("\n"); + + sysReleaseMap(&map); + CloseArchive(zip); + return result; } -int install_package(const char* path, bool* wipe_cache, const char* install_file, bool needs_mount, - int retry_count) { +int install_package(const std::string& path, bool* wipe_cache, const std::string& install_file, + bool needs_mount, int retry_count) { + CHECK(!path.empty()); + CHECK(!install_file.empty()); + CHECK(wipe_cache != nullptr); + modified_flash = true; auto start = std::chrono::system_clock::now(); @@ -631,7 +634,7 @@ int install_package(const char* path, bool* wipe_cache, const char* install_file LOG(ERROR) << "failed to set up expected mounts for install; aborting"; result = INSTALL_ERROR; } else { - result = really_install_package(path, wipe_cache, needs_mount, log_buffer, retry_count, + result = really_install_package(path, wipe_cache, needs_mount, &log_buffer, retry_count, &max_temperature); } -- cgit v1.2.3 From b656a154ea497c1a179079f082b0a0701453bec5 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Tue, 18 Apr 2017 23:54:29 -0700 Subject: Move sysMapFile and sysReleaseMap into MemMapping class. Test: recovery_component_test Test: recovery_unit_test Test: Apply an OTA on angler. Change-Id: I7170f03e4ce1fe06184ca1d7bcce0a695f33ac4d --- install.cpp | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) (limited to 'install.cpp') diff --git a/install.cpp b/install.cpp index d86f46caa..689f4a0c6 100644 --- a/install.cpp +++ b/install.cpp @@ -569,7 +569,7 @@ static int really_install_package(const std::string& path, bool* wipe_cache, boo } MemMapping map; - if (sysMapFile(path.c_str(), &map) != 0) { + if (!map.MapFile(path)) { LOG(ERROR) << "failed to map file"; return INSTALL_CORRUPT; } @@ -577,7 +577,6 @@ static int really_install_package(const std::string& path, bool* wipe_cache, boo // Verify package. if (!verify_package(map.addr, map.length)) { log_buffer->push_back(android::base::StringPrintf("error: %d", kZipVerificationFailure)); - sysReleaseMap(&map); return INSTALL_CORRUPT; } @@ -588,7 +587,6 @@ static int really_install_package(const std::string& path, bool* wipe_cache, boo LOG(ERROR) << "Can't open " << path << " : " << ErrorCodeString(err); log_buffer->push_back(android::base::StringPrintf("error: %d", kZipOpenFailure)); - sysReleaseMap(&map); CloseArchive(zip); return INSTALL_CORRUPT; } @@ -596,7 +594,6 @@ static int really_install_package(const std::string& path, bool* wipe_cache, boo // Additionally verify the compatibility of the package. if (!verify_package_compatibility(zip)) { log_buffer->push_back(android::base::StringPrintf("error: %d", kPackageCompatibilityFailure)); - sysReleaseMap(&map); CloseArchive(zip); return INSTALL_CORRUPT; } @@ -611,7 +608,6 @@ static int really_install_package(const std::string& path, bool* wipe_cache, boo ui->SetEnableReboot(true); ui->Print("\n"); - sysReleaseMap(&map); CloseArchive(zip); return result; } -- cgit v1.2.3 From ec9706738f35a859f66fd0758b73381055804f63 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Wed, 3 May 2017 11:00:48 -0700 Subject: Remove EXPAND/STRINGIFY macros. They are error-prone by putting anything into a string (e.g. EXPAND(RECOVERY_API_VERSION) would become "RECOVER_API_VERSION" if we forgot to pass -DRECOVERY_API_VERSION=3). RECOVERY_API_VERSION is the only user (in bootable/recovery) that gets stringified. Assign it to a typed var and sanity check the value. Don't see other reference to the macros from device-specific recovery directories (they can still define that locally if really needed). Test: recovery_component_test Test: Sideload an OTA on angler and marlin respectively. Change-Id: I358bbdf8f0a99db5ce4c7bc2fdcafe8013501b64 --- install.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'install.cpp') diff --git a/install.cpp b/install.cpp index 689f4a0c6..2cc06603b 100644 --- a/install.cpp +++ b/install.cpp @@ -287,7 +287,7 @@ int update_binary_command(const std::string& path, ZipArchiveHandle zip, int ret *cmd = { binary, - EXPAND(RECOVERY_API_VERSION), // defined in Android.mk + std::to_string(kRecoveryApiVersion), std::to_string(status_fd), path, }; -- cgit v1.2.3 From 8be0f39fec7f26164fd0791ff6d15bde65fc849c Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Thu, 4 May 2017 00:29:31 +0000 Subject: Revert "Remove EXPAND/STRINGIFY macros." This reverts commit ec9706738f35a859f66fd0758b73381055804f63. Reason for revert: It's not a good idea to put RECOVERY_API_VERSION in common.h, which might be included by device-specific codes (but with RECOVERY_API_VERSION undefined). Change-Id: I9feb9c64a5af3e9165164622a59b043aa28a8b8c --- install.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'install.cpp') diff --git a/install.cpp b/install.cpp index 2cc06603b..689f4a0c6 100644 --- a/install.cpp +++ b/install.cpp @@ -287,7 +287,7 @@ int update_binary_command(const std::string& path, ZipArchiveHandle zip, int ret *cmd = { binary, - std::to_string(kRecoveryApiVersion), + EXPAND(RECOVERY_API_VERSION), // defined in Android.mk std::to_string(status_fd), path, }; -- cgit v1.2.3 From 00d5757186c279ba5e8a52a6f5209be3e7152025 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Tue, 2 May 2017 15:48:54 -0700 Subject: Add a binary path param to update_binary_command(). This allows writing native tests for non-A/B update_binary_command(). Prior to this CL, it was extracting the updater to a hard-coded location (/tmp/update_binary) that's not available under the test environment. Test: recovery_component_test on angler and marlin respectively. Test: Sideload OTA packages on angler and marlin respectively. Change-Id: I78b9cc211d90c0a16a84e94e339b65759300e2a8 --- install.cpp | 44 +++++++++++++++++++++++--------------------- 1 file changed, 23 insertions(+), 21 deletions(-) (limited to 'install.cpp') diff --git a/install.cpp b/install.cpp index 689f4a0c6..a1f2e4fbd 100644 --- a/install.cpp +++ b/install.cpp @@ -51,6 +51,7 @@ #include "error_code.h" #include "otautil/SysUtil.h" #include "otautil/ThermalUtil.h" +#include "private/install.h" #include "roots.h" #include "ui.h" #include "verifier.h" @@ -125,12 +126,6 @@ static void read_source_target_build(ZipArchiveHandle zip, std::vector* cmd); - #ifdef AB_OTA_UPDATER // Parses the metadata of the OTA package in |zip| and checks whether we are @@ -211,8 +206,9 @@ static int check_newer_ab_build(ZipArchiveHandle zip) { return 0; } -int update_binary_command(const std::string& path, ZipArchiveHandle zip, int /* retry_count */, - int status_fd, std::vector* cmd) { +int update_binary_command(const std::string& package, ZipArchiveHandle zip, + const std::string& binary_path, int /* retry_count */, int status_fd, + std::vector* cmd) { CHECK(cmd != nullptr); int ret = check_newer_ab_build(zip); if (ret != 0) { @@ -246,8 +242,8 @@ int update_binary_command(const std::string& path, ZipArchiveHandle zip, int /* } long payload_offset = payload_entry.offset; *cmd = { - "/sbin/update_engine_sideload", - "--payload=file://" + path, + binary_path, + "--payload=file://" + package, android::base::StringPrintf("--offset=%ld", payload_offset), "--headers=" + std::string(payload_properties.begin(), payload_properties.end()), android::base::StringPrintf("--status_fd=%d", status_fd), @@ -257,8 +253,9 @@ int update_binary_command(const std::string& path, ZipArchiveHandle zip, int /* #else // !AB_OTA_UPDATER -int update_binary_command(const std::string& path, ZipArchiveHandle zip, int retry_count, - int status_fd, std::vector* cmd) { +int update_binary_command(const std::string& package, ZipArchiveHandle zip, + const std::string& binary_path, int retry_count, int status_fd, + std::vector* cmd) { CHECK(cmd != nullptr); // On traditional updates we extract the update binary from the package. @@ -270,11 +267,10 @@ int update_binary_command(const std::string& path, ZipArchiveHandle zip, int ret return INSTALL_CORRUPT; } - const char* binary = "/tmp/update_binary"; - unlink(binary); - int fd = creat(binary, 0755); + unlink(binary_path.c_str()); + int fd = creat(binary_path.c_str(), 0755); if (fd == -1) { - PLOG(ERROR) << "Failed to create " << binary; + PLOG(ERROR) << "Failed to create " << binary_path; return INSTALL_ERROR; } @@ -286,10 +282,10 @@ int update_binary_command(const std::string& path, ZipArchiveHandle zip, int ret } *cmd = { - binary, + binary_path, EXPAND(RECOVERY_API_VERSION), // defined in Android.mk std::to_string(status_fd), - path, + package, }; if (retry_count > 0) { cmd->push_back("retry"); @@ -308,7 +304,7 @@ static void log_max_temperature(int* max_temperature) { } // If the package contains an update binary, extract it and run it. -static int try_update_binary(const std::string& path, ZipArchiveHandle zip, bool* wipe_cache, +static int try_update_binary(const std::string& package, ZipArchiveHandle zip, bool* wipe_cache, std::vector* log_buffer, int retry_count, int* max_temperature) { read_source_target_build(zip, log_buffer); @@ -317,7 +313,13 @@ static int try_update_binary(const std::string& path, ZipArchiveHandle zip, bool pipe(pipefd); std::vector args; - int ret = update_binary_command(path, zip, retry_count, pipefd[1], &args); +#ifdef AB_OTA_UPDATER + int ret = update_binary_command(package, zip, "/sbin/update_engine_sideload", retry_count, + pipefd[1], &args); +#else + int ret = update_binary_command(package, zip, "/tmp/update-binary", retry_count, pipefd[1], + &args); +#endif if (ret) { close(pipefd[0]); close(pipefd[1]); @@ -472,7 +474,7 @@ static int try_update_binary(const std::string& path, ZipArchiveHandle zip, bool return INSTALL_RETRY; } if (!WIFEXITED(status) || WEXITSTATUS(status) != 0) { - LOG(ERROR) << "Error in " << path << " (Status " << WEXITSTATUS(status) << ")"; + LOG(ERROR) << "Error in " << package << " (Status " << WEXITSTATUS(status) << ")"; return INSTALL_ERROR; } -- cgit v1.2.3 From 379571098d1b597a2a5f159dfc7522cc4fde6c63 Mon Sep 17 00:00:00 2001 From: Tianjie Xu Date: Wed, 7 Jun 2017 17:59:55 -0700 Subject: Fix a race condition for temperature_logger It's a rare deadlock that happens on fugu when the updater exits too fast due to a format error. Bug: 62379170 Test: fake-ota fails gracefully without hanging. Change-Id: Icbd6cff4f6b44ca20f4d75f8039332111f696cc5 --- install.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'install.cpp') diff --git a/install.cpp b/install.cpp index a1f2e4fbd..db4ba9373 100644 --- a/install.cpp +++ b/install.cpp @@ -27,6 +27,7 @@ #include #include +#include #include #include #include @@ -294,11 +295,12 @@ int update_binary_command(const std::string& package, ZipArchiveHandle zip, } #endif // !AB_OTA_UPDATER -static void log_max_temperature(int* max_temperature) { +static void log_max_temperature(int* max_temperature, const std::atomic& logger_finished) { CHECK(max_temperature != nullptr); std::mutex mtx; std::unique_lock lck(mtx); - while (finish_log_temperature.wait_for(lck, 20s) == std::cv_status::timeout) { + while (!logger_finished.load() && + finish_log_temperature.wait_for(lck, 20s) == std::cv_status::timeout) { *max_temperature = std::max(*max_temperature, GetMaxValueFromThermalZone()); } } @@ -403,7 +405,8 @@ static int try_update_binary(const std::string& package, ZipArchiveHandle zip, b } close(pipefd[1]); - std::thread temperature_logger(log_max_temperature, max_temperature); + std::atomic logger_finished(false); + std::thread temperature_logger(log_max_temperature, max_temperature, std::ref(logger_finished)); *wipe_cache = false; bool retry_update = false; @@ -467,6 +470,7 @@ static int try_update_binary(const std::string& package, ZipArchiveHandle zip, b int status; waitpid(pid, &status, 0); + logger_finished.store(true); finish_log_temperature.notify_one(); temperature_logger.join(); -- cgit v1.2.3 From a29d8d69d23b2ed2ead83c67f84c9c474dd563cb Mon Sep 17 00:00:00 2001 From: Daniel Micay Date: Fri, 23 Jun 2017 08:33:24 -0400 Subject: avoid assuming build number is a 32-bit integer The install logging currently assumes that the build number is a 32-bit integer and prints an error when that doesn't hold true. However, that's only a convention used by Google builds. From build/core/version_defaults.mk: ifeq "" "$(BUILD_NUMBER)" # BUILD_NUMBER should be set to the source control value that # represents the current state of the source code. E.g., a # perforce changelist number or a git hash. Can be an arbitrary string # (to allow for source control that uses something other than numbers), # but must be a single word and a valid file name. # # If no BUILD_NUMBER is set, create a useful "I am an engineering build # from this date/time" value. Make it start with a non-digit so that # anyone trying to parse it as an integer will probably get "0". BUILD_NUMBER := eng.$(shell echo $${USER:0:6}).$(shell $(DATE) +%Y%m%d.%H%M%S) endif Change-Id: I8e7cec0618783f69545ba76d0dce2bbc1681784c --- install.cpp | 22 +++++++++------------- 1 file changed, 9 insertions(+), 13 deletions(-) (limited to 'install.cpp') diff --git a/install.cpp b/install.cpp index db4ba9373..7ba8f0139 100644 --- a/install.cpp +++ b/install.cpp @@ -66,18 +66,14 @@ static constexpr float VERIFICATION_PROGRESS_FRACTION = 0.25; static std::condition_variable finish_log_temperature; // This function parses and returns the build.version.incremental -static int parse_build_number(const std::string& str) { +static std::string parse_build_number(const std::string& str) { size_t pos = str.find('='); if (pos != std::string::npos) { - std::string num_string = android::base::Trim(str.substr(pos+1)); - int build_number; - if (android::base::ParseInt(num_string.c_str(), &build_number, 0)) { - return build_number; - } + return android::base::Trim(str.substr(pos+1)); } LOG(ERROR) << "Failed to parse build number in " << str; - return -1; + return ""; } bool read_metadata_from_package(ZipArchiveHandle zip, std::string* metadata) { @@ -114,14 +110,14 @@ static void read_source_target_build(ZipArchiveHandle zip, std::vectorpush_back(android::base::StringPrintf("source_build: %d", source_build)); + std::string source_build = parse_build_number(str); + if (!source_build.empty()) { + log_buffer->push_back("source_build: " + source_build); } } else if (android::base::StartsWith(str, "post-build-incremental")) { - int target_build = parse_build_number(str); - if (target_build != -1) { - log_buffer->push_back(android::base::StringPrintf("target_build: %d", target_build)); + std::string target_build = parse_build_number(str); + if (!target_build.empty()) { + log_buffer->push_back("target_build: " + target_build); } } } -- cgit v1.2.3 From de6735e80cc65be50381388640d94f1b1d0f20fa Mon Sep 17 00:00:00 2001 From: Tianjie Xu Date: Mon, 10 Jul 2017 15:13:33 -0700 Subject: Fix the android-cloexec-* warnings in bootable/recovery Add the O_CLOEXEC or 'e' accordingly. Bug: 63510015 Test: recovery tests pass Change-Id: I7094bcc6af22c9687eb535116b2ca6a59178b303 --- install.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'install.cpp') diff --git a/install.cpp b/install.cpp index 7ba8f0139..7fbf5c01f 100644 --- a/install.cpp +++ b/install.cpp @@ -265,7 +265,7 @@ int update_binary_command(const std::string& package, ZipArchiveHandle zip, } unlink(binary_path.c_str()); - int fd = creat(binary_path.c_str(), 0755); + int fd = open(binary_path.c_str(), O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC, 0755); if (fd == -1) { PLOG(ERROR) << "Failed to create " << binary_path; return INSTALL_ERROR; -- cgit v1.2.3