From 5234ad466c7006430fcd62f0f0ceeb768da5ec93 Mon Sep 17 00:00:00 2001 From: Tao Bao Date: Mon, 23 Sep 2019 10:28:54 -0700 Subject: applypatch: Add backup_source parameter to PatchPartition. And set it to false when installing recovery image via applypatch. We only need to back up the source partition when doing in-place update (e.g. when updating a given partition under recovery). When installing recovery image via applypatch, we won't touch the source partition (i.e. /boot). Removing the backup step also allows dropping the dac_override_allowed permission. Previously it was needed due to the access to /cache. Because applypatch runs as root:root, while /cache is owned by system:cache with 0770. Bug: 68319577 Test: Invoke the code that installs recovery image; check that recovery is installed successfully without denials. Test: recovery_unit_test passes on taimen. Change-Id: I549a770b511762189d6672a2835b6e403d695919 --- applypatch/applypatch.cpp | 16 +++++++++------- applypatch/applypatch_modes.cpp | 2 +- applypatch/include/applypatch/applypatch.h | 7 ++++--- tests/unit/applypatch_test.cpp | 4 ++-- updater/install.cpp | 2 +- 5 files changed, 17 insertions(+), 14 deletions(-) diff --git a/applypatch/applypatch.cpp b/applypatch/applypatch.cpp index 90d8e8604..336860cb9 100644 --- a/applypatch/applypatch.cpp +++ b/applypatch/applypatch.cpp @@ -47,7 +47,7 @@ using namespace std::string_literals; static bool GenerateTarget(const Partition& target, const FileContents& source_file, - const Value& patch, const Value* bonus_data); + const Value& patch, const Value* bonus_data, bool backup_source); bool LoadFileContents(const std::string& filename, FileContents* file) { // No longer allow loading contents from eMMC partitions. @@ -266,7 +266,7 @@ int ShowLicenses() { } bool PatchPartition(const Partition& target, const Partition& source, const Value& patch, - const Value* bonus) { + const Value* bonus, bool backup_source) { LOG(INFO) << "Patching " << target.name; // We try to load and check against the target hash first. @@ -280,7 +280,7 @@ bool PatchPartition(const Partition& target, const Partition& source, const Valu FileContents source_file; if (ReadPartitionToBuffer(source, &source_file, true)) { - return GenerateTarget(target, source_file, patch, bonus); + return GenerateTarget(target, source_file, patch, bonus, backup_source); } LOG(ERROR) << "Failed to find any match"; @@ -326,7 +326,7 @@ bool FlashPartition(const Partition& partition, const std::string& source_filena } static bool GenerateTarget(const Partition& target, const FileContents& source_file, - const Value& patch, const Value* bonus_data) { + const Value& patch, const Value* bonus_data, bool backup_source) { uint8_t expected_sha1[SHA_DIGEST_LENGTH]; if (ParseSha1(target.hash, expected_sha1) != 0) { LOG(ERROR) << "Failed to parse target hash \"" << target.hash << "\""; @@ -351,11 +351,11 @@ static bool GenerateTarget(const Partition& target, const FileContents& source_f } // We write the original source to cache, in case the partition write is interrupted. - if (!CheckAndFreeSpaceOnCache(source_file.data.size())) { + if (backup_source && !CheckAndFreeSpaceOnCache(source_file.data.size())) { LOG(ERROR) << "Not enough free space on /cache"; return false; } - if (!SaveFileContents(Paths::Get().cache_temp_source(), &source_file)) { + if (backup_source && !SaveFileContents(Paths::Get().cache_temp_source(), &source_file)) { LOG(ERROR) << "Failed to back up source file"; return false; } @@ -415,7 +415,9 @@ static bool GenerateTarget(const Partition& target, const FileContents& source_f } // Delete the backup copy of the source. - unlink(Paths::Get().cache_temp_source().c_str()); + if (backup_source) { + unlink(Paths::Get().cache_temp_source().c_str()); + } // Success! return true; diff --git a/applypatch/applypatch_modes.cpp b/applypatch/applypatch_modes.cpp index b46659808..bb5eeae9d 100644 --- a/applypatch/applypatch_modes.cpp +++ b/applypatch/applypatch_modes.cpp @@ -87,7 +87,7 @@ static int PatchMode(const std::string& target_emmc, const std::string& source_e bonus = std::make_unique(Value::Type::BLOB, std::move(bonus_contents)); } - return PatchPartition(target, source, patch, bonus.get()) ? 0 : 1; + return PatchPartition(target, source, patch, bonus.get(), false) ? 0 : 1; } static void Usage() { diff --git a/applypatch/include/applypatch/applypatch.h b/applypatch/include/applypatch/applypatch.h index 6fc6f0fc9..799f4b2d7 100644 --- a/applypatch/include/applypatch/applypatch.h +++ b/applypatch/include/applypatch/applypatch.h @@ -73,10 +73,11 @@ std::ostream& operator<<(std::ostream& os, const Partition& partition); // the 'target' Partition. While patching, it will backup the data on the source partition to // /cache, so that the patching could be resumed on interruption even if both of the source and // target partitions refer to the same device. The function is idempotent if called multiple times. -// An optional arg 'bonus' can be provided, if the patch was generated with a bonus output. -// Returns the patching result. +// 'bonus' can be provided if the patch was generated with a bonus output, or nullptr. +// 'backup_source' indicates whether the source partition should be backed up prior to the update +// (e.g. when doing in-place update). Returns the patching result. bool PatchPartition(const Partition& target, const Partition& source, const Value& patch, - const Value* bonus); + const Value* bonus, bool backup_source); // Returns whether the contents of the eMMC target or the cached file match the embedded hash. // It will look for the backup on /cache if the given partition doesn't match the checksum. diff --git a/tests/unit/applypatch_test.cpp b/tests/unit/applypatch_test.cpp index 794f2c103..218a224f8 100644 --- a/tests/unit/applypatch_test.cpp +++ b/tests/unit/applypatch_test.cpp @@ -141,7 +141,7 @@ TEST_F(ApplyPatchTest, PatchPartition) { ASSERT_TRUE(LoadFileContents(from_testdata_base("bonus.file"), &bonus_fc)); Value bonus(Value::Type::BLOB, std::string(bonus_fc.data.cbegin(), bonus_fc.data.cend())); - ASSERT_TRUE(PatchPartition(target_partition, source_partition, patch, &bonus)); + ASSERT_TRUE(PatchPartition(target_partition, source_partition, patch, &bonus, false)); } // Tests patching an eMMC target without a separate bonus file (i.e. recovery-from-boot patch has @@ -151,7 +151,7 @@ TEST_F(ApplyPatchTest, PatchPartitionWithoutBonusFile) { ASSERT_TRUE(LoadFileContents(from_testdata_base("recovery-from-boot-with-bonus.p"), &patch_fc)); Value patch(Value::Type::BLOB, std::string(patch_fc.data.cbegin(), patch_fc.data.cend())); - ASSERT_TRUE(PatchPartition(target_partition, source_partition, patch, nullptr)); + ASSERT_TRUE(PatchPartition(target_partition, source_partition, patch, nullptr, false)); } class FreeCacheTest : public ::testing::Test { diff --git a/updater/install.cpp b/updater/install.cpp index be0ceb06c..b617f62c1 100644 --- a/updater/install.cpp +++ b/updater/install.cpp @@ -271,7 +271,7 @@ Value* PatchPartitionFn(const char* name, State* state, return StringValue(""); } - bool result = PatchPartition(target, source, *values[0], nullptr); + bool result = PatchPartition(target, source, *values[0], nullptr, true); return StringValue(result ? "t" : ""); } -- cgit v1.2.3