From fb5de5bc8f315ac699460865dbaeb50734fd0ffc Mon Sep 17 00:00:00 2001 From: Alessio Balsini Date: Thu, 30 Jan 2020 16:28:05 +0000 Subject: [PATCH] Propagate failure of MetadataBuilder creation in CreateUpdateSnapshots In a device with malformed metadata, the MetadataBuilder returns a nullptr that may cause segmentation faults when using the builder. Fix by handling the nullptr exception in CreateUpdateSnapshot and propagating the error to its caller. Bug: n/a Test: libsnapshot_test Change-Id: Ie9148a552cf4bb223ab8d54b1d30d2b13d92bb22 Signed-off-by: Alessio Balsini --- fs_mgr/libsnapshot/snapshot.cpp | 9 +++++++++ fs_mgr/libsnapshot/snapshot_test.cpp | 6 ++++++ 2 files changed, 15 insertions(+) diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp index 88731dfb6..785882a8d 100644 --- a/fs_mgr/libsnapshot/snapshot.cpp +++ b/fs_mgr/libsnapshot/snapshot.cpp @@ -1957,8 +1957,17 @@ Return SnapshotManager::CreateUpdateSnapshots(const DeltaArchiveManifest& manife auto current_super = device_->GetSuperDevice(current_slot); auto current_metadata = MetadataBuilder::New(opener, current_super, current_slot); + if (current_metadata == nullptr) { + LOG(ERROR) << "Cannot create metadata builder."; + return Return::Error(); + } + auto target_metadata = MetadataBuilder::NewForUpdate(opener, current_super, current_slot, target_slot); + if (target_metadata == nullptr) { + LOG(ERROR) << "Cannot create target metadata builder."; + return Return::Error(); + } // Delete partitions with target suffix in |current_metadata|. Otherwise, // partition_cow_creator recognizes these left-over partitions as used space. diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp index 634e0b491..c49c49e4d 100644 --- a/fs_mgr/libsnapshot/snapshot_test.cpp +++ b/fs_mgr/libsnapshot/snapshot_test.cpp @@ -793,6 +793,7 @@ class SnapshotUpdateTest : public SnapshotTest { // Initialize source partition metadata using |manifest_|. src_ = MetadataBuilder::New(*opener_, "super", 0); + ASSERT_NE(src_, nullptr); ASSERT_TRUE(FillFakeMetadata(src_.get(), manifest_, "_a")); // Add sys_b which is like system_other. ASSERT_TRUE(src_->AddGroup("group_b", kGroupSize)); @@ -987,6 +988,7 @@ TEST_F(SnapshotUpdateTest, FullUpdateFlow) { // Test that partitions prioritize using space in super. auto tgt = MetadataBuilder::New(*opener_, "super", 1); + ASSERT_NE(tgt, nullptr); ASSERT_NE(nullptr, tgt->FindPartition("sys_b-cow")); ASSERT_NE(nullptr, tgt->FindPartition("vnd_b-cow")); ASSERT_EQ(nullptr, tgt->FindPartition("prd_b-cow")); @@ -1200,7 +1202,9 @@ TEST_F(SnapshotUpdateTest, ReclaimCow) { // Check that the old COW space is reclaimed and does not occupy space of mapped partitions. auto src = MetadataBuilder::New(*opener_, "super", 1); + ASSERT_NE(src, nullptr); auto tgt = MetadataBuilder::New(*opener_, "super", 0); + ASSERT_NE(tgt, nullptr); for (const auto& cow_part_name : {"sys_a-cow", "vnd_a-cow", "prd_a-cow"}) { auto* cow_part = tgt->FindPartition(cow_part_name); ASSERT_NE(nullptr, cow_part) << cow_part_name << " does not exist in target metadata"; @@ -1291,6 +1295,7 @@ TEST_F(SnapshotUpdateTest, MergeCannotRemoveCow) { SetSize(vnd_, 5_MiB); SetSize(prd_, 5_MiB); src_ = MetadataBuilder::New(*opener_, "super", 0); + ASSERT_NE(src_, nullptr); src_->RemoveGroupAndPartitions(group_->name() + "_a"); src_->RemoveGroupAndPartitions(group_->name() + "_b"); ASSERT_TRUE(FillFakeMetadata(src_.get(), manifest_, "_a")); @@ -1664,6 +1669,7 @@ TEST_P(FlashAfterUpdateTest, FlashSlotAfterUpdate) { // Simulate flashing |flashed_slot|. This clears the UPDATED flag. auto flashed_builder = MetadataBuilder::New(*opener_, "super", flashed_slot); + ASSERT_NE(flashed_builder, nullptr); flashed_builder->RemoveGroupAndPartitions(group_->name() + flashed_slot_suffix); flashed_builder->RemoveGroupAndPartitions(kCowGroupName); ASSERT_TRUE(FillFakeMetadata(flashed_builder.get(), manifest_, flashed_slot_suffix));