Pass in source slot to ctor of dynamic control
When DynamicPartitionControlAndroid is constructed, it initializes both
source and target slot to -1. These values get updated during
PreparePartitionsForUpdate call. And we only
PreparePartitionsForUpdate() when applying an OTA or applocating space
for an OTA(not when verifying OTA metadata). Which means if
VerifyPayloadApplicable() is called before any call two other APIs, we
could be using an "Uninitialiazed" dynamic partition control.
To mitigate this problem, we pass in source_slot at ctor of
DynamicPartitionControl, also make IsDynamicPartition() api take in a
slot number to avoid reading uninitialized member fields.
Bug: 181643302
Test: apply an OTA, abort, restart update_engine, verify a payload
Change-Id: I9a8a0fe8a9aca48e91241e15bdec33a1c1228553
diff --git a/aosp/boot_control_android.cc b/aosp/boot_control_android.cc
index 3b20fc2..c1ac0d4 100644
--- a/aosp/boot_control_android.cc
+++ b/aosp/boot_control_android.cc
@@ -67,7 +67,8 @@
LOG(INFO) << "Loaded boot control hidl hal.";
- dynamic_control_ = std::make_unique<DynamicPartitionControlAndroid>();
+ dynamic_control_ =
+ std::make_unique<DynamicPartitionControlAndroid>(GetCurrentSlot());
return true;
}
diff --git a/aosp/dynamic_partition_control_android.cc b/aosp/dynamic_partition_control_android.cc
index aba6deb..4220445 100644
--- a/aosp/dynamic_partition_control_android.cc
+++ b/aosp/dynamic_partition_control_android.cc
@@ -116,12 +116,14 @@
return FeatureFlag(FeatureFlag::Value::NONE);
}
-DynamicPartitionControlAndroid::DynamicPartitionControlAndroid()
+DynamicPartitionControlAndroid::DynamicPartitionControlAndroid(
+ uint32_t source_slot)
: dynamic_partitions_(
GetFeatureFlag(kUseDynamicPartitions, kRetrfoitDynamicPartitions)),
virtual_ab_(GetFeatureFlag(kVirtualAbEnabled, kVirtualAbRetrofit)),
virtual_ab_compression_(GetFeatureFlag(kVirtualAbCompressionEnabled,
- kVirtualAbCompressionRetrofit)) {
+ kVirtualAbCompressionRetrofit)),
+ source_slot_(source_slot) {
if (GetVirtualAbFeatureFlag().IsEnabled()) {
snapshot_ = SnapshotManager::New();
} else {
@@ -1019,8 +1021,8 @@
// target slot.
const auto& partition_name_suffix =
partition_name + SlotSuffixForSlotNumber(slot);
- if (UpdateUsesSnapshotCompression() && IsDynamicPartition(partition_name) &&
- slot != current_slot) {
+ if (UpdateUsesSnapshotCompression() && slot != current_slot &&
+ IsDynamicPartition(partition_name, slot)) {
return {
{.mountable_device_path = base::FilePath{std::string{VABC_DEVICE_DIR}}
.Append(partition_name_suffix)
@@ -1211,6 +1213,14 @@
uint32_t slot,
uint32_t current_slot,
std::vector<std::string>* partitions) {
+ CHECK(slot == source_slot_ || target_slot_ != UINT32_MAX)
+ << " source slot: " << source_slot_ << " target slot: " << target_slot_
+ << " slot: " << slot
+ << " attempting to query dynamic partition metadata for target slot "
+ "before PreparePartitionForUpdate() is called. The "
+ "metadata in target slot isn't valid until "
+ "PreparePartitionForUpdate() is called, contining execution would "
+ "likely cause problems.";
bool slot_enables_dynamic_partitions =
GetDynamicPartitionsFeatureFlag().IsEnabled();
// Check if the target slot has dynamic partitions, this may happen when
@@ -1347,16 +1357,22 @@
}
bool DynamicPartitionControlAndroid::IsDynamicPartition(
- const std::string& partition_name) {
- if (dynamic_partition_list_.empty() &&
+ const std::string& partition_name, uint32_t slot) {
+ if (slot >= dynamic_partition_list_.size()) {
+ LOG(ERROR) << "Seeing unexpected slot # " << slot << " currently assuming "
+ << dynamic_partition_list_.size() << " slots";
+ return false;
+ }
+ auto& dynamic_partition_list = dynamic_partition_list_[slot];
+ if (dynamic_partition_list.empty() &&
GetDynamicPartitionsFeatureFlag().IsEnabled()) {
// Use the DAP config of the target slot.
CHECK(ListDynamicPartitionsForSlot(
- target_slot_, source_slot_, &dynamic_partition_list_));
+ slot, source_slot_, &dynamic_partition_list));
}
- return std::find(dynamic_partition_list_.begin(),
- dynamic_partition_list_.end(),
- partition_name) != dynamic_partition_list_.end();
+ return std::find(dynamic_partition_list.begin(),
+ dynamic_partition_list.end(),
+ partition_name) != dynamic_partition_list.end();
}
bool DynamicPartitionControlAndroid::UpdateUsesSnapshotCompression() {
diff --git a/aosp/dynamic_partition_control_android.h b/aosp/dynamic_partition_control_android.h
index a23827b..4e75a9b 100644
--- a/aosp/dynamic_partition_control_android.h
+++ b/aosp/dynamic_partition_control_android.h
@@ -38,7 +38,7 @@
// Per earlier discussion with VAB team, this directory is unlikely to change.
// So we declare it as a constant here.
static constexpr std::string_view VABC_DEVICE_DIR = "/dev/block/mapper/";
- DynamicPartitionControlAndroid();
+ explicit DynamicPartitionControlAndroid(uint32_t source_slot);
~DynamicPartitionControlAndroid();
FeatureFlag GetDynamicPartitionsFeatureFlag() override;
@@ -110,7 +110,7 @@
bool UnmapAllPartitions() override;
- bool IsDynamicPartition(const std::string& part_name) override;
+ bool IsDynamicPartition(const std::string& part_name, uint32_t slot) override;
bool UpdateUsesSnapshotCompression() override;
@@ -329,7 +329,7 @@
uint32_t source_slot_ = UINT32_MAX;
uint32_t target_slot_ = UINT32_MAX;
- std::vector<std::string> dynamic_partition_list_;
+ std::vector<std::vector<std::string>> dynamic_partition_list_{2UL};
DISALLOW_COPY_AND_ASSIGN(DynamicPartitionControlAndroid);
};
diff --git a/aosp/dynamic_partition_control_android_unittest.cc b/aosp/dynamic_partition_control_android_unittest.cc
index 4a12b83..eb3f60c 100644
--- a/aosp/dynamic_partition_control_android_unittest.cc
+++ b/aosp/dynamic_partition_control_android_unittest.cc
@@ -402,7 +402,8 @@
.WillByDefault(Return(FeatureFlag(FeatureFlag::Value::NONE)));
ON_CALL(dynamicControl(), UpdateUsesSnapshotCompression())
.WillByDefault(Return(false));
- ON_CALL(dynamicControl(), IsDynamicPartition(_)).WillByDefault(Return(true));
+ ON_CALL(dynamicControl(), IsDynamicPartition(_, _))
+ .WillByDefault(Return(true));
EXPECT_CALL(dynamicControl(),
DeviceExists(AnyOf(GetDevice(S("vendor")),
@@ -442,7 +443,7 @@
.WillByDefault(Return(FeatureFlag(FeatureFlag::Value::LAUNCH)));
ON_CALL(dynamicControl(), UpdateUsesSnapshotCompression())
.WillByDefault(Return(true));
- EXPECT_CALL(dynamicControl(), IsDynamicPartition(_))
+ EXPECT_CALL(dynamicControl(), IsDynamicPartition(_, _))
.Times(AtLeast(1))
.WillRepeatedly(Return(true));
diff --git a/aosp/mock_dynamic_partition_control_android.h b/aosp/mock_dynamic_partition_control_android.h
index 682ddfd..d80dfb5 100644
--- a/aosp/mock_dynamic_partition_control_android.h
+++ b/aosp/mock_dynamic_partition_control_android.h
@@ -35,6 +35,8 @@
class MockDynamicPartitionControlAndroid
: public DynamicPartitionControlAndroid {
public:
+ MockDynamicPartitionControlAndroid()
+ : DynamicPartitionControlAndroid(0 /*source slot*/) {}
MOCK_METHOD(
bool,
MapPartitionOnDeviceMapper,
@@ -100,7 +102,10 @@
(override));
MOCK_METHOD(bool, MapAllPartitions, (), (override));
MOCK_METHOD(bool, UnmapAllPartitions, (), (override));
- MOCK_METHOD(bool, IsDynamicPartition, (const std::string&), (override));
+ MOCK_METHOD(bool,
+ IsDynamicPartition,
+ (const std::string&, uint32_t slot),
+ (override));
MOCK_METHOD(bool, UpdateUsesSnapshotCompression, (), (override));
void set_fake_mapped_devices(const std::set<std::string>& fake) override {
diff --git a/common/dynamic_partition_control_interface.h b/common/dynamic_partition_control_interface.h
index 8b29c3b..da27932 100644
--- a/common/dynamic_partition_control_interface.h
+++ b/common/dynamic_partition_control_interface.h
@@ -170,7 +170,8 @@
const std::optional<std::string>&,
bool is_append = false) = 0;
- virtual bool IsDynamicPartition(const std::string& part_name) = 0;
+ virtual bool IsDynamicPartition(const std::string& part_name,
+ uint32_t slot) = 0;
// Create virtual block devices for all partitions.
virtual bool MapAllPartitions() = 0;
diff --git a/common/dynamic_partition_control_stub.cc b/common/dynamic_partition_control_stub.cc
index 7d0ef18..05803fe 100644
--- a/common/dynamic_partition_control_stub.cc
+++ b/common/dynamic_partition_control_stub.cc
@@ -114,7 +114,7 @@
}
bool DynamicPartitionControlStub::IsDynamicPartition(
- const std::string& part_name) {
+ const std::string& part_name, uint32_t slot) {
return false;
}
diff --git a/common/dynamic_partition_control_stub.h b/common/dynamic_partition_control_stub.h
index 5ab82f5..eb7154c 100644
--- a/common/dynamic_partition_control_stub.h
+++ b/common/dynamic_partition_control_stub.h
@@ -71,7 +71,7 @@
bool MapAllPartitions() override;
bool UnmapAllPartitions() override;
- bool IsDynamicPartition(const std::string& part_name) override;
+ bool IsDynamicPartition(const std::string& part_name, uint32_t slot) override;
bool UpdateUsesSnapshotCompression() override;
};
} // namespace chromeos_update_engine
diff --git a/common/mock_dynamic_partition_control.h b/common/mock_dynamic_partition_control.h
index 74f4efc..391d3eb 100644
--- a/common/mock_dynamic_partition_control.h
+++ b/common/mock_dynamic_partition_control.h
@@ -79,7 +79,10 @@
VerifyExtentsForUntouchedPartitions,
(uint32_t, uint32_t, const std::vector<std::string>&),
(override));
- MOCK_METHOD(bool, IsDynamicPartition, (const std::string&), (override));
+ MOCK_METHOD(bool,
+ IsDynamicPartition,
+ (const std::string&, uint32_t slot),
+ (override));
MOCK_METHOD(bool, UpdateUsesSnapshotCompression, (), (override));
};
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index f26dd48..82e589e 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -240,13 +240,13 @@
const InstallPlan::Partition& install_part =
install_plan_->partitions[num_previous_partitions + current_partition_];
auto dynamic_control = boot_control_->GetDynamicPartitionControl();
- partition_writer_ =
- CreatePartitionWriter(partition,
- install_part,
- dynamic_control,
- block_size_,
- interactive_,
- IsDynamicPartition(install_part.name));
+ partition_writer_ = CreatePartitionWriter(
+ partition,
+ install_part,
+ dynamic_control,
+ block_size_,
+ interactive_,
+ IsDynamicPartition(install_part.name, install_plan_->target_slot));
// Open source fds if we have a delta payload, or for partitions in the
// partial update.
bool source_may_exist = manifest_.partial_update() ||
@@ -1520,9 +1520,10 @@
return true;
}
-bool DeltaPerformer::IsDynamicPartition(const std::string& part_name) {
+bool DeltaPerformer::IsDynamicPartition(const std::string& part_name,
+ uint32_t slot) {
return boot_control_->GetDynamicPartitionControl()->IsDynamicPartition(
- part_name);
+ part_name, slot);
}
std::unique_ptr<PartitionWriter> DeltaPerformer::CreatePartitionWriter(
@@ -1538,7 +1539,7 @@
dynamic_control,
block_size_,
interactive_,
- IsDynamicPartition(install_part.name));
+ IsDynamicPartition(install_part.name, install_plan_->target_slot));
}
} // namespace chromeos_update_engine
diff --git a/payload_consumer/delta_performer.h b/payload_consumer/delta_performer.h
index 31fa6b2..c54316b 100644
--- a/payload_consumer/delta_performer.h
+++ b/payload_consumer/delta_performer.h
@@ -325,7 +325,7 @@
ErrorCode CheckTimestampError() const;
// Check if partition `part_name` is a dynamic partition.
- bool IsDynamicPartition(const std::string& part_name);
+ bool IsDynamicPartition(const std::string& part_name, uint32_t slot);
// Update Engine preference store.
PrefsInterface* prefs_;
diff --git a/payload_consumer/filesystem_verifier_action.cc b/payload_consumer/filesystem_verifier_action.cc
index e3a3e34..9221caa 100644
--- a/payload_consumer/filesystem_verifier_action.cc
+++ b/payload_consumer/filesystem_verifier_action.cc
@@ -168,8 +168,9 @@
<< partition.name << ") on device " << part_path;
auto success = false;
if (dynamic_control_->UpdateUsesSnapshotCompression() &&
- dynamic_control_->IsDynamicPartition(partition.name) &&
- verifier_step_ == VerifierStep::kVerifyTargetHash) {
+ verifier_step_ == VerifierStep::kVerifyTargetHash &&
+ dynamic_control_->IsDynamicPartition(partition.name,
+ install_plan_.target_slot)) {
success = InitializeFdVABC();
} else {
if (part_path.empty()) {
diff --git a/payload_consumer/filesystem_verifier_action_unittest.cc b/payload_consumer/filesystem_verifier_action_unittest.cc
index d163ec2..f618626 100644
--- a/payload_consumer/filesystem_verifier_action_unittest.cc
+++ b/payload_consumer/filesystem_verifier_action_unittest.cc
@@ -419,7 +419,7 @@
.WillByDefault(Return(true));
ON_CALL(dynamic_control, OpenCowReader(_, _, _))
.WillByDefault(Return(nullptr));
- ON_CALL(dynamic_control, IsDynamicPartition(part.name))
+ ON_CALL(dynamic_control, IsDynamicPartition(part.name, _))
.WillByDefault(Return(true));
EXPECT_CALL(dynamic_control, UpdateUsesSnapshotCompression())