From c02be81efcf2ebb81bfb771d20cb6c22b766adc1 Mon Sep 17 00:00:00 2001 From: Ramon Pantin Date: Thu, 10 Jan 2019 10:59:59 -0800 Subject: [PATCH] Fixed a few minor nits in liblp Bug: 122616553 Test: built and ran liblp_test on Pixel 3 with Q weekly build Added missing __attribute__((packed)) in two metadata structures. Fixed error logging message when repairing primary metadata. Few very minor additions related to metadata validation. Fixed an off by one error in the validation of partition name length. Change-Id: Ic777baf97871c786a209da7c32bc13c1360a8482 Signed-off-by: Ramon Pantin --- fs_mgr/liblp/builder.cpp | 7 ++++++- fs_mgr/liblp/include/liblp/metadata_format.h | 6 +++--- fs_mgr/liblp/reader.cpp | 4 ++++ fs_mgr/liblp/writer.cpp | 4 ++-- 4 files changed, 15 insertions(+), 6 deletions(-) diff --git a/fs_mgr/liblp/builder.cpp b/fs_mgr/liblp/builder.cpp index 07f9d66d0..8d08689e2 100644 --- a/fs_mgr/liblp/builder.cpp +++ b/fs_mgr/liblp/builder.cpp @@ -267,6 +267,11 @@ void MetadataBuilder::ImportExtents(Partition* dest, const LpMetadata& metadata, } static bool VerifyDeviceProperties(const BlockDeviceInfo& device_info) { + if (device_info.logical_block_size == 0) { + LERROR << "Block device " << device_info.partition_name + << " logical block size must not be zero."; + return false; + } if (device_info.logical_block_size % LP_SECTOR_SIZE != 0) { LERROR << "Block device " << device_info.partition_name << " logical block size must be a multiple of 512."; @@ -333,7 +338,7 @@ bool MetadataBuilder::Init(const std::vector& block_devices, out.alignment = device_info.alignment; out.alignment_offset = device_info.alignment_offset; out.size = device_info.size; - if (device_info.partition_name.size() >= sizeof(out.partition_name)) { + if (device_info.partition_name.size() > sizeof(out.partition_name)) { LERROR << "Partition name " << device_info.partition_name << " exceeds maximum length."; return false; } diff --git a/fs_mgr/liblp/include/liblp/metadata_format.h b/fs_mgr/liblp/include/liblp/metadata_format.h index 9c5ec5c93..8934aaff2 100644 --- a/fs_mgr/liblp/include/liblp/metadata_format.h +++ b/fs_mgr/liblp/include/liblp/metadata_format.h @@ -127,7 +127,7 @@ typedef struct LpMetadataGeometry { * num_entries, and the result must not overflow a 32-bit signed integer. */ typedef struct LpMetadataTableDescriptor { - /* 0: Location of the table, relative to the metadata header. */ + /* 0: Location of the table, relative to end of the metadata header. */ uint32_t offset; /* 4: Number of entries in the table. */ uint32_t num_entries; @@ -272,7 +272,7 @@ typedef struct LpMetadataPartitionGroup { /* 40: Maximum size in bytes. If 0, the group has no maximum size. */ uint64_t maximum_size; -} LpMetadataPartitionGroup; +} __attribute__((packed)) LpMetadataPartitionGroup; /* This flag is only intended to be used with super_empty.img and super.img on * retrofit devices. If set, the group needs a slot suffix to be interpreted @@ -323,7 +323,7 @@ typedef struct LpMetadataBlockDevice { /* 60: Flags (see LP_BLOCK_DEVICE_* flags below). */ uint32_t flags; -} LpMetadataBlockDevice; +} __attribute__((packed)) LpMetadataBlockDevice; /* This flag is only intended to be used with super_empty.img and super.img on * retrofit devices. On these devices there are A and B super partitions, and diff --git a/fs_mgr/liblp/reader.cpp b/fs_mgr/liblp/reader.cpp index 24c6b2c31..dcee6d2d8 100644 --- a/fs_mgr/liblp/reader.cpp +++ b/fs_mgr/liblp/reader.cpp @@ -256,6 +256,10 @@ static std::unique_ptr ParseMetadata(const LpMetadataGeometry& geome LERROR << "Logical partition has invalid attribute set."; return nullptr; } + if (partition.first_extent_index + partition.num_extents < partition.first_extent_index) { + LERROR << "Logical partition first_extent_index + num_extents overflowed."; + return nullptr; + } if (partition.first_extent_index + partition.num_extents > header.extents.num_entries) { LERROR << "Logical partition has invalid extent list."; return nullptr; diff --git a/fs_mgr/liblp/writer.cpp b/fs_mgr/liblp/writer.cpp index 454258b75..0ab0d8cf6 100644 --- a/fs_mgr/liblp/writer.cpp +++ b/fs_mgr/liblp/writer.cpp @@ -367,11 +367,11 @@ bool UpdatePartitionTable(const IPartitionOpener& opener, const std::string& sup // safety. std::string old_blob; if (!ValidateAndSerializeMetadata(opener, *backup.get(), slot_suffix, &old_blob)) { - LERROR << "Error serializing primary metadata to repair corrupted backup"; + LERROR << "Error serializing backup metadata to repair corrupted primary"; return false; } if (!WritePrimaryMetadata(fd, metadata, slot_number, old_blob, writer)) { - LERROR << "Error writing primary metadata to repair corrupted backup"; + LERROR << "Error writing backup metadata to repair corrupted primary"; return false; } }