From 8ca3220bfe116c8bcecb61a801d904d84484598e Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Wed, 25 Jul 2018 13:05:14 -0700 Subject: bootloader_message: Build libbootloader_message as shared lib. And uses the shared lib version of libbase and libfs_mgr. Bug: 78793464 Test: `m dist` Test: Run recovery_{unit,component}_test on marlin. Change-Id: I750c02d0bfccd6e58fb01f641de02532ace52e00 --- bootloader_message/Android.bp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/bootloader_message/Android.bp b/bootloader_message/Android.bp index ab23733cd..5cd21323c 100644 --- a/bootloader_message/Android.bp +++ b/bootloader_message/Android.bp @@ -14,7 +14,7 @@ // limitations under the License. // -cc_library_static { +cc_library { name: "libbootloader_message", recovery_available: true, srcs: ["bootloader_message.cpp"], @@ -22,7 +22,7 @@ cc_library_static { "-Wall", "-Werror", ], - static_libs: [ + shared_libs: [ "libbase", "libfs_mgr", ], -- cgit v1.2.3 From 4d9e62d8a07b233da4d82a42eb30de64cf2b45bd Mon Sep 17 00:00:00 2001 From: Tianjie Xu Date: Fri, 11 May 2018 10:41:44 -0700 Subject: Add proto3 support for care_map Switching to the protobuf format helps to make the care_map more extensible. As we have such plans in the future, add the support to parse the protobuf message in the update_verifier. Bug: 77867897 Test: unit tests pass, update_verifier successfully verifies a care_map.pb Change-Id: I9fe83cb4dd3cc8d6fd0260f2a47338fe142d3938 --- tests/Android.mk | 3 +- tests/component/update_verifier_test.cpp | 113 +++++++++++++++++---- update_verifier/Android.bp | 7 ++ update_verifier/care_map.proto | 31 ++++++ .../include/update_verifier/update_verifier.h | 10 +- update_verifier/update_verifier.cpp | 72 ++++++++----- 6 files changed, 191 insertions(+), 45 deletions(-) create mode 100644 update_verifier/care_map.proto diff --git a/tests/Android.mk b/tests/Android.mk index 1fa259ebc..e68e77eb0 100644 --- a/tests/Android.mk +++ b/tests/Android.mk @@ -113,7 +113,8 @@ LOCAL_SRC_FILES := \ component/verifier_test.cpp LOCAL_SHARED_LIBRARIES := \ - libhidlbase + libhidlbase \ + libprotobuf-cpp-lite tune2fs_static_libraries := \ libext2_com_err \ diff --git a/tests/component/update_verifier_test.cpp b/tests/component/update_verifier_test.cpp index f6ef6dcfd..a97071635 100644 --- a/tests/component/update_verifier_test.cpp +++ b/tests/component/update_verifier_test.cpp @@ -14,14 +14,20 @@ * limitations under the License. */ +#include + #include +#include +#include #include #include #include #include +#include #include -#include + +#include "care_map.pb.h" class UpdateVerifierTest : public ::testing::Test { protected: @@ -30,7 +36,30 @@ class UpdateVerifierTest : public ::testing::Test { verity_supported = android::base::EqualsIgnoreCase(verity_mode, "enforcing"); } + // Returns a serialized string of the proto3 message according to the given partition info. + std::string ConstructProto( + std::vector>& partitions) { + UpdateVerifier::CareMap result; + for (const auto& partition : partitions) { + UpdateVerifier::CareMap::PartitionInfo info; + if (partition.find("name") != partition.end()) { + info.set_name(partition.at("name")); + } + if (partition.find("ranges") != partition.end()) { + info.set_ranges(partition.at("ranges")); + } + if (partition.find("fingerprint") != partition.end()) { + info.set_fingerprint(partition.at("fingerprint")); + } + + *result.add_partitions() = info; + } + + return result.SerializeAsString(); + } + bool verity_supported; + TemporaryFile care_map_file; }; TEST_F(UpdateVerifierTest, verify_image_no_care_map) { @@ -45,26 +74,26 @@ TEST_F(UpdateVerifierTest, verify_image_smoke) { return; } - TemporaryFile temp_file; std::string content = "system\n2,0,1"; - ASSERT_TRUE(android::base::WriteStringToFile(content, temp_file.path)); - ASSERT_TRUE(verify_image(temp_file.path)); + ASSERT_TRUE(android::base::WriteStringToFile(content, care_map_file.path)); + ASSERT_TRUE(verify_image(care_map_file.path)); // Leading and trailing newlines should be accepted. - ASSERT_TRUE(android::base::WriteStringToFile("\n" + content + "\n\n", temp_file.path)); - ASSERT_TRUE(verify_image(temp_file.path)); + ASSERT_TRUE(android::base::WriteStringToFile("\n" + content + "\n\n", care_map_file.path)); + ASSERT_TRUE(verify_image(care_map_file.path)); +} + +TEST_F(UpdateVerifierTest, verify_image_empty_care_map) { + ASSERT_FALSE(verify_image(care_map_file.path)); } TEST_F(UpdateVerifierTest, verify_image_wrong_lines) { // The care map file can have only 2 / 4 / 6 lines. - TemporaryFile temp_file; - ASSERT_FALSE(verify_image(temp_file.path)); + ASSERT_TRUE(android::base::WriteStringToFile("line1", care_map_file.path)); + ASSERT_FALSE(verify_image(care_map_file.path)); - ASSERT_TRUE(android::base::WriteStringToFile("line1", temp_file.path)); - ASSERT_FALSE(verify_image(temp_file.path)); - - ASSERT_TRUE(android::base::WriteStringToFile("line1\nline2\nline3", temp_file.path)); - ASSERT_FALSE(verify_image(temp_file.path)); + ASSERT_TRUE(android::base::WriteStringToFile("line1\nline2\nline3", care_map_file.path)); + ASSERT_FALSE(verify_image(care_map_file.path)); } TEST_F(UpdateVerifierTest, verify_image_malformed_care_map) { @@ -74,10 +103,9 @@ TEST_F(UpdateVerifierTest, verify_image_malformed_care_map) { return; } - TemporaryFile temp_file; std::string content = "system\n2,1,0"; - ASSERT_TRUE(android::base::WriteStringToFile(content, temp_file.path)); - ASSERT_FALSE(verify_image(temp_file.path)); + ASSERT_TRUE(android::base::WriteStringToFile(content, care_map_file.path)); + ASSERT_FALSE(verify_image(care_map_file.path)); } TEST_F(UpdateVerifierTest, verify_image_legacy_care_map) { @@ -87,8 +115,55 @@ TEST_F(UpdateVerifierTest, verify_image_legacy_care_map) { return; } - TemporaryFile temp_file; std::string content = "/dev/block/bootdevice/by-name/system\n2,1,0"; - ASSERT_TRUE(android::base::WriteStringToFile(content, temp_file.path)); - ASSERT_TRUE(verify_image(temp_file.path)); + ASSERT_TRUE(android::base::WriteStringToFile(content, care_map_file.path)); + ASSERT_TRUE(verify_image(care_map_file.path)); +} + +TEST_F(UpdateVerifierTest, verify_image_protobuf_care_map_smoke) { + // This test relies on dm-verity support. + if (!verity_supported) { + GTEST_LOG_(INFO) << "Test skipped on devices without dm-verity support."; + return; + } + + std::vector> partitions = { + { { "name", "system" }, { "ranges", "2,0,1" } }, + }; + + std::string proto = ConstructProto(partitions); + ASSERT_TRUE(android::base::WriteStringToFile(proto, care_map_file.path)); + ASSERT_TRUE(verify_image(care_map_file.path)); +} + +TEST_F(UpdateVerifierTest, verify_image_protobuf_care_map_missing_name) { + // This test relies on dm-verity support. + if (!verity_supported) { + GTEST_LOG_(INFO) << "Test skipped on devices without dm-verity support."; + return; + } + + std::vector> partitions = { + { { "ranges", "2,0,1" } }, + }; + + std::string proto = ConstructProto(partitions); + ASSERT_TRUE(android::base::WriteStringToFile(proto, care_map_file.path)); + ASSERT_FALSE(verify_image(care_map_file.path)); +} + +TEST_F(UpdateVerifierTest, verify_image_protobuf_care_map_bad_ranges) { + // This test relies on dm-verity support. + if (!verity_supported) { + GTEST_LOG_(INFO) << "Test skipped on devices without dm-verity support."; + return; + } + + std::vector> partitions = { + { { "name", "system" }, { "ranges", "3,0,1" } }, + }; + + std::string proto = ConstructProto(partitions); + ASSERT_TRUE(android::base::WriteStringToFile(proto, care_map_file.path)); + ASSERT_FALSE(verify_image(care_map_file.path)); } diff --git a/update_verifier/Android.bp b/update_verifier/Android.bp index f6c7056d9..f4dc1f498 100644 --- a/update_verifier/Android.bp +++ b/update_verifier/Android.bp @@ -33,6 +33,7 @@ cc_library_static { ], srcs: [ + "care_map.proto", "update_verifier.cpp", ], @@ -49,6 +50,11 @@ cc_library_static { "libbase", "libcutils", ], + + proto: { + type: "lite", + export_proto_headers: true, + } } cc_binary { @@ -74,6 +80,7 @@ cc_binary { "libhardware", "libhidlbase", "liblog", + "libprotobuf-cpp-lite", "libutils", ], diff --git a/update_verifier/care_map.proto b/update_verifier/care_map.proto new file mode 100644 index 000000000..442ddd4a9 --- /dev/null +++ b/update_verifier/care_map.proto @@ -0,0 +1,31 @@ +/* + * Copyright (C) 2018 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. + */ + +syntax = "proto3"; + +package UpdateVerifier; +option optimize_for = LITE_RUNTIME; + +message CareMap { + message PartitionInfo { + string name = 1; + string ranges = 2; + string id = 3; + string fingerprint = 4; + } + + repeated PartitionInfo partitions = 1; +} diff --git a/update_verifier/include/update_verifier/update_verifier.h b/update_verifier/include/update_verifier/update_verifier.h index 16b394e98..534384e1d 100644 --- a/update_verifier/include/update_verifier/update_verifier.h +++ b/update_verifier/include/update_verifier/update_verifier.h @@ -20,5 +20,13 @@ int update_verifier(int argc, char** argv); -// Exposed for testing purpose. +// Returns true to indicate a passing verification (or the error should be ignored); Otherwise +// returns false on fatal errors, where we should reject the current boot and trigger a fallback. +// This function tries to process the care_map.txt as protobuf message; and falls back to use the +// plain text format if the parse failed. +// +// Note that update_verifier should be backward compatible to not reject care_map.txt from old +// releases, which could otherwise fail to boot into the new release. For example, we've changed +// the care_map format between N and O. An O update_verifier would fail to work with N care_map.txt. +// This could be a result of sideloading an O OTA while the device having a pending N update. bool verify_image(const std::string& care_map_name); diff --git a/update_verifier/update_verifier.cpp b/update_verifier/update_verifier.cpp index dc7276326..5e5aa1819 100644 --- a/update_verifier/update_verifier.cpp +++ b/update_verifier/update_verifier.cpp @@ -60,6 +60,7 @@ #include #include +#include "care_map.pb.h" #include "otautil/rangeset.h" using android::sp; @@ -189,33 +190,12 @@ static bool read_blocks(const std::string& partition, const std::string& range_s return ret; } -// Returns true to indicate a passing verification (or the error should be ignored); Otherwise -// returns false on fatal errors, where we should reject the current boot and trigger a fallback. -// Note that update_verifier should be backward compatible to not reject care_map.txt from old -// releases, which could otherwise fail to boot into the new release. For example, we've changed -// the care_map format between N and O. An O update_verifier would fail to work with N -// care_map.txt. This could be a result of sideloading an O OTA while the device having a pending N -// update. -bool verify_image(const std::string& care_map_name) { - android::base::unique_fd care_map_fd(TEMP_FAILURE_RETRY(open(care_map_name.c_str(), O_RDONLY))); - // If the device is flashed before the current boot, it may not have care_map.txt - // in /data/ota_package. To allow the device to continue booting in this situation, - // we should print a warning and skip the block verification. - if (care_map_fd.get() == -1) { - PLOG(WARNING) << "Failed to open " << care_map_name; - return true; - } +static bool process_care_map_plain_text(const std::string& care_map_contents) { // care_map file has up to six lines, where every two lines make a pair. Within each pair, the // first line has the partition name (e.g. "system"), while the second line holds the ranges of // all the blocks to verify. - std::string file_content; - if (!android::base::ReadFdToString(care_map_fd.get(), &file_content)) { - LOG(ERROR) << "Error reading care map contents to string."; - return false; - } - - std::vector lines; - lines = android::base::Split(android::base::Trim(file_content), "\n"); + std::vector lines = + android::base::Split(android::base::Trim(care_map_contents), "\n"); if (lines.size() != 2 && lines.size() != 4 && lines.size() != 6) { LOG(ERROR) << "Invalid lines in care_map: found " << lines.size() << " lines, expecting 2 or 4 or 6 lines."; @@ -237,6 +217,50 @@ bool verify_image(const std::string& care_map_name) { return true; } +bool verify_image(const std::string& care_map_name) { + android::base::unique_fd care_map_fd(TEMP_FAILURE_RETRY(open(care_map_name.c_str(), O_RDONLY))); + // If the device is flashed before the current boot, it may not have care_map.txt in + // /data/ota_package. To allow the device to continue booting in this situation, we should + // print a warning and skip the block verification. + if (care_map_fd.get() == -1) { + PLOG(WARNING) << "Failed to open " << care_map_name; + return true; + } + + std::string file_content; + if (!android::base::ReadFdToString(care_map_fd.get(), &file_content)) { + PLOG(ERROR) << "Failed to read " << care_map_name; + return false; + } + + if (file_content.empty()) { + LOG(ERROR) << "Unexpected empty care map"; + return false; + } + + UpdateVerifier::CareMap care_map; + // Falls back to use the plain text version if we cannot parse the file as protobuf message. + if (!care_map.ParseFromString(file_content)) { + return process_care_map_plain_text(file_content); + } + + for (const auto& partition : care_map.partitions()) { + if (partition.name().empty()) { + LOG(ERROR) << "Unexpected empty partition name."; + return false; + } + if (partition.ranges().empty()) { + LOG(ERROR) << "Unexpected block ranges for partition " << partition.name(); + return false; + } + if (!read_blocks(partition.name(), partition.ranges())) { + return false; + } + } + + return true; +} + static int reboot_device() { if (android_reboot(ANDROID_RB_RESTART2, 0, nullptr) == -1) { LOG(ERROR) << "Failed to reboot."; -- cgit v1.2.3 From cfb3c92302356c9078fc175c04be06074a106f91 Mon Sep 17 00:00:00 2001 From: Hridya Valsaraju Date: Thu, 26 Jul 2018 13:04:06 -0700 Subject: Move recovery from /sbin to /system/bin Executables should be in /system/bin rather than sbin. Bug: 78793464 Test: boot into recovery, try adb sideload Change-Id: I194589119a099d29e56b0648f0906a5ae2aa6770 --- Android.mk | 2 +- adb_install.cpp | 2 +- etc/init.rc | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/Android.mk b/Android.mk index 69490a57e..93e658c5a 100644 --- a/Android.mk +++ b/Android.mk @@ -162,7 +162,7 @@ LOCAL_MODULE := recovery LOCAL_FORCE_STATIC_EXECUTABLE := true -LOCAL_MODULE_PATH := $(TARGET_RECOVERY_ROOT_OUT)/sbin +LOCAL_MODULE_PATH := $(TARGET_RECOVERY_ROOT_OUT)/system/bin # Cannot link with LLD: undefined symbol: UsbNoPermissionsLongHelpText # http://b/77543887, lld does not handle -Wl,--gc-sections as well as ld. diff --git a/adb_install.cpp b/adb_install.cpp index 4ee5333c7..97dff221d 100644 --- a/adb_install.cpp +++ b/adb_install.cpp @@ -82,7 +82,7 @@ int apply_from_adb(bool* wipe_cache) { pid_t child; if ((child = fork()) == 0) { - execl("/sbin/recovery", "recovery", "--adbd", nullptr); + execl("/system/bin/recovery", "recovery", "--adbd", nullptr); _exit(EXIT_FAILURE); } diff --git a/etc/init.rc b/etc/init.rc index 96c37b11c..8e18438b5 100644 --- a/etc/init.rc +++ b/etc/init.rc @@ -85,7 +85,7 @@ service charger /charger -r critical seclabel u:r:charger:s0 -service recovery /sbin/recovery +service recovery /system/bin/recovery seclabel u:r:recovery:s0 service adbd /system/bin/adbd --root_seclabel=u:r:su:s0 --device_banner=recovery -- cgit v1.2.3 From 038c4a11db2652bc9a212ab9c375bbb6d1a80a28 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Fri, 27 Jul 2018 11:18:30 -0700 Subject: minadbd: avoid overrriding services_to_fd. Previously, we were relying on linker ordering pulling in minadbd's copy of services_to_fd instead of libadbd's, which breaks when we switch to dynamically linking. Separate out libadbd's services into a separate function that's in a file that isn't built into libadbd, so that we can provide our own here. Bug: http://b/111831478 Test: mma Change-Id: I2479947b2d81db5e750020fffc2c2c770cb31a78 --- minadbd/minadbd_services.cpp | 30 ++++++++---------------------- 1 file changed, 8 insertions(+), 22 deletions(-) diff --git a/minadbd/minadbd_services.cpp b/minadbd/minadbd_services.cpp index 043c51a6a..ab1939e92 100644 --- a/minadbd/minadbd_services.cpp +++ b/minadbd/minadbd_services.cpp @@ -21,15 +21,18 @@ #include #include +#include #include #include #include "adb.h" +#include "adb_unique_fd.h" #include "fdevent.h" #include "fuse_adb_provider.h" +#include "services.h" #include "sysdeps.h" -static void sideload_host_service(int sfd, const std::string& args) { +static void sideload_host_service(unique_fd sfd, const std::string& args) { int file_size; int block_size; if (sscanf(args.c_str(), "%d:%d", &file_size, &block_size) != 2) { @@ -45,22 +48,7 @@ static void sideload_host_service(int sfd, const std::string& args) { exit(result == 0 ? 0 : 1); } -static int create_service_thread(void (*func)(int, const std::string&), const std::string& args) { - int s[2]; - if (adb_socketpair(s)) { - printf("cannot create service socket pair\n"); - return -1; - } - - std::thread([s, func, args]() { func(s[1], args); }).detach(); - - VLOG(SERVICES) << "service thread started, " << s[0] << ":" << s[1]; - return s[0]; -} - -int service_to_fd(const char* name, atransport* /* transport */) { - int ret = -1; - +unique_fd daemon_service_to_fd(const char* name, atransport* /* transport */) { if (!strncmp(name, "sideload:", 9)) { // this exit status causes recovery to print a special error // message saying to use a newer adb (that supports @@ -68,10 +56,8 @@ int service_to_fd(const char* name, atransport* /* transport */) { exit(3); } else if (!strncmp(name, "sideload-host:", 14)) { std::string arg(name + 14); - ret = create_service_thread(sideload_host_service, arg); - } - if (ret >= 0) { - close_on_exec(ret); + return create_service_thread("sideload-host", + std::bind(sideload_host_service, std::placeholders::_1, arg)); } - return ret; + return unique_fd{}; } -- cgit v1.2.3