From 21671eda3e2561a326cf27acc4cdc4f5911bfc61 Mon Sep 17 00:00:00 2001 From: David Anderson Date: Tue, 10 Jul 2018 09:21:33 -0700 Subject: [PATCH] liblp: Refactor the partition table update API. This change separates the WritePartitionTable function into two separate functions: FlashPartitionTable and UpdatePartitionTable. This will be cleaner than having extra unnecessary parameters and makes it clearer when to call each one. The motivation for this change is to improve the reliability of UpdatePartitionTable() and to be able to simulate I/O failures for testing. Bug: 79173901 Test: liblp_test gtest Change-Id: Iee5eb2e99fb76aef0b93a65bf85a89907e7cd9bf --- fs_mgr/liblp/include/liblp/writer.h | 40 ++++---- fs_mgr/liblp/io_test.cpp | 26 +++--- fs_mgr/liblp/writer.cpp | 137 ++++++++++++++++------------ 3 files changed, 112 insertions(+), 91 deletions(-) diff --git a/fs_mgr/liblp/include/liblp/writer.h b/fs_mgr/liblp/include/liblp/writer.h index efa409dfd..6e3c9cb55 100644 --- a/fs_mgr/liblp/include/liblp/writer.h +++ b/fs_mgr/liblp/include/liblp/writer.h @@ -22,28 +22,26 @@ namespace android { namespace fs_mgr { -// When flashing the initial logical partition layout, we also write geometry -// information at the start and end of the big physical partition. This helps -// locate metadata and backup metadata in the case of corruption or a failed -// update. For normal changes to the metadata, we never modify the geometry. -enum class SyncMode { - // Write geometry information. - Flash, - // Normal update of a single slot. - Update -}; +// Place an initial partition table on the device. This will overwrite the +// existing geometry, and should not be used for normal partition table +// updates. False can be returned if the geometry is incompatible with the +// block device or an I/O error occurs. +bool FlashPartitionTable(const std::string& block_device, const LpMetadata& metadata, + uint32_t slot_number); -// Write the given partition table to the given block device, writing only -// copies according to the given sync mode. -// -// This will perform some verification, such that the device has enough space -// to store the metadata as well as all of its extents. -// -// The slot number indicates which metadata slot to use. -bool WritePartitionTable(const char* block_device, const LpMetadata& metadata, SyncMode sync_mode, - uint32_t slot_number); -bool WritePartitionTable(int fd, const LpMetadata& metadata, SyncMode sync_mode, - uint32_t slot_number); +// Update the partition table for a given metadata slot number. False is +// returned if an error occurs, which can include: +// - Invalid slot number. +// - I/O error. +// - Corrupt or missing metadata geometry on disk. +// - Incompatible geometry. +bool UpdatePartitionTable(const std::string& block_device, const LpMetadata& metadata, + uint32_t slot_number); + +// These variants are for testing only. The path-based functions should be used +// for actual operation, so that open() is called with the correct flags. +bool FlashPartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_number); +bool UpdatePartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_number); // Helper function to serialize geometry and metadata to a normal file, for // flashing or debugging. diff --git a/fs_mgr/liblp/io_test.cpp b/fs_mgr/liblp/io_test.cpp index 25956547f..29d8ca5b5 100644 --- a/fs_mgr/liblp/io_test.cpp +++ b/fs_mgr/liblp/io_test.cpp @@ -102,7 +102,7 @@ static unique_fd CreateFlashedDisk() { if (!exported) { return {}; } - if (!WritePartitionTable(fd, *exported.get(), SyncMode::Flash, 0)) { + if (!FlashPartitionTable(fd, *exported.get(), 0)) { return {}; } return fd; @@ -131,7 +131,7 @@ TEST(liblp, ExportDiskTooSmall) { unique_fd fd = CreateFakeDisk(); ASSERT_GE(fd, 0); - EXPECT_FALSE(WritePartitionTable(fd, *exported.get(), SyncMode::Flash, 0)); + EXPECT_FALSE(FlashPartitionTable(fd, *exported.get(), 0)); } // Test the basics of flashing a partition and reading it back. @@ -146,7 +146,7 @@ TEST(liblp, FlashAndReadback) { // Export and flash. unique_ptr exported = builder->Export(); ASSERT_NE(exported, nullptr); - ASSERT_TRUE(WritePartitionTable(fd, *exported.get(), SyncMode::Flash, 0)); + ASSERT_TRUE(FlashPartitionTable(fd, *exported.get(), 0)); // Read back. Note that some fields are only filled in during // serialization, so exported and imported will not be identical. For @@ -195,7 +195,7 @@ TEST(liblp, UpdateAnyMetadataSlot) { // Change the name before writing to the next slot. strncpy(imported->partitions[0].name, "vendor", sizeof(imported->partitions[0].name)); - ASSERT_TRUE(WritePartitionTable(fd, *imported.get(), SyncMode::Update, 1)); + ASSERT_TRUE(UpdatePartitionTable(fd, *imported.get(), 1)); // Read back the original slot, make sure it hasn't changed. imported = ReadMetadata(fd, 0); @@ -231,7 +231,7 @@ TEST(liblp, InvalidMetadataSlot) { unique_ptr metadata = ReadMetadata(fd, 0); ASSERT_NE(metadata, nullptr); for (uint32_t i = 1; i < kMetadataSlots; i++) { - ASSERT_TRUE(WritePartitionTable(fd, *metadata.get(), SyncMode::Update, i)); + ASSERT_TRUE(UpdatePartitionTable(fd, *metadata.get(), i)); } // Verify that we can't read unavailable slots. @@ -246,25 +246,25 @@ TEST(liblp, NoChangingGeometry) { unique_ptr imported = ReadMetadata(fd, 0); ASSERT_NE(imported, nullptr); - ASSERT_TRUE(WritePartitionTable(fd, *imported.get(), SyncMode::Update, 1)); + ASSERT_TRUE(UpdatePartitionTable(fd, *imported.get(), 1)); imported->geometry.metadata_max_size += LP_SECTOR_SIZE; - ASSERT_FALSE(WritePartitionTable(fd, *imported.get(), SyncMode::Update, 1)); + ASSERT_FALSE(UpdatePartitionTable(fd, *imported.get(), 1)); imported = ReadMetadata(fd, 0); ASSERT_NE(imported, nullptr); imported->geometry.metadata_slot_count++; - ASSERT_FALSE(WritePartitionTable(fd, *imported.get(), SyncMode::Update, 1)); + ASSERT_FALSE(UpdatePartitionTable(fd, *imported.get(), 1)); imported = ReadMetadata(fd, 0); ASSERT_NE(imported, nullptr); imported->geometry.first_logical_sector++; - ASSERT_FALSE(WritePartitionTable(fd, *imported.get(), SyncMode::Update, 1)); + ASSERT_FALSE(UpdatePartitionTable(fd, *imported.get(), 1)); imported = ReadMetadata(fd, 0); ASSERT_NE(imported, nullptr); imported->geometry.last_logical_sector--; - ASSERT_FALSE(WritePartitionTable(fd, *imported.get(), SyncMode::Update, 1)); + ASSERT_FALSE(UpdatePartitionTable(fd, *imported.get(), 1)); } // Test that changing one bit of metadata is enough to break the checksum. @@ -353,8 +353,8 @@ TEST(liblp, TooManyPartitions) { ASSERT_GE(fd, 0); // Check that we are able to write our table. - ASSERT_TRUE(WritePartitionTable(fd, *exported.get(), SyncMode::Flash, 0)); - ASSERT_TRUE(WritePartitionTable(fd, *exported.get(), SyncMode::Update, 1)); + ASSERT_TRUE(FlashPartitionTable(fd, *exported.get(), 0)); + ASSERT_TRUE(UpdatePartitionTable(fd, *exported.get(), 1)); // Check that adding one more partition overflows the metadata allotment. partition = builder->AddPartition("final", TEST_GUID, LP_PARTITION_ATTR_NONE); @@ -364,7 +364,7 @@ TEST(liblp, TooManyPartitions) { ASSERT_NE(exported, nullptr); // The new table should be too large to be written. - ASSERT_FALSE(WritePartitionTable(fd, *exported.get(), SyncMode::Update, 1)); + ASSERT_FALSE(UpdatePartitionTable(fd, *exported.get(), 1)); // Check that the first and last logical sectors weren't touched when we // wrote this almost-full metadata. diff --git a/fs_mgr/liblp/writer.cpp b/fs_mgr/liblp/writer.cpp index 89cbabdb4..e3dba47da 100644 --- a/fs_mgr/liblp/writer.cpp +++ b/fs_mgr/liblp/writer.cpp @@ -73,8 +73,14 @@ static std::string SerializeMetadata(const LpMetadata& input) { // Perform sanity checks so we don't accidentally overwrite valid metadata // with potentially invalid metadata, or random partition data with metadata. -static bool ValidateGeometryAndMetadata(const LpMetadata& metadata, uint64_t blockdevice_size, - uint64_t metadata_size) { +static bool ValidateAndSerializeMetadata(int fd, const LpMetadata& metadata, std::string* blob) { + uint64_t blockdevice_size; + if (!GetDescriptorSize(fd, &blockdevice_size)) { + return false; + } + + *blob = SerializeMetadata(metadata); + const LpMetadataHeader& header = metadata.header; const LpMetadataGeometry& geometry = metadata.geometry; // Validate the usable sector range. @@ -83,7 +89,7 @@ static bool ValidateGeometryAndMetadata(const LpMetadata& metadata, uint64_t blo return false; } // Make sure we're writing within the space reserved. - if (metadata_size > geometry.metadata_max_size) { + if (blob->size() > geometry.metadata_max_size) { LERROR << "Logical partition metadata is too large."; return false; } @@ -124,63 +130,14 @@ static bool ValidateGeometryAndMetadata(const LpMetadata& metadata, uint64_t blo return true; } -bool WritePartitionTable(int fd, const LpMetadata& metadata, SyncMode sync_mode, - uint32_t slot_number) { - uint64_t size; - if (!GetDescriptorSize(fd, &size)) { - return false; - } - - const LpMetadataGeometry& geometry = metadata.geometry; - if (sync_mode != SyncMode::Flash) { - // Verify that the old geometry is identical. If it's not, then we've - // based this new metadata on invalid assumptions. - LpMetadataGeometry old_geometry; - if (!ReadLogicalPartitionGeometry(fd, &old_geometry)) { - return false; - } - if (!CompareGeometry(geometry, old_geometry)) { - LERROR << "Incompatible geometry in new logical partition metadata"; - return false; - } - } - +static bool WriteMetadata(int fd, const LpMetadataGeometry& geometry, uint32_t slot_number, + const std::string& blob) { // Make sure we're writing to a valid metadata slot. if (slot_number >= geometry.metadata_slot_count) { LERROR << "Invalid logical partition metadata slot number."; return false; } - // Before writing geometry and/or logical partition tables, perform some - // basic checks that the geometry and tables are coherent, and will fit - // on the given block device. - std::string blob = SerializeMetadata(metadata); - if (!ValidateGeometryAndMetadata(metadata, size, blob.size())) { - return false; - } - - // First write geometry if this is a flash operation. It gets written to - // the first and last 4096-byte regions of the device. - if (sync_mode == SyncMode::Flash) { - std::string blob = SerializeGeometry(metadata.geometry); - if (SeekFile64(fd, 0, SEEK_SET) < 0) { - PERROR << __PRETTY_FUNCTION__ << "lseek failed: offset 0"; - return false; - } - if (!android::base::WriteFully(fd, blob.data(), blob.size())) { - PERROR << __PRETTY_FUNCTION__ << "write " << blob.size() << " bytes failed"; - return false; - } - if (SeekFile64(fd, -LP_METADATA_GEOMETRY_SIZE, SEEK_END) < 0) { - PERROR << __PRETTY_FUNCTION__ << "lseek failed: offset " << -LP_METADATA_GEOMETRY_SIZE; - return false; - } - if (!android::base::WriteFully(fd, blob.data(), blob.size())) { - PERROR << __PRETTY_FUNCTION__ << "backup write " << blob.size() << " bytes failed"; - return false; - } - } - // Write the primary copy of the metadata. int64_t primary_offset = GetPrimaryMetadataOffset(geometry, slot_number); if (SeekFile64(fd, primary_offset, SEEK_SET) < 0) { @@ -211,14 +168,80 @@ bool WritePartitionTable(int fd, const LpMetadata& metadata, SyncMode sync_mode, return true; } -bool WritePartitionTable(const char* block_device, const LpMetadata& metadata, SyncMode sync_mode, +bool FlashPartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_number) { + // Before writing geometry and/or logical partition tables, perform some + // basic checks that the geometry and tables are coherent, and will fit + // on the given block device. + std::string metadata_blob; + if (!ValidateAndSerializeMetadata(fd, metadata, &metadata_blob)) { + return false; + } + + // Write geometry to the first and last 4096 bytes of the device. + std::string blob = SerializeGeometry(metadata.geometry); + if (SeekFile64(fd, 0, SEEK_SET) < 0) { + PERROR << __PRETTY_FUNCTION__ << "lseek failed: offset 0"; + return false; + } + if (!android::base::WriteFully(fd, blob.data(), blob.size())) { + PERROR << __PRETTY_FUNCTION__ << "write " << blob.size() << " bytes failed"; + return false; + } + if (SeekFile64(fd, -LP_METADATA_GEOMETRY_SIZE, SEEK_END) < 0) { + PERROR << __PRETTY_FUNCTION__ << "lseek failed: offset " << -LP_METADATA_GEOMETRY_SIZE; + return false; + } + if (!android::base::WriteFully(fd, blob.data(), blob.size())) { + PERROR << __PRETTY_FUNCTION__ << "backup write " << blob.size() << " bytes failed"; + return false; + } + + // Write metadata to the correct slot, now that geometry is in place. + return WriteMetadata(fd, metadata.geometry, slot_number, metadata_blob); +} + +bool UpdatePartitionTable(int fd, const LpMetadata& metadata, uint32_t slot_number) { + // Before writing geometry and/or logical partition tables, perform some + // basic checks that the geometry and tables are coherent, and will fit + // on the given block device. + std::string blob; + if (!ValidateAndSerializeMetadata(fd, metadata, &blob)) { + return false; + } + + // Verify that the old geometry is identical. If it's not, then we might be + // writing a table that was built for a different device, so we must reject + // it. + const LpMetadataGeometry& geometry = metadata.geometry; + LpMetadataGeometry old_geometry; + if (!ReadLogicalPartitionGeometry(fd, &old_geometry)) { + return false; + } + if (!CompareGeometry(geometry, old_geometry)) { + LERROR << "Incompatible geometry in new logical partition metadata"; + return false; + } + return WriteMetadata(fd, geometry, slot_number, blob); +} + +bool FlashPartitionTable(const std::string& block_device, const LpMetadata& metadata, uint32_t slot_number) { - android::base::unique_fd fd(open(block_device, O_RDWR | O_SYNC)); + android::base::unique_fd fd(open(block_device.c_str(), O_RDWR | O_SYNC)); if (fd < 0) { PERROR << __PRETTY_FUNCTION__ << "open failed: " << block_device; return false; } - return WritePartitionTable(fd, metadata, sync_mode, slot_number); + return FlashPartitionTable(fd, metadata, slot_number); +} + +bool UpdatePartitionTable(const std::string& block_device, const LpMetadata& metadata, + uint32_t slot_number) { + android::base::unique_fd fd(open(block_device.c_str(), O_RDWR | O_SYNC)); + if (fd < 0) { + PERROR << __PRETTY_FUNCTION__ << "open failed: " << block_device; + return false; + } + return UpdatePartitionTable(fd, metadata, slot_number); } bool WriteToImageFile(int fd, const LpMetadata& input) {