commit 25650707dd3b125ceb2c50451ad48f8794d89266
author Alessio Balsini <balsini@google.com> Thu Jan 30 16:28:05 2020 +0000
committer Alessio Balsini <balsini@google.com> Thu Jan 30 22:42:54 2020 +0000
tree ce9f934cdba0a7bd816b4e9783d7b08150a0030e
parent 5bd3461a0918363d68d80600816ec9409feb94eb

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 <balsini@google.com>

fs_mgr/libsnapshot/snapshot.cpp

diff --git a/fs_mgr/libsnapshot/snapshot.cpp b/fs_mgr/libsnapshot/snapshot.cpp
index 88731df..785882a 100644
--- a/fs_mgr/libsnapshot/snapshot.cpp
+++ b/fs_mgr/libsnapshot/snapshot.cpp
@@ -1957,8 +1957,17 @@
     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.

fs_mgr/libsnapshot/snapshot_test.cpp

diff --git a/fs_mgr/libsnapshot/snapshot_test.cpp b/fs_mgr/libsnapshot/snapshot_test.cpp
index 634e0b4..c49c49e 100644
--- a/fs_mgr/libsnapshot/snapshot_test.cpp
+++ b/fs_mgr/libsnapshot/snapshot_test.cpp
@@ -793,6 +793,7 @@
 
         // 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 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 @@
 
     // 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 @@
     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 @@
 
     // 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));