liblp: Require block-aligned partition sizes.
DM_TABLE_LOAD will reject dm-linear entries if their size is not a
multiple of the backing device's logical block size. For example, a
partition of 10GiB+512 bytes will fail to map in device-mapper if the
logical block size is 4096 bytes. To address this, this patch adds a
few changes to liblp:
The block size given to lpmake is now recorded in LpGeometryMetadata.
The block size must be a multiple of the sector size. In addition,
partiton sizes are now aligned to the block size, and the super
partition must have enough free space to allocate at least one block (in
addition to storing metadata).
GrowPartition now has multiple checks that the block-size invariant is not
violated, to ensure that no invalid partition tables will be created.
Bug: 79173901
Test: liblp_test gtest
Change-Id: I484aac1f9b90ebd92dc1c89ce1e09cd89bbb441e
diff --git a/fs_mgr/liblp/builder.cpp b/fs_mgr/liblp/builder.cpp
index d3c785d..eb429b9 100644
--- a/fs_mgr/liblp/builder.cpp
+++ b/fs_mgr/liblp/builder.cpp
@@ -48,10 +48,20 @@
PERROR << __PRETTY_FUNCTION__ << "BLKIOMIN failed";
return false;
}
- if (ioctl(fd, BLKALIGNOFF, &device_info->alignment_offset) < 0) {
+
+ int alignment_offset;
+ if (ioctl(fd, BLKALIGNOFF, &alignment_offset) < 0) {
PERROR << __PRETTY_FUNCTION__ << "BLKIOMIN failed";
return false;
}
+ int logical_block_size;
+ if (ioctl(fd, BLKSSZGET, &logical_block_size) < 0) {
+ PERROR << __PRETTY_FUNCTION__ << "BLKSSZGET failed";
+ return false;
+ }
+
+ device_info->alignment_offset = static_cast<uint32_t>(alignment_offset);
+ device_info->logical_block_size = static_cast<uint32_t>(logical_block_size);
return true;
#else
(void)block_device;
@@ -178,6 +188,7 @@
device_info_.alignment = geometry_.alignment;
device_info_.alignment_offset = geometry_.alignment_offset;
+ device_info_.logical_block_size = geometry_.logical_block_size;
return true;
}
@@ -201,6 +212,10 @@
LERROR << "Block device size must be a multiple of 512.";
return false;
}
+ if (device_info_.logical_block_size % LP_SECTOR_SIZE != 0) {
+ LERROR << "Logical block size must be a multiple of 512.";
+ return false;
+ }
if (device_info_.alignment_offset % LP_SECTOR_SIZE != 0) {
LERROR << "Alignment offset is not sector-aligned.";
return false;
@@ -244,6 +259,18 @@
return false;
}
+ // Finally, the size of the allocatable space must be a multiple of the
+ // logical block size. If we have no more free space after this
+ // computation, then we abort. Note that the last sector is inclusive,
+ // so we have to account for that.
+ uint64_t num_free_sectors = last_sector - first_sector + 1;
+ uint64_t sectors_per_block = device_info_.logical_block_size / LP_SECTOR_SIZE;
+ if (num_free_sectors < sectors_per_block) {
+ LERROR << "Not enough space to allocate any partition tables.";
+ return false;
+ }
+ last_sector = first_sector + (num_free_sectors / sectors_per_block) * sectors_per_block - 1;
+
geometry_.first_logical_sector = first_sector;
geometry_.last_logical_sector = last_sector;
geometry_.metadata_max_size = metadata_max_size;
@@ -251,6 +278,7 @@
geometry_.alignment = device_info_.alignment;
geometry_.alignment_offset = device_info_.alignment_offset;
geometry_.block_device_size = device_info_.size;
+ geometry_.logical_block_size = device_info.logical_block_size;
return true;
}
@@ -297,6 +325,7 @@
uint64_t end;
Interval(uint64_t start, uint64_t end) : start(start), end(end) {}
+ uint64_t length() const { return end - start; }
bool operator<(const Interval& other) const { return start < other.start; }
};
@@ -343,31 +372,46 @@
free_regions.emplace_back(last_free_extent_start, geometry_.last_logical_sector + 1);
}
+ const uint64_t sectors_per_block = device_info_.logical_block_size / LP_SECTOR_SIZE;
+ CHECK(sectors_needed % sectors_per_block == 0);
+
// Find gaps that we can use for new extents. Note we store new extents in a
// temporary vector, and only commit them if we are guaranteed enough free
// space.
std::vector<std::unique_ptr<LinearExtent>> new_extents;
- for (const auto& region : free_regions) {
- // This gap is enough to hold the remainder of the space requested, so we
- // can allocate what we need and return.
- if (region.end - region.start >= sectors_needed) {
- auto extent = std::make_unique<LinearExtent>(sectors_needed, region.start);
- sectors_needed -= extent->num_sectors();
- new_extents.push_back(std::move(extent));
- break;
+ for (auto& region : free_regions) {
+ if (region.length() % sectors_per_block != 0) {
+ // This should never happen, because it would imply that we
+ // once allocated an extent that was not a multiple of the
+ // block size. That extent would be rejected by DM_TABLE_LOAD.
+ LERROR << "Region " << region.start << ".." << region.end
+ << " is not a multiple of the block size, " << sectors_per_block;
+
+ // If for some reason the final region is mis-sized we still want
+ // to be able to grow partitions. So just to be safe, round the
+ // region down to the nearest block.
+ region.end = region.start + (region.length() / sectors_per_block) * sectors_per_block;
+ if (!region.length()) {
+ continue;
+ }
}
- // This gap is not big enough to fit the remainder of the space requested,
- // so consume the whole thing and keep looking for more.
- auto extent = std::make_unique<LinearExtent>(region.end - region.start, region.start);
- sectors_needed -= extent->num_sectors();
+ uint64_t sectors = std::min(sectors_needed, region.length());
+ CHECK(sectors % sectors_per_block == 0);
+
+ auto extent = std::make_unique<LinearExtent>(sectors, region.start);
new_extents.push_back(std::move(extent));
+ sectors_needed -= sectors;
+ if (!sectors_needed) {
+ break;
+ }
}
if (sectors_needed) {
LERROR << "Not enough free space to expand partition: " << partition->name();
return false;
}
+ // Everything succeeded, so commit the new extents.
for (auto& extent : new_extents) {
partition->AddExtent(std::move(extent));
}
@@ -435,6 +479,12 @@
void MetadataBuilder::set_block_device_info(const BlockDeviceInfo& device_info) {
device_info_.size = device_info.size;
+ // Note that if the logical block size changes, we're probably in trouble:
+ // we could have already built extents that will only work on the previous
+ // size.
+ DCHECK(partitions_.empty() ||
+ device_info_.logical_block_size == device_info.logical_block_size);
+
// The kernel does not guarantee these values are present, so we only
// replace existing values if the new values are non-zero.
if (device_info.alignment) {
@@ -447,7 +497,7 @@
bool MetadataBuilder::ResizePartition(Partition* partition, uint64_t requested_size) {
// Align the space needed up to the nearest sector.
- uint64_t aligned_size = AlignTo(requested_size, LP_SECTOR_SIZE);
+ uint64_t aligned_size = AlignTo(requested_size, device_info_.logical_block_size);
if (aligned_size > partition->size()) {
return GrowPartition(partition, aligned_size);
diff --git a/fs_mgr/liblp/builder_test.cpp b/fs_mgr/liblp/builder_test.cpp
index 4334d51..f1a91c4 100644
--- a/fs_mgr/liblp/builder_test.cpp
+++ b/fs_mgr/liblp/builder_test.cpp
@@ -92,11 +92,11 @@
Partition* system = builder->AddPartition("system", TEST_GUID, LP_PARTITION_ATTR_READONLY);
ASSERT_NE(system, nullptr);
EXPECT_EQ(builder->ResizePartition(system, 10000), true);
- EXPECT_EQ(system->size(), 10240);
+ EXPECT_EQ(system->size(), 12288);
EXPECT_EQ(system->extents().size(), 1);
- builder->ResizePartition(system, 9000);
- EXPECT_EQ(system->size(), 9216);
+ builder->ResizePartition(system, 7000);
+ EXPECT_EQ(system->size(), 8192);
EXPECT_EQ(system->extents().size(), 1);
}
@@ -120,13 +120,13 @@
TEST(liblp, InternalAlignment) {
// Test the metadata fitting within alignment.
- BlockDeviceInfo device_info(1024 * 1024, 768 * 1024, 0);
+ BlockDeviceInfo device_info(1024 * 1024, 768 * 1024, 0, 4096);
unique_ptr<MetadataBuilder> builder = MetadataBuilder::New(device_info, 1024, 2);
ASSERT_NE(builder, nullptr);
unique_ptr<LpMetadata> exported = builder->Export();
ASSERT_NE(exported, nullptr);
EXPECT_EQ(exported->geometry.first_logical_sector, 1536);
- EXPECT_EQ(exported->geometry.last_logical_sector, 2035);
+ EXPECT_EQ(exported->geometry.last_logical_sector, 2031);
// Test a large alignment offset thrown in.
device_info.alignment_offset = 753664;
@@ -135,7 +135,7 @@
exported = builder->Export();
ASSERT_NE(exported, nullptr);
EXPECT_EQ(exported->geometry.first_logical_sector, 1472);
- EXPECT_EQ(exported->geometry.last_logical_sector, 2035);
+ EXPECT_EQ(exported->geometry.last_logical_sector, 2031);
// Alignment offset without alignment doesn't mean anything.
device_info.alignment = 0;
@@ -150,7 +150,7 @@
exported = builder->Export();
ASSERT_NE(exported, nullptr);
EXPECT_EQ(exported->geometry.first_logical_sector, 78);
- EXPECT_EQ(exported->geometry.last_logical_sector, 1975);
+ EXPECT_EQ(exported->geometry.last_logical_sector, 1973);
// Test a small alignment with no alignment offset.
device_info.alignment = 11 * 1024;
@@ -163,7 +163,7 @@
}
TEST(liblp, InternalPartitionAlignment) {
- BlockDeviceInfo device_info(512 * 1024 * 1024, 768 * 1024, 753664);
+ BlockDeviceInfo device_info(512 * 1024 * 1024, 768 * 1024, 753664, 4096);
unique_ptr<MetadataBuilder> builder = MetadataBuilder::New(device_info, 32 * 1024, 2);
Partition* a = builder->AddPartition("a", TEST_GUID, 0);
@@ -381,7 +381,7 @@
static const size_t kMetadataSize = 64 * 1024;
// No space to store metadata + geometry.
- BlockDeviceInfo device_info(kDiskSize, 0, 0);
+ BlockDeviceInfo device_info(kDiskSize, 0, 0, 4096);
unique_ptr<MetadataBuilder> builder = MetadataBuilder::New(device_info, kMetadataSize, 1);
EXPECT_EQ(builder, nullptr);
@@ -390,8 +390,8 @@
builder = MetadataBuilder::New(device_info, kMetadataSize, 1);
EXPECT_EQ(builder, nullptr);
- // Space for metadata + geometry + one free sector.
- device_info.size += LP_SECTOR_SIZE;
+ // Space for metadata + geometry + one free block.
+ device_info.size += device_info.logical_block_size;
builder = MetadataBuilder::New(device_info, kMetadataSize, 1);
EXPECT_NE(builder, nullptr);
@@ -424,19 +424,21 @@
ASSERT_EQ(device_info.alignment % LP_SECTOR_SIZE, 0);
ASSERT_EQ(device_info.alignment_offset % LP_SECTOR_SIZE, 0);
ASSERT_LE(device_info.alignment_offset, INT_MAX);
+ ASSERT_EQ(device_info.logical_block_size % LP_SECTOR_SIZE, 0);
// Having an alignment offset > alignment doesn't really make sense.
ASSERT_LT(device_info.alignment_offset, device_info.alignment);
}
TEST(liblp, UpdateBlockDeviceInfo) {
- BlockDeviceInfo device_info(1024 * 1024, 4096, 1024);
+ BlockDeviceInfo device_info(1024 * 1024, 4096, 1024, 4096);
unique_ptr<MetadataBuilder> builder = MetadataBuilder::New(device_info, 1024, 1);
ASSERT_NE(builder, nullptr);
EXPECT_EQ(builder->block_device_info().size, device_info.size);
EXPECT_EQ(builder->block_device_info().alignment, device_info.alignment);
EXPECT_EQ(builder->block_device_info().alignment_offset, device_info.alignment_offset);
+ EXPECT_EQ(builder->block_device_info().logical_block_size, device_info.logical_block_size);
device_info.alignment = 0;
device_info.alignment_offset = 2048;
@@ -450,3 +452,27 @@
EXPECT_EQ(builder->block_device_info().alignment, 8192);
EXPECT_EQ(builder->block_device_info().alignment_offset, 2048);
}
+
+TEST(liblp, InvalidBlockSize) {
+ BlockDeviceInfo device_info(1024 * 1024, 0, 0, 513);
+ unique_ptr<MetadataBuilder> builder = MetadataBuilder::New(device_info, 1024, 1);
+ EXPECT_EQ(builder, nullptr);
+}
+
+TEST(liblp, AlignedExtentSize) {
+ BlockDeviceInfo device_info(1024 * 1024, 0, 0, 4096);
+ unique_ptr<MetadataBuilder> builder = MetadataBuilder::New(device_info, 1024, 1);
+ ASSERT_NE(builder, nullptr);
+
+ Partition* partition = builder->AddPartition("system", TEST_GUID, 0);
+ ASSERT_NE(partition, nullptr);
+ ASSERT_TRUE(builder->ResizePartition(partition, 512));
+ EXPECT_EQ(partition->size(), 4096);
+}
+
+TEST(liblp, AlignedFreeSpace) {
+ // Only one sector free - at least one block is required.
+ BlockDeviceInfo device_info(10240, 0, 0, 4096);
+ unique_ptr<MetadataBuilder> builder = MetadataBuilder::New(device_info, 512, 1);
+ ASSERT_EQ(builder, nullptr);
+}
diff --git a/fs_mgr/liblp/include/liblp/builder.h b/fs_mgr/liblp/include/liblp/builder.h
index 0f96e3a..e83b922 100644
--- a/fs_mgr/liblp/include/liblp/builder.h
+++ b/fs_mgr/liblp/include/liblp/builder.h
@@ -32,11 +32,16 @@
// By default, partitions are aligned on a 1MiB boundary.
static const uint32_t kDefaultPartitionAlignment = 1024 * 1024;
+static const uint32_t kDefaultBlockSize = 4096;
struct BlockDeviceInfo {
- BlockDeviceInfo() : size(0), alignment(0), alignment_offset(0) {}
- BlockDeviceInfo(uint64_t size, uint32_t alignment, uint32_t alignment_offset)
- : size(size), alignment(alignment), alignment_offset(alignment_offset) {}
+ BlockDeviceInfo() : size(0), alignment(0), alignment_offset(0), logical_block_size(0) {}
+ BlockDeviceInfo(uint64_t size, uint32_t alignment, uint32_t alignment_offset,
+ uint32_t logical_block_size)
+ : size(size),
+ alignment(alignment),
+ alignment_offset(alignment_offset),
+ logical_block_size(logical_block_size) {}
// Size of the block device, in bytes.
uint64_t size;
// Optimal target alignment, in bytes. Partition extents will be aligned to
@@ -46,6 +51,8 @@
// |alignment_offset| on the target device is correctly aligned on its
// parent device. This value must be 0 or a multiple of 512.
uint32_t alignment_offset;
+ // Block size, for aligning extent sizes and partition sizes.
+ uint32_t logical_block_size;
};
// Abstraction around dm-targets that can be encoded into logical partition tables.
@@ -88,6 +95,8 @@
};
class Partition final {
+ friend class MetadataBuilder;
+
public:
Partition(const std::string& name, const std::string& guid, uint32_t attributes);
@@ -97,10 +106,6 @@
// Remove all extents from this partition.
void RemoveExtents();
- // Remove and/or shrink extents until the partition is the requested size.
- // See MetadataBuilder::ShrinkPartition for more information.
- void ShrinkTo(uint64_t requested_size);
-
const std::string& name() const { return name_; }
uint32_t attributes() const { return attributes_; }
const std::string& guid() const { return guid_; }
@@ -108,6 +113,8 @@
uint64_t size() const { return size_; }
private:
+ void ShrinkTo(uint64_t aligned_size);
+
std::string name_;
std::string guid_;
std::vector<std::unique_ptr<Extent>> extents_;
@@ -144,7 +151,7 @@
// size. This is a convenience method for tests.
static std::unique_ptr<MetadataBuilder> New(uint64_t blockdev_size, uint32_t metadata_max_size,
uint32_t metadata_slot_count) {
- BlockDeviceInfo device_info(blockdev_size, 0, 0);
+ BlockDeviceInfo device_info(blockdev_size, 0, 0, kDefaultBlockSize);
return New(device_info, metadata_max_size, metadata_slot_count);
}
diff --git a/fs_mgr/liblp/include/liblp/metadata_format.h b/fs_mgr/liblp/include/liblp/metadata_format.h
index e1323e1..52c80f7 100644
--- a/fs_mgr/liblp/include/liblp/metadata_format.h
+++ b/fs_mgr/liblp/include/liblp/metadata_format.h
@@ -136,6 +136,12 @@
* can be used to verify the geometry against a target device.
*/
uint64_t block_device_size;
+
+ /* 76: Logical block size of the super partition block device. This is the
+ * minimal alignment for partition and extent sizes, and it must be a
+ * multiple of LP_SECTOR_SIZE.
+ */
+ uint32_t logical_block_size;
} __attribute__((packed)) LpMetadataGeometry;
/* The logical partition metadata has a number of tables; they are described
diff --git a/fs_mgr/liblp/utility_test.cpp b/fs_mgr/liblp/utility_test.cpp
index 092dbf1..7bf42ae 100644
--- a/fs_mgr/liblp/utility_test.cpp
+++ b/fs_mgr/liblp/utility_test.cpp
@@ -40,7 +40,8 @@
80000,
0,
0,
- 1024 * 1024};
+ 1024 * 1024,
+ 4096};
EXPECT_EQ(GetPrimaryMetadataOffset(geometry, 0), 4096);
EXPECT_EQ(GetPrimaryMetadataOffset(geometry, 1), 4096 + 16384);
EXPECT_EQ(GetPrimaryMetadataOffset(geometry, 2), 4096 + 16384 * 2);