Skip to contentSkip to conversation

dm vdo: additional metadata validation

PATCH New Matthew Sakai 1 participant 3 messages Feb 24

Subsystems (2)
lore
Share & tools
lore

Matthew Sakai

author maintainer
#0 lore raw

v2 cover dm vdo: additional metadata validation

We identified a few places where dm-vdo uses a value from storage without sanity-checking that its value makes sense. Adding additional checks can prevent additional metadata corruption in the unusual cases where these values get corrupted.

Changes from v1:
1/2 Fixed the upper bound check to handle MAX_ZONES correctly. 1/2 Added a lower bound check for the region count.

Matthew Sakai (2):
dm vdo indexer: validate saved region and zone counts dm vdo slab-depot: validate old zone count on load

 drivers/md/dm-vdo/indexer/index-layout.c | 9 ++++++++-
 drivers/md/dm-vdo/slab-depot.c           | 4 ++++
 2 files changed, 12 insertions(+), 1 deletion(-)
Signature · 1 line
2.48.1
author maintainer
#1 lore raw

PATCH v2 1/2 dm vdo indexer: validate saved region and zone counts

Verify that the loaded zone count is in the valid range before using it as a loop iterator. Also validate the region_count to catch cases where too few regions are defined.

---
 drivers/md/dm-vdo/indexer/index-layout.c | 9 ++++++++-
 1 file changed, 8 insertions(+), 1 deletion(-)
drivers/md/dm-vdo/indexer/index-layout.c +8 -1
index 61edf2b72427..90a2e4b7345c 100644
@@ -1444,8 +1444,11 @@ static int __must_check reconstruct_index_save(struct index_save_layout *isl,
 	u64 next_block = isl->index_save.start_block;
 	u64 last_block = next_block + isl->index_save.block_count;
 
-	isl->zone_count = table->header.region_count - 3;
+	if (table->header.region_count < 4)
+		return vdo_log_error_strerror(UDS_CORRUPT_DATA,
+					      "invalid region count");
 
+	isl->zone_count = table->header.region_count - 3;
 	last_region = &table->regions[table->header.region_count - 1];
 	if (last_region->kind == RL_KIND_EMPTY) {
 		isl->free_space = *last_region;
@@ -1459,6 +1462,10 @@ static int __must_check reconstruct_index_save(struct index_save_layout *isl,
 		};
 	}
 
+	if (isl->zone_count > MAX_ZONES)
+		return vdo_log_error_strerror(UDS_CORRUPT_DATA,
+					      "invalid zone count");
+
 	isl->header = table->regions[0];
 	result = verify_region(&isl->header, next_block++, RL_KIND_HEADER,
 			       RL_SOLE_INSTANCE);
Signature · 1 line
2.48.1
author maintainer
#2 lore raw

PATCH v2 2/2 dm vdo slab-depot: validate old zone count on load

In reply to Matthew Sakai

Verify the old zone count has a valid value before using it to compute slab summary entry offsets.

---
 drivers/md/dm-vdo/slab-depot.c | 4 ++++
 1 file changed, 4 insertions(+)
drivers/md/dm-vdo/slab-depot.c +4 -0
index 034ecaa51f48..ad00afc2c168 100644
@@ -4262,6 +4262,10 @@ int vdo_decode_slab_depot(struct slab_depot_state_2_0 state, struct vdo *vdo,
 	}
 	slab_size_shift = ilog2(slab_size);
 
+	if (state.zone_count > MAX_VDO_PHYSICAL_ZONES)
+		return vdo_log_error_strerror(UDS_CORRUPT_DATA,
+					      "invalid zone count");
+
 	result = vdo_allocate_extended(struct slab_depot,
 				       vdo->thread_config.physical_zone_count,
 				       struct block_allocator, __func__, &depot);
Signature · 1 line
2.48.1