udf: fix partition descriptor append bookkeeping
Subsystems (2)
PATCH 1/1 udf: fix partition descriptor append bookkeeping
Mounting a crafted UDF image with repeated partition descriptors can trigger a heap out-of-bounds write in part_descs_loc[].
handle_partition_descriptor() deduplicates entries by partition number, but appended slots never record partnum. As a result duplicate Partition Descriptors are appended repeatedly and num_part_descs keeps growing.
Once the table is full, the growth path still sizes the allocation from partnum even though inserts are indexed by num_part_descs. If partnum is already aligned to PART_DESC_ALLOC_STEP, ALIGN(partnum, step) can keep the old capacity and the next append writes past the end of the table.
Store partnum in the appended slot and size growth from the next append count so deduplication and capacity tracking follow the same model.
--- fs/udf/super.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)
fs/udf/super.c +3 -1
@@ -1694,7 +1694,8 @@ static struct udf_vds_record *handle_partition_descriptor(return &(data->part_descs_loc[i].rec);if (data->num_part_descs >= data->size_part_descs) {struct part_desc_seq_scan_data *new_loc; -unsigned int new_size =ALIGN (partnum,PART_DESC_ALLOC_STEP ); +unsigned int need = data->num_part_descs +1 ; +unsigned int new_size =ALIGN (need,PART_DESC_ALLOC_STEP ); new_loc =kzalloc_objs (*new_loc, new_size);if (!new_loc)
@@ -1705,6 +1706,7 @@ static struct udf_vds_record *handle_partition_descriptor( data->part_descs_loc = new_loc; data->size_part_descs = new_size; } + data->part_descs_loc[data->num_part_descs].partnum = partnum;return &(data->part_descs_loc[data->num_part_descs++].rec); }
base-commit: 1f318b96cc84d7c2ab792fcc0bfd42a7ca890681
Signature · 1 line
2.43.0
Quoted 13 lines · Seohyeon Maeng
Mounting a crafted UDF image with repeated partition descriptors can trigger a heap out-of-bounds write in part_descs_loc[].
handle_partition_descriptor()deduplicates entries by partition number, but appended slots never record partnum. As a result duplicate Partition Descriptors are appended repeatedly andnum_part_descskeeps growing.Once the table is full, the growth path still sizes the allocation from partnum even though inserts are indexed by
num_part_descs. If partnum is already aligned toPART_DESC_ALLOC_STEP,ALIGN(partnum, step)can keep the old capacity and the next append writes past the end of the table.
Quoted 6 lines
Store partnum in the appended slot and size growth from the next append count so deduplication and capacity tracking follow the same model.
Fixes ee4af50ca94f ("udf: Fix mounting of Win7 created UDF filesystems")Signed-off-by Seohyeon Maeng <[email protected]>
Thanks! I've merged the fix to my tree with a slight modification to set:
new_size = data->num_part_descs + PART_DESC_ALLOC_STEP;
instead of the +1 and ALIGN() calls which were mostly pointless.
Honza
Quoted 31 lines
--- fs/udf/super.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-)fs/udf/super.c +3
-1@@ -1694,7 +1694,8 @@ static struct udf_vds_record *handle_partition_descriptor(return &(data->part_descs_loc[i].rec);if (data->num_part_descs >= data->size_part_descs) {struct part_desc_seq_scan_data *new_loc; -unsigned int new_size =ALIGN (partnum,PART_DESC_ALLOC_STEP ); +unsigned int need = data->num_part_descs +1 ; +unsigned int new_size =ALIGN (need,PART_DESC_ALLOC_STEP ); new_loc =kzalloc_objs (*new_loc, new_size);if (!new_loc)@@ -1705,6 +1706,7 @@ static struct udf_vds_record *handle_partition_descriptor( data->part_descs_loc = new_loc; data->size_part_descs = new_size; } + data->part_descs_loc[data->num_part_descs].partnum = partnum;return &(data->part_descs_loc[data->num_part_descs++].rec); }base-commit:
1f318b96cc84d7c2ab792fcc0bfd42a7ca890681Signature · 1 line
2.43.0
Signature · 2 lines
Jan Kara <[email protected]> SUSE Labs, CR