liblp: Add integer overflow checks when aligning.
This will prevent ubsan crashes on invalid inputs.
Bug: 155510366
Test: liblp_test gtest
Change-Id: Id6dd8badd0025d6cac3113c3f9076ea3f4d9c175
diff --git a/fs_mgr/liblp/builder.cpp b/fs_mgr/liblp/builder.cpp
index dc3b985..c37d70e 100644
--- a/fs_mgr/liblp/builder.cpp
+++ b/fs_mgr/liblp/builder.cpp
@@ -19,6 +19,7 @@
#include <string.h>
#include <algorithm>
+#include <limits>
#include <android-base/unique_fd.h>
@@ -369,7 +370,10 @@
}
// Align the metadata size up to the nearest sector.
- metadata_max_size = AlignTo(metadata_max_size, LP_SECTOR_SIZE);
+ if (!AlignTo(metadata_max_size, LP_SECTOR_SIZE, &metadata_max_size)) {
+ LERROR << "Max metadata size " << metadata_max_size << " is too large.";
+ return false;
+ }
// Validate and build the block device list.
uint32_t logical_block_size = 0;
@@ -401,10 +405,15 @@
// untouched to be compatible code that looks for an MBR. Thus we
// start counting free sectors at sector 1, not 0.
uint64_t free_area_start = LP_SECTOR_SIZE;
+ bool ok;
if (out.alignment) {
- free_area_start = AlignTo(free_area_start, out.alignment);
+ ok = AlignTo(free_area_start, out.alignment, &free_area_start);
} else {
- free_area_start = AlignTo(free_area_start, logical_block_size);
+ ok = AlignTo(free_area_start, logical_block_size, &free_area_start);
+ }
+ if (!ok) {
+ LERROR << "Integer overflow computing free area start";
+ return false;
}
out.first_logical_sector = free_area_start / LP_SECTOR_SIZE;
@@ -441,10 +450,15 @@
// Compute the first free sector, factoring in alignment.
uint64_t free_area_start = total_reserved;
+ bool ok;
if (super.alignment || super.alignment_offset) {
- free_area_start = AlignTo(free_area_start, super.alignment);
+ ok = AlignTo(free_area_start, super.alignment, &free_area_start);
} else {
- free_area_start = AlignTo(free_area_start, logical_block_size);
+ ok = AlignTo(free_area_start, logical_block_size, &free_area_start);
+ }
+ if (!ok) {
+ LERROR << "Integer overflow computing free area start";
+ return false;
}
super.first_logical_sector = free_area_start / LP_SECTOR_SIZE;
@@ -544,7 +558,11 @@
const Interval& current = extents[i];
DCHECK(previous.device_index == current.device_index);
- uint64_t aligned = AlignSector(block_devices_[current.device_index], previous.end);
+ uint64_t aligned;
+ if (!AlignSector(block_devices_[current.device_index], previous.end, &aligned)) {
+ LERROR << "Sector " << previous.end << " caused integer overflow.";
+ continue;
+ }
if (aligned >= current.start) {
// There is no gap between these two extents, try the next one.
// Note that we check with >= instead of >, since alignment may
@@ -730,7 +748,10 @@
// Choose an aligned sector for the midpoint. This could lead to one half
// being slightly larger than the other, but this will not restrict the
// size of partitions (it might lead to one extra extent if "B" overflows).
- midpoint = AlignSector(super, midpoint);
+ if (!AlignSector(super, midpoint, &midpoint)) {
+ LERROR << "Unexpected integer overflow aligning midpoint " << midpoint;
+ return free_list;
+ }
std::vector<Interval> first_half;
std::vector<Interval> second_half;
@@ -768,7 +789,11 @@
// If the sector ends where the next aligned chunk begins, then there's
// no missing gap to try and allocate.
const auto& block_device = block_devices_[extent->device_index()];
- uint64_t next_aligned_sector = AlignSector(block_device, extent->end_sector());
+ uint64_t next_aligned_sector;
+ if (!AlignSector(block_device, extent->end_sector(), &next_aligned_sector)) {
+ LERROR << "Integer overflow aligning sector " << extent->end_sector();
+ return nullptr;
+ }
if (extent->end_sector() == next_aligned_sector) {
return nullptr;
}
@@ -925,13 +950,19 @@
return size;
}
-uint64_t MetadataBuilder::AlignSector(const LpMetadataBlockDevice& block_device,
- uint64_t sector) const {
+bool MetadataBuilder::AlignSector(const LpMetadataBlockDevice& block_device, uint64_t sector,
+ uint64_t* out) const {
// Note: when reading alignment info from the Kernel, we don't assume it
// is aligned to the sector size, so we round up to the nearest sector.
uint64_t lba = sector * LP_SECTOR_SIZE;
- uint64_t aligned = AlignTo(lba, block_device.alignment);
- return AlignTo(aligned, LP_SECTOR_SIZE) / LP_SECTOR_SIZE;
+ if (!AlignTo(lba, block_device.alignment, out)) {
+ return false;
+ }
+ if (!AlignTo(*out, LP_SECTOR_SIZE, out)) {
+ return false;
+ }
+ *out /= LP_SECTOR_SIZE;
+ return true;
}
bool MetadataBuilder::FindBlockDeviceByName(const std::string& partition_name,
@@ -1005,7 +1036,12 @@
bool MetadataBuilder::ResizePartition(Partition* partition, uint64_t requested_size,
const std::vector<Interval>& free_region_hint) {
// Align the space needed up to the nearest sector.
- uint64_t aligned_size = AlignTo(requested_size, geometry_.logical_block_size);
+ uint64_t aligned_size;
+ if (!AlignTo(requested_size, geometry_.logical_block_size, &aligned_size)) {
+ LERROR << "Cannot resize partition " << partition->name() << " to " << requested_size
+ << " bytes; integer overflow.";
+ return false;
+ }
uint64_t old_size = partition->size();
if (!ValidatePartitionSizeChange(partition, old_size, aligned_size, false)) {
diff --git a/fs_mgr/liblp/builder_test.cpp b/fs_mgr/liblp/builder_test.cpp
index 52a3217..1a3250a 100644
--- a/fs_mgr/liblp/builder_test.cpp
+++ b/fs_mgr/liblp/builder_test.cpp
@@ -228,8 +228,9 @@
ASSERT_EQ(extent.target_type, LP_TARGET_TYPE_LINEAR);
EXPECT_EQ(extent.num_sectors, 80);
+ uint64_t aligned_lba;
uint64_t lba = extent.target_data * LP_SECTOR_SIZE;
- uint64_t aligned_lba = AlignTo(lba, device_info.alignment);
+ ASSERT_TRUE(AlignTo(lba, device_info.alignment, &aligned_lba));
EXPECT_EQ(lba, aligned_lba);
}
@@ -1051,3 +1052,17 @@
EXPECT_EQ(e2->physical_sector(), 3072);
EXPECT_EQ(e2->end_sector(), 4197368);
}
+
+TEST_F(BuilderTest, ResizeOverflow) {
+ BlockDeviceInfo super("super", 8_GiB, 786432, 229376, 4096);
+ std::vector<BlockDeviceInfo> block_devices = {super};
+
+ unique_ptr<MetadataBuilder> builder = MetadataBuilder::New(block_devices, "super", 65536, 2);
+ ASSERT_NE(builder, nullptr);
+
+ ASSERT_TRUE(builder->AddGroup("group", 0));
+
+ Partition* p = builder->AddPartition("system", "default", 0);
+ ASSERT_NE(p, nullptr);
+ ASSERT_FALSE(builder->ResizePartition(p, 18446744073709551615ULL));
+}
diff --git a/fs_mgr/liblp/include/liblp/builder.h b/fs_mgr/liblp/include/liblp/builder.h
index bd39150..732dbea 100644
--- a/fs_mgr/liblp/include/liblp/builder.h
+++ b/fs_mgr/liblp/include/liblp/builder.h
@@ -359,7 +359,7 @@
bool GrowPartition(Partition* partition, uint64_t aligned_size,
const std::vector<Interval>& free_region_hint);
void ShrinkPartition(Partition* partition, uint64_t aligned_size);
- uint64_t AlignSector(const LpMetadataBlockDevice& device, uint64_t sector) const;
+ bool AlignSector(const LpMetadataBlockDevice& device, uint64_t sector, uint64_t* out) const;
uint64_t TotalSizeOfGroup(PartitionGroup* group) const;
bool UpdateBlockDeviceInfo(size_t index, const BlockDeviceInfo& info);
bool FindBlockDeviceByName(const std::string& partition_name, uint32_t* index) const;
diff --git a/fs_mgr/liblp/utility.h b/fs_mgr/liblp/utility.h
index f210eaf..c4fe3ed 100644
--- a/fs_mgr/liblp/utility.h
+++ b/fs_mgr/liblp/utility.h
@@ -21,6 +21,7 @@
#include <stdint.h>
#include <sys/types.h>
+#include <limits>
#include <string>
#include <string_view>
@@ -66,16 +67,26 @@
void SHA256(const void* data, size_t length, uint8_t out[32]);
// Align |base| such that it is evenly divisible by |alignment|, which does not
-// have to be a power of two.
-constexpr uint64_t AlignTo(uint64_t base, uint32_t alignment) {
+// have to be a power of two. Return false on overflow.
+template <typename T>
+bool AlignTo(T base, uint32_t alignment, T* out) {
+ static_assert(std::numeric_limits<T>::is_integer);
+ static_assert(!std::numeric_limits<T>::is_signed);
if (!alignment) {
- return base;
+ *out = base;
+ return true;
}
- uint64_t remainder = base % alignment;
+ T remainder = base % alignment;
if (remainder == 0) {
- return base;
+ *out = base;
+ return true;
}
- return base + (alignment - remainder);
+ T to_add = alignment - remainder;
+ if (to_add > std::numeric_limits<T>::max() - base) {
+ return false;
+ }
+ *out = base + to_add;
+ return true;
}
// Update names from C++ strings.
diff --git a/fs_mgr/liblp/utility_test.cpp b/fs_mgr/liblp/utility_test.cpp
index b64861d..fc90872 100644
--- a/fs_mgr/liblp/utility_test.cpp
+++ b/fs_mgr/liblp/utility_test.cpp
@@ -14,6 +14,8 @@
* limitations under the License.
*/
+#include <optional>
+
#include <gtest/gtest.h>
#include <liblp/builder.h>
#include <liblp/liblp.h>
@@ -58,15 +60,28 @@
EXPECT_EQ(GetBackupMetadataOffset(geometry, 0), backup_start + 16384 * 0);
}
+std::optional<uint64_t> AlignTo(uint64_t base, uint32_t alignment) {
+ uint64_t r;
+ if (!AlignTo(base, alignment, &r)) {
+ return {};
+ }
+ return {r};
+}
+
TEST(liblp, AlignTo) {
- EXPECT_EQ(AlignTo(37, 0), 37);
- EXPECT_EQ(AlignTo(1024, 1024), 1024);
- EXPECT_EQ(AlignTo(555, 1024), 1024);
- EXPECT_EQ(AlignTo(555, 1000), 1000);
- EXPECT_EQ(AlignTo(0, 1024), 0);
- EXPECT_EQ(AlignTo(54, 32), 64);
- EXPECT_EQ(AlignTo(32, 32), 32);
- EXPECT_EQ(AlignTo(17, 32), 32);
+ EXPECT_EQ(AlignTo(37, 0), std::optional<uint64_t>(37));
+ EXPECT_EQ(AlignTo(1024, 1024), std::optional<uint64_t>(1024));
+ EXPECT_EQ(AlignTo(555, 1024), std::optional<uint64_t>(1024));
+ EXPECT_EQ(AlignTo(555, 1000), std::optional<uint64_t>(1000));
+ EXPECT_EQ(AlignTo(0, 1024), std::optional<uint64_t>(0));
+ EXPECT_EQ(AlignTo(54, 32), std::optional<uint64_t>(64));
+ EXPECT_EQ(AlignTo(32, 32), std::optional<uint64_t>(32));
+ EXPECT_EQ(AlignTo(17, 32), std::optional<uint64_t>(32));
+
+ auto u32limit = std::numeric_limits<uint32_t>::max();
+ auto u64limit = std::numeric_limits<uint64_t>::max();
+ EXPECT_EQ(AlignTo(u64limit - u32limit + 1, u32limit), std::optional<uint64_t>{u64limit});
+ EXPECT_EQ(AlignTo(std::numeric_limits<uint64_t>::max(), 2), std::optional<uint64_t>{});
}
TEST(liblp, GetPartitionSlotSuffix) {