Allow creation of a logical partition
- InstallPlan: allow source partition to be missing if size == 0
- DeltaPerformer: don't open source partition if size == 0
Also:
- DeltaPerformer: test for source_fd_ before using it
(to avoid segfault)
- Fix DeltaPerformerTest to generate payloads with correct
source sizes and paths. Previously, this works because source_path
is always set even when source_size = 0.
Bug: 124958572
Test: python system/update_engine/scripts/update_device.py <ota-img>
Test: update_engine_unittests
Change-Id: I53bda1d12accdf91e7fa616be5da562e189038d5
diff --git a/payload_consumer/delta_performer.cc b/payload_consumer/delta_performer.cc
index 489c821..ae73d03 100644
--- a/payload_consumer/delta_performer.cc
+++ b/payload_consumer/delta_performer.cc
@@ -358,7 +358,10 @@
install_plan_->partitions[num_previous_partitions + current_partition_];
// Open source fds if we have a delta payload with minor version >= 2.
if (payload_->type == InstallPayloadType::kDelta &&
- GetMinorVersion() != kInPlaceMinorPayloadVersion) {
+ GetMinorVersion() != kInPlaceMinorPayloadVersion &&
+ // With dynamic partitions we could create a new partition in a
+ // delta payload, and we shouldn't open source partition in that case.
+ install_part.source_size > 0) {
source_path_ = install_part.source_path;
int err;
source_fd_ = OpenFile(source_path_.c_str(), O_RDONLY, false, &err);
@@ -1167,6 +1170,8 @@
if (operation.has_dst_length())
TEST_AND_RETURN_FALSE(operation.dst_length() % block_size_ == 0);
+ TEST_AND_RETURN_FALSE(source_fd_ != nullptr);
+
if (operation.has_src_sha256_hash()) {
brillo::Blob source_hash;
brillo::Blob expected_source_hash(operation.src_sha256_hash().begin(),
@@ -1236,6 +1241,11 @@
FileDescriptorPtr DeltaPerformer::ChooseSourceFD(
const InstallOperation& operation, ErrorCode* error) {
+ if (source_fd_ == nullptr) {
+ LOG(ERROR) << "ChooseSourceFD fail: source_fd_ == nullptr";
+ return nullptr;
+ }
+
if (!operation.has_src_sha256_hash()) {
// When the operation doesn't include a source hash, we attempt the error
// corrected device first since we can't verify the block in the raw device
diff --git a/payload_consumer/delta_performer_unittest.cc b/payload_consumer/delta_performer_unittest.cc
index 61b58ed..b7a38cc 100644
--- a/payload_consumer/delta_performer_unittest.cc
+++ b/payload_consumer/delta_performer_unittest.cc
@@ -177,19 +177,22 @@
brillo::Blob GeneratePayload(const brillo::Blob& blob_data,
const vector<AnnotatedOperation>& aops,
- bool sign_payload) {
+ bool sign_payload,
+ PartitionConfig* old_part = nullptr) {
return GeneratePayload(blob_data,
aops,
sign_payload,
kMaxSupportedMajorPayloadVersion,
- kMaxSupportedMinorPayloadVersion);
+ kMaxSupportedMinorPayloadVersion,
+ old_part);
}
brillo::Blob GeneratePayload(const brillo::Blob& blob_data,
const vector<AnnotatedOperation>& aops,
bool sign_payload,
uint64_t major_version,
- uint32_t minor_version) {
+ uint32_t minor_version,
+ PartitionConfig* old_part = nullptr) {
test_utils::ScopedTempFile blob_file("Blob-XXXXXX");
EXPECT_TRUE(test_utils::WriteFileVector(blob_file.path(), blob_data));
@@ -200,24 +203,29 @@
PayloadFile payload;
EXPECT_TRUE(payload.Init(config));
- PartitionConfig old_part(kPartitionNameRoot);
+ std::unique_ptr<PartitionConfig> old_part_uptr;
+ if (!old_part) {
+ old_part_uptr = std::make_unique<PartitionConfig>(kPartitionNameRoot);
+ old_part = old_part_uptr.get();
+ }
if (minor_version != kFullPayloadMinorVersion) {
// When generating a delta payload we need to include the old partition
// information to mark it as a delta payload.
- old_part.path = "/dev/null";
- old_part.size = 0;
+ if (old_part->path.empty()) {
+ old_part->path = "/dev/null";
+ }
}
PartitionConfig new_part(kPartitionNameRoot);
new_part.path = "/dev/zero";
new_part.size = 1234;
- payload.AddPartition(old_part, new_part, aops);
+ payload.AddPartition(*old_part, new_part, aops);
// We include a kernel partition without operations.
- old_part.name = kPartitionNameKernel;
+ old_part->name = kPartitionNameKernel;
new_part.name = kPartitionNameKernel;
new_part.size = 0;
- payload.AddPartition(old_part, new_part, {});
+ payload.AddPartition(*old_part, new_part, {});
test_utils::ScopedTempFile payload_file("Payload-XXXXXX");
string private_key =
@@ -233,7 +241,8 @@
}
brillo::Blob GenerateSourceCopyPayload(const brillo::Blob& copied_data,
- bool add_hash) {
+ bool add_hash,
+ PartitionConfig* old_part = nullptr) {
PayloadGenerationConfig config;
const uint64_t kDefaultBlockSize = config.block_size;
EXPECT_EQ(0U, copied_data.size() % kDefaultBlockSize);
@@ -247,7 +256,7 @@
if (add_hash)
aop.op.set_src_sha256_hash(src_hash.data(), src_hash.size());
- return GeneratePayload(brillo::Blob(), {aop}, false);
+ return GeneratePayload(brillo::Blob(), {aop}, false, old_part);
}
// Apply |payload_data| on partition specified in |source_path|.
@@ -571,11 +580,16 @@
EXPECT_TRUE(HashCalculator::RawHashOfData(expected_data, &src_hash));
aop.op.set_src_sha256_hash(src_hash.data(), src_hash.size());
- brillo::Blob payload_data = GeneratePayload(brillo::Blob(), {aop}, false);
-
test_utils::ScopedTempFile source("Source-XXXXXX");
EXPECT_TRUE(test_utils::WriteFileVector(source.path(), expected_data));
+ PartitionConfig old_part(kPartitionNameRoot);
+ old_part.path = source.path();
+ old_part.size = expected_data.size();
+
+ brillo::Blob payload_data =
+ GeneratePayload(brillo::Blob(), {aop}, false, &old_part);
+
EXPECT_EQ(expected_data, ApplyPayload(payload_data, source.path(), true));
}
@@ -594,11 +608,16 @@
EXPECT_TRUE(HashCalculator::RawHashOfData(src, &src_hash));
aop.op.set_src_sha256_hash(src_hash.data(), src_hash.size());
- brillo::Blob payload_data = GeneratePayload(puffdiff_payload, {aop}, false);
-
test_utils::ScopedTempFile source("Source-XXXXXX");
EXPECT_TRUE(test_utils::WriteFileVector(source.path(), src));
+ PartitionConfig old_part(kPartitionNameRoot);
+ old_part.path = source.path();
+ old_part.size = src.size();
+
+ brillo::Blob payload_data =
+ GeneratePayload(puffdiff_payload, {aop}, false, &old_part);
+
brillo::Blob dst(std::begin(dst_deflates), std::end(dst_deflates));
EXPECT_EQ(dst, ApplyPayload(payload_data, source.path(), true));
}
@@ -617,11 +636,16 @@
EXPECT_TRUE(HashCalculator::RawHashOfData(expected_data, &src_hash));
aop.op.set_src_sha256_hash(src_hash.data(), src_hash.size());
- brillo::Blob payload_data = GeneratePayload(brillo::Blob(), {aop}, false);
-
test_utils::ScopedTempFile source("Source-XXXXXX");
EXPECT_TRUE(test_utils::WriteFileVector(source.path(), actual_data));
+ PartitionConfig old_part(kPartitionNameRoot);
+ old_part.path = source.path();
+ old_part.size = actual_data.size();
+
+ brillo::Blob payload_data =
+ GeneratePayload(brillo::Blob(), {aop}, false, &old_part);
+
EXPECT_EQ(actual_data, ApplyPayload(payload_data, source.path(), false));
}
@@ -640,7 +664,12 @@
FakeFileDescriptor* fake_fec = SetFakeECCFile(kCopyOperationSize);
brillo::Blob expected_data = FakeFileDescriptorData(kCopyOperationSize);
- brillo::Blob payload_data = GenerateSourceCopyPayload(expected_data, true);
+ PartitionConfig old_part(kPartitionNameRoot);
+ old_part.path = source.path();
+ old_part.size = invalid_data.size();
+
+ brillo::Blob payload_data =
+ GenerateSourceCopyPayload(expected_data, true, &old_part);
EXPECT_EQ(expected_data, ApplyPayload(payload_data, source.path(), true));
// Verify that the fake_fec was actually used.
EXPECT_EQ(1U, fake_fec->GetReadOps().size());
@@ -661,8 +690,13 @@
// the expected.
FakeFileDescriptor* fake_fec = SetFakeECCFile(kCopyOperationSize / 2);
+ PartitionConfig old_part(kPartitionNameRoot);
+ old_part.path = source.path();
+ old_part.size = expected_data.size();
+
// The payload operation doesn't include an operation hash.
- brillo::Blob payload_data = GenerateSourceCopyPayload(expected_data, false);
+ brillo::Blob payload_data =
+ GenerateSourceCopyPayload(expected_data, false, &old_part);
EXPECT_EQ(expected_data, ApplyPayload(payload_data, source.path(), true));
// Verify that the fake_fec was attempted to be used. Since the file
// descriptor is shorter it can actually do more than one read to realize it
diff --git a/payload_consumer/install_plan.cc b/payload_consumer/install_plan.cc
index f52cd2d..2e7b6d4 100644
--- a/payload_consumer/install_plan.cc
+++ b/payload_consumer/install_plan.cc
@@ -98,7 +98,8 @@
bool InstallPlan::LoadPartitionsFromSlots(BootControlInterface* boot_control) {
bool result = true;
for (Partition& partition : partitions) {
- if (source_slot != BootControlInterface::kInvalidSlot) {
+ if (source_slot != BootControlInterface::kInvalidSlot &&
+ partition.source_size > 0) {
result = boot_control->GetPartitionDevice(
partition.name, source_slot, &partition.source_path) &&
result;
