Verify the extents for untouched dynamic partitions during partial update am: 24f960986b
Original change: https://android-review.googlesource.com/c/platform/system/update_engine/+/1354125
Change-Id: I7b7abc50011138b5ed23a266d3477fa8182ea65e
diff --git a/common/dynamic_partition_control_interface.h b/common/dynamic_partition_control_interface.h
index 7289dee..7c2d0b0 100644
--- a/common/dynamic_partition_control_interface.h
+++ b/common/dynamic_partition_control_interface.h
@@ -130,6 +130,13 @@
// the result in |path|. Returns true on success.
// Sample result: /dev/block/by-name/
virtual bool GetDeviceDir(std::string* path) = 0;
+
+ // Verifies that the untouched dynamic partitions in the target metadata have
+ // the same extents as the source metadata.
+ virtual bool VerifyExtentsForUntouchedPartitions(
+ uint32_t source_slot,
+ uint32_t target_slot,
+ const std::vector<std::string>& partitions) = 0;
};
} // namespace chromeos_update_engine
diff --git a/common/dynamic_partition_control_stub.cc b/common/dynamic_partition_control_stub.cc
index cde36af..5a8ca43 100644
--- a/common/dynamic_partition_control_stub.cc
+++ b/common/dynamic_partition_control_stub.cc
@@ -76,4 +76,11 @@
return true;
}
+bool DynamicPartitionControlStub::VerifyExtentsForUntouchedPartitions(
+ uint32_t source_slot,
+ uint32_t target_slot,
+ const std::vector<std::string>& partitions) {
+ return true;
+}
+
} // namespace chromeos_update_engine
diff --git a/common/dynamic_partition_control_stub.h b/common/dynamic_partition_control_stub.h
index 28e3e6a..94dba1b 100644
--- a/common/dynamic_partition_control_stub.h
+++ b/common/dynamic_partition_control_stub.h
@@ -51,6 +51,11 @@
bool ListDynamicPartitionsForSlot(
uint32_t current_slot, std::vector<std::string>* partitions) override;
bool GetDeviceDir(std::string* path) override;
+
+ bool VerifyExtentsForUntouchedPartitions(
+ uint32_t source_slot,
+ uint32_t target_slot,
+ const std::vector<std::string>& partitions) override;
};
} // namespace chromeos_update_engine
diff --git a/dynamic_partition_control_android.cc b/dynamic_partition_control_android.cc
index 6817c21..ba749d9 100644
--- a/dynamic_partition_control_android.cc
+++ b/dynamic_partition_control_android.cc
@@ -293,9 +293,16 @@
std::unique_ptr<MetadataBuilder>
DynamicPartitionControlAndroid::LoadMetadataBuilder(
- const std::string& super_device, uint32_t source_slot) {
- return LoadMetadataBuilder(
- super_device, source_slot, BootControlInterface::kInvalidSlot);
+ const std::string& super_device, uint32_t slot) {
+ auto builder = MetadataBuilder::New(PartitionOpener(), super_device, slot);
+ if (builder == nullptr) {
+ LOG(WARNING) << "No metadata slot " << BootControlInterface::SlotName(slot)
+ << " in " << super_device;
+ return nullptr;
+ }
+ LOG(INFO) << "Loaded metadata from slot "
+ << BootControlInterface::SlotName(slot) << " in " << super_device;
+ return builder;
}
std::unique_ptr<MetadataBuilder>
@@ -303,26 +310,19 @@
const std::string& super_device,
uint32_t source_slot,
uint32_t target_slot) {
- std::unique_ptr<MetadataBuilder> builder;
- if (target_slot == BootControlInterface::kInvalidSlot) {
- builder =
- MetadataBuilder::New(PartitionOpener(), super_device, source_slot);
- } else {
- bool always_keep_source_slot = !target_supports_snapshot_;
- builder = MetadataBuilder::NewForUpdate(PartitionOpener(),
- super_device,
- source_slot,
- target_slot,
- always_keep_source_slot);
- }
-
+ bool always_keep_source_slot = !target_supports_snapshot_;
+ auto builder = MetadataBuilder::NewForUpdate(PartitionOpener(),
+ super_device,
+ source_slot,
+ target_slot,
+ always_keep_source_slot);
if (builder == nullptr) {
LOG(WARNING) << "No metadata slot "
<< BootControlInterface::SlotName(source_slot) << " in "
<< super_device;
return nullptr;
}
- LOG(INFO) << "Loaded metadata from slot "
+ LOG(INFO) << "Created metadata for new update from slot "
<< BootControlInterface::SlotName(source_slot) << " in "
<< super_device;
return builder;
@@ -495,6 +495,7 @@
}
}
+ // TODO(xunchang) support partial update on non VAB enabled devices.
TEST_AND_RETURN_FALSE(PrepareDynamicPartitionsForUpdate(
source_slot, target_slot, manifest, delete_source));
@@ -1113,6 +1114,28 @@
return true;
}
+bool DynamicPartitionControlAndroid::VerifyExtentsForUntouchedPartitions(
+ uint32_t source_slot,
+ uint32_t target_slot,
+ const std::vector<std::string>& partitions) {
+ std::string device_dir_str;
+ TEST_AND_RETURN_FALSE(GetDeviceDir(&device_dir_str));
+ base::FilePath device_dir(device_dir_str);
+
+ auto source_super_device =
+ device_dir.Append(GetSuperPartitionName(source_slot)).value();
+ auto source_builder = LoadMetadataBuilder(source_super_device, source_slot);
+ TEST_AND_RETURN_FALSE(source_builder != nullptr);
+
+ auto target_super_device =
+ device_dir.Append(GetSuperPartitionName(target_slot)).value();
+ auto target_builder = LoadMetadataBuilder(target_super_device, target_slot);
+ TEST_AND_RETURN_FALSE(target_builder != nullptr);
+
+ return MetadataBuilder::VerifyExtentsAgainstSourceMetadata(
+ *source_builder, source_slot, *target_builder, target_slot, partitions);
+}
+
bool DynamicPartitionControlAndroid::ExpectMetadataMounted() {
// No need to mount metadata for non-Virtual A/B devices.
if (!GetVirtualAbFeatureFlag().IsEnabled()) {
diff --git a/dynamic_partition_control_android.h b/dynamic_partition_control_android.h
index 69026a4..08656fd 100644
--- a/dynamic_partition_control_android.h
+++ b/dynamic_partition_control_android.h
@@ -57,6 +57,11 @@
bool ListDynamicPartitionsForSlot(
uint32_t current_slot, std::vector<std::string>* partitions) override;
+ bool VerifyExtentsForUntouchedPartitions(
+ uint32_t source_slot,
+ uint32_t target_slot,
+ const std::vector<std::string>& partitions) override;
+
bool GetDeviceDir(std::string* path) override;
// Return the device for partition |partition_name| at slot |slot|.
@@ -85,16 +90,14 @@
virtual bool UnmapPartitionOnDeviceMapper(
const std::string& target_partition_name);
- // Retrieve metadata from |super_device| at slot |source_slot|.
- //
- // If |target_slot| != kInvalidSlot, before returning the metadata, this
- // function modifies the metadata so that during updates, the metadata can be
- // written to |target_slot|. In particular, on retrofit devices, the returned
- // metadata automatically includes block devices at |target_slot|.
- //
- // If |target_slot| == kInvalidSlot, this function returns metadata at
- // |source_slot| without modifying it. This is the same as
- // LoadMetadataBuilder(const std::string&, uint32_t).
+ // Retrieves metadata from |super_device| at slot |slot|.
+ virtual std::unique_ptr<android::fs_mgr::MetadataBuilder> LoadMetadataBuilder(
+ const std::string& super_device, uint32_t slot);
+
+ // Retrieves metadata from |super_device| at slot |source_slot|. And modifies
+ // the metadata so that during updates, the metadata can be written to
+ // |target_slot|. In particular, on retrofit devices, the returned metadata
+ // automatically includes block devices at |target_slot|.
virtual std::unique_ptr<android::fs_mgr::MetadataBuilder> LoadMetadataBuilder(
const std::string& super_device,
uint32_t source_slot,
@@ -133,10 +136,6 @@
virtual bool GetDmDevicePathByName(const std::string& name,
std::string* path);
- // Retrieve metadata from |super_device| at slot |source_slot|.
- virtual std::unique_ptr<android::fs_mgr::MetadataBuilder> LoadMetadataBuilder(
- const std::string& super_device, uint32_t source_slot);
-
// Return the name of the super partition (which stores super partition
// metadata) for a given slot.
virtual std::string GetSuperPartitionName(uint32_t slot);
diff --git a/dynamic_partition_control_android_unittest.cc b/dynamic_partition_control_android_unittest.cc
index 3738170..4154b36 100644
--- a/dynamic_partition_control_android_unittest.cc
+++ b/dynamic_partition_control_android_unittest.cc
@@ -115,6 +115,14 @@
const PartitionSuffixSizes& sizes,
uint32_t partition_attr = 0) {
EXPECT_CALL(dynamicControl(),
+ LoadMetadataBuilder(GetSuperDevice(slot), slot))
+ .Times(AnyNumber())
+ .WillRepeatedly(Invoke([sizes, partition_attr](auto, auto) {
+ return NewFakeMetadata(PartitionSuffixSizesToManifest(sizes),
+ partition_attr);
+ }));
+
+ EXPECT_CALL(dynamicControl(),
LoadMetadataBuilder(GetSuperDevice(slot), slot, _))
.Times(AnyNumber())
.WillRepeatedly(Invoke([sizes, partition_attr](auto, auto, auto) {
diff --git a/mock_dynamic_partition_control.h b/mock_dynamic_partition_control.h
index 1aaebd8..e85df32 100644
--- a/mock_dynamic_partition_control.h
+++ b/mock_dynamic_partition_control.h
@@ -52,6 +52,10 @@
(override));
MOCK_METHOD(std::unique_ptr<::android::fs_mgr::MetadataBuilder>,
LoadMetadataBuilder,
+ (const std::string&, uint32_t),
+ (override));
+ MOCK_METHOD(std::unique_ptr<::android::fs_mgr::MetadataBuilder>,
+ LoadMetadataBuilder,
(const std::string&, uint32_t, uint32_t),
(override));
MOCK_METHOD(bool,
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index af1baa4..7d837db 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -806,15 +806,32 @@
auto generator = partition_update_generator::Create(boot_control_,
manifest_.block_size());
- std::vector<PartitionUpdate> other_partitions;
+ std::vector<PartitionUpdate> untouched_static_partitions;
TEST_AND_RETURN_FALSE(
generator->GenerateOperationsForPartitionsNotInPayload(
install_plan_->source_slot,
install_plan_->target_slot,
touched_partitions,
- &other_partitions));
- partitions_.insert(
- partitions_.end(), other_partitions.begin(), other_partitions.end());
+ &untouched_static_partitions));
+ partitions_.insert(partitions_.end(),
+ untouched_static_partitions.begin(),
+ untouched_static_partitions.end());
+
+ // Save the untouched dynamic partitions in install plan.
+ std::vector<std::string> dynamic_partitions;
+ if (!boot_control_->GetDynamicPartitionControl()
+ ->ListDynamicPartitionsForSlot(install_plan_->source_slot,
+ &dynamic_partitions)) {
+ LOG(ERROR) << "Failed to load dynamic partitions from slot "
+ << install_plan_->source_slot;
+ return false;
+ }
+ install_plan_->untouched_dynamic_partitions.clear();
+ for (const auto& name : dynamic_partitions) {
+ if (touched_partitions.find(name) == touched_partitions.end()) {
+ install_plan_->untouched_dynamic_partitions.push_back(name);
+ }
+ }
}
// Fill in the InstallPlan::partitions based on the partitions from the
diff --git a/payload_consumer/filesystem_verifier_action.cc b/payload_consumer/filesystem_verifier_action.cc
index 5b2b5f4..61917ea 100644
--- a/payload_consumer/filesystem_verifier_action.cc
+++ b/payload_consumer/filesystem_verifier_action.cc
@@ -28,6 +28,7 @@
#include <base/bind.h>
#include <brillo/data_encoding.h>
#include <brillo/streams/file_stream.h>
+#include <base/strings/string_util.h>
#include "update_engine/common/utils.h"
@@ -89,6 +90,20 @@
void FilesystemVerifierAction::StartPartitionHashing() {
if (partition_index_ == install_plan_.partitions.size()) {
+ if (!install_plan_.untouched_dynamic_partitions.empty()) {
+ LOG(INFO) << "Verifying extents of untouched dynamic partitions ["
+ << base::JoinString(install_plan_.untouched_dynamic_partitions,
+ ", ")
+ << "]";
+ if (!dynamic_control_->VerifyExtentsForUntouchedPartitions(
+ install_plan_.source_slot,
+ install_plan_.target_slot,
+ install_plan_.untouched_dynamic_partitions)) {
+ Cleanup(ErrorCode::kFilesystemVerifierError);
+ return;
+ }
+ }
+
Cleanup(ErrorCode::kSuccess);
return;
}
diff --git a/payload_consumer/filesystem_verifier_action.h b/payload_consumer/filesystem_verifier_action.h
index 7d179df..6a8823a 100644
--- a/payload_consumer/filesystem_verifier_action.h
+++ b/payload_consumer/filesystem_verifier_action.h
@@ -57,8 +57,13 @@
class FilesystemVerifierAction : public InstallPlanAction {
public:
- FilesystemVerifierAction()
- : verity_writer_(verity_writer::CreateVerityWriter()) {}
+ explicit FilesystemVerifierAction(
+ DynamicPartitionControlInterface* dynamic_control)
+ : verity_writer_(verity_writer::CreateVerityWriter()),
+ dynamic_control_(dynamic_control) {
+ CHECK(dynamic_control_);
+ }
+
~FilesystemVerifierAction() override = default;
void PerformAction() override;
@@ -123,6 +128,9 @@
// Write verity data of the current partition.
std::unique_ptr<VerityWriterInterface> verity_writer_;
+ // Verifies the untouched dynamic partitions for partial updates.
+ DynamicPartitionControlInterface* dynamic_control_{nullptr};
+
// Reads and hashes this many bytes from the head of the input stream. When
// the partition starts to be hashed, this field is initialized from the
// corresponding InstallPlan::Partition size which is the total size
diff --git a/payload_consumer/filesystem_verifier_action_unittest.cc b/payload_consumer/filesystem_verifier_action_unittest.cc
index cb33404..2971849 100644
--- a/payload_consumer/filesystem_verifier_action_unittest.cc
+++ b/payload_consumer/filesystem_verifier_action_unittest.cc
@@ -27,6 +27,7 @@
#include <brillo/secure_blob.h>
#include <gtest/gtest.h>
+#include "update_engine/common/dynamic_partition_control_stub.h"
#include "update_engine/common/hash_calculator.h"
#include "update_engine/common/test_utils.h"
#include "update_engine/common/utils.h"
@@ -51,6 +52,7 @@
brillo::FakeMessageLoop loop_{nullptr};
ActionProcessor processor_;
+ DynamicPartitionControlStub dynamic_control_stub_;
};
class FilesystemVerifierActionTestDelegate : public ActionProcessorDelegate {
@@ -188,7 +190,8 @@
void FilesystemVerifierActionTest::BuildActions(
const InstallPlan& install_plan) {
auto feeder_action = std::make_unique<ObjectFeederAction<InstallPlan>>();
- auto verifier_action = std::make_unique<FilesystemVerifierAction>();
+ auto verifier_action =
+ std::make_unique<FilesystemVerifierAction>(&dynamic_control_stub_);
auto collector_action =
std::make_unique<ObjectCollectorAction<InstallPlan>>();
@@ -217,7 +220,8 @@
};
TEST_F(FilesystemVerifierActionTest, MissingInputObjectTest) {
- auto copier_action = std::make_unique<FilesystemVerifierAction>();
+ auto copier_action =
+ std::make_unique<FilesystemVerifierAction>(&dynamic_control_stub_);
auto collector_action =
std::make_unique<ObjectCollectorAction<InstallPlan>>();
diff --git a/payload_consumer/install_plan.h b/payload_consumer/install_plan.h
index 63178bd..f04c650 100644
--- a/payload_consumer/install_plan.h
+++ b/payload_consumer/install_plan.h
@@ -158,6 +158,10 @@
// If not blank, a base-64 encoded representation of the PEM-encoded
// public key in the response.
std::string public_key_rsa;
+
+ // The name of dynamic partitions not included in the payload. Only used
+ // for partial updates.
+ std::vector<std::string> untouched_dynamic_partitions;
};
class InstallPlanAction;
diff --git a/payload_consumer/partition_update_generator_android.cc b/payload_consumer/partition_update_generator_android.cc
index aa3f2e5..5768dd6 100644
--- a/payload_consumer/partition_update_generator_android.cc
+++ b/payload_consumer/partition_update_generator_android.cc
@@ -50,25 +50,14 @@
BootControlInterface::Slot target_slot,
const std::set<std::string>& partitions_in_payload,
std::vector<PartitionUpdate>* update_list) {
- auto ret = GetStaticAbPartitionsOnDevice();
- if (!ret.has_value()) {
+ auto ab_partitions = GetStaticAbPartitionsOnDevice();
+ if (!ab_partitions.has_value()) {
LOG(ERROR) << "Failed to load static a/b partitions";
return false;
}
- auto ab_partitions = ret.value();
-
- // Add the dynamic partitions.
- auto dynamic_control = boot_control_->GetDynamicPartitionControl();
- std::vector<std::string> dynamic_partitions;
- if (!dynamic_control->ListDynamicPartitionsForSlot(source_slot,
- &dynamic_partitions)) {
- LOG(ERROR) << "Failed to load dynamic partitions from slot " << source_slot;
- return false;
- }
- ab_partitions.insert(dynamic_partitions.begin(), dynamic_partitions.end());
std::vector<PartitionUpdate> partition_updates;
- for (const auto& partition_name : ab_partitions) {
+ for (const auto& partition_name : ab_partitions.value()) {
if (partitions_in_payload.find(partition_name) !=
partitions_in_payload.end()) {
LOG(INFO) << partition_name << " has included in payload";
@@ -159,13 +148,15 @@
return std::nullopt;
}
- if (is_source_dynamic != is_target_dynamic) {
- LOG(ERROR) << "Source slot " << source_slot << " for partition "
- << partition_name << " is " << (is_source_dynamic ? "" : "not")
- << " dynamic, but target slot " << target_slot << " is "
+ if (is_source_dynamic || is_target_dynamic) {
+ LOG(ERROR) << "Partition " << partition_name << " is expected to be a"
+ << " static partition. source slot is "
+ << (is_source_dynamic ? "" : "not")
+ << " dynamic, and target slot " << target_slot << " is "
<< (is_target_dynamic ? "" : "not") << " dynamic.";
return std::nullopt;
}
+
auto source_size = utils::FileSize(source_device);
auto target_size = utils::FileSize(target_device);
if (source_size == -1 || target_size == -1 || source_size != target_size ||
@@ -175,11 +166,8 @@
return std::nullopt;
}
- return CreatePartitionUpdate(partition_name,
- source_device,
- target_device,
- source_size,
- is_source_dynamic);
+ return CreatePartitionUpdate(
+ partition_name, source_device, target_device, source_size);
}
std::optional<PartitionUpdate>
@@ -187,8 +175,7 @@
const std::string& partition_name,
const std::string& source_device,
const std::string& target_device,
- int64_t partition_size,
- bool is_dynamic) {
+ int64_t partition_size) {
PartitionUpdate partition_update;
partition_update.set_partition_name(partition_name);
auto old_partition_info = partition_update.mutable_old_partition_info();
@@ -202,18 +189,15 @@
auto new_partition_info = partition_update.mutable_new_partition_info();
new_partition_info->set_size(partition_size);
new_partition_info->set_hash(raw_hash->data(), raw_hash->size());
- // TODO(xunchang) TBD, should we skip hashing and verification of the
- // dynamic partitions not in payload?
- if (!is_dynamic) {
- auto copy_operation = partition_update.add_operations();
- copy_operation->set_type(InstallOperation::SOURCE_COPY);
- Extent copy_extent;
- copy_extent.set_start_block(0);
- copy_extent.set_num_blocks(partition_size / block_size_);
- *copy_operation->add_src_extents() = copy_extent;
- *copy_operation->add_dst_extents() = copy_extent;
- }
+ auto copy_operation = partition_update.add_operations();
+ copy_operation->set_type(InstallOperation::SOURCE_COPY);
+ Extent copy_extent;
+ copy_extent.set_start_block(0);
+ copy_extent.set_num_blocks(partition_size / block_size_);
+
+ *copy_operation->add_src_extents() = copy_extent;
+ *copy_operation->add_dst_extents() = copy_extent;
return partition_update;
}
diff --git a/payload_consumer/partition_update_generator_android.h b/payload_consumer/partition_update_generator_android.h
index 8f33077..97b7d83 100644
--- a/payload_consumer/partition_update_generator_android.h
+++ b/payload_consumer/partition_update_generator_android.h
@@ -56,8 +56,7 @@
const std::string& partition_name,
const std::string& source_device,
const std::string& target_device,
- int64_t partition_size,
- bool is_dynamic);
+ int64_t partition_size);
std::optional<PartitionUpdate> CreatePartitionUpdate(
const std::string& partition_name,
diff --git a/payload_generator/generate_delta_main.cc b/payload_generator/generate_delta_main.cc
index f7df211..eb00333 100644
--- a/payload_generator/generate_delta_main.cc
+++ b/payload_generator/generate_delta_main.cc
@@ -252,8 +252,8 @@
nullptr,
new FileFetcher(),
true /* interactive */);
- auto filesystem_verifier_action =
- std::make_unique<FilesystemVerifierAction>();
+ auto filesystem_verifier_action = std::make_unique<FilesystemVerifierAction>(
+ fake_boot_control.GetDynamicPartitionControl());
BondActions(install_plan_action.get(), download_action.get());
BondActions(download_action.get(), filesystem_verifier_action.get());
diff --git a/update_attempter.cc b/update_attempter.cc
index 60c2c36..f37973e 100644
--- a/update_attempter.cc
+++ b/update_attempter.cc
@@ -818,8 +818,8 @@
system_state_->hardware()),
false,
session_id_);
- auto filesystem_verifier_action =
- std::make_unique<FilesystemVerifierAction>();
+ auto filesystem_verifier_action = std::make_unique<FilesystemVerifierAction>(
+ system_state_->boot_control()->GetDynamicPartitionControl());
auto update_complete_action = std::make_unique<OmahaRequestAction>(
system_state_,
new OmahaEvent(OmahaEvent::kTypeUpdateComplete),
diff --git a/update_attempter_android.cc b/update_attempter_android.cc
index a554d38..7fc13e1 100644
--- a/update_attempter_android.cc
+++ b/update_attempter_android.cc
@@ -738,8 +738,8 @@
true /* interactive */);
download_action->set_delegate(this);
download_action->set_base_offset(base_offset_);
- auto filesystem_verifier_action =
- std::make_unique<FilesystemVerifierAction>();
+ auto filesystem_verifier_action = std::make_unique<FilesystemVerifierAction>(
+ boot_control_->GetDynamicPartitionControl());
auto postinstall_runner_action =
std::make_unique<PostinstallRunnerAction>(boot_control_, hardware_);
filesystem_verifier_action->set_delegate(this);
diff --git a/update_attempter_unittest.cc b/update_attempter_unittest.cc
index 0086dd5..305dbdb 100644
--- a/update_attempter_unittest.cc
+++ b/update_attempter_unittest.cc
@@ -663,7 +663,8 @@
EXPECT_EQ(
ErrorCode::kOmahaResponseHandlerError,
GetErrorCodeForAction(&omaha_response_handler_action, ErrorCode::kError));
- FilesystemVerifierAction filesystem_verifier_action;
+ DynamicPartitionControlStub dynamic_control_stub;
+ FilesystemVerifierAction filesystem_verifier_action(&dynamic_control_stub);
EXPECT_EQ(
ErrorCode::kFilesystemVerifierError,
GetErrorCodeForAction(&filesystem_verifier_action, ErrorCode::kError));