Re: [PATCH] udf: fix partition descriptor append bookkeeping

[Jan Kara]

On Tue 10-03-26 17:16:52, Seohyeon Maeng wrote:
> 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.
> 
> Fixes: ee4af50ca94f ("udf: Fix mounting of Win7 created UDF filesystems")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Seohyeon Maeng <bioloidgp@xxxxxxxxx>

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

> ---
>  fs/udf/super.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index 27f463fd1d89..3c3c645de0bc 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -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
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR