Re: [PATCH] udf: validate vat20->lengthHeader and VAT inode size in udf_load_vat()

[Jan Kara]

On Sat 19-09-26 22:26:00, Hui Peng wrote:
> In udf_load_vat() (fs/udf/super.c), validate that vat20->lengthHeader is
> at least sizeof(struct virtualAllocationTable20) and does not exceed the
> mapped buffer or VAT inode size so computing the VAT table pointer and
> entry count cannot underflow or read out of bounds.
> 
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@xxxxxxxxx>

Thanks. The i_lenAlloc checks are pointless - we verify i_size matches
i_lenAlloc when loading the inode for IN_ICB inodes. But otherwise the
patch looks good so I've added it to my tree without these checks.

								Honza

> ---
> diff --git a/fs/udf/super.c b/fs/udf/super.c
> index 2ba5973ef4dd..2f0f087bcced 100644
> --- a/fs/udf/super.c
> +++ b/fs/udf/super.c
> @@ -1252,6 +1252,15 @@ static int udf_load_vat(struct super_block *sb, int p_index, int type1_index)
>  			(sbi->s_vat_inode->i_size - 36) >> 2;
>  	} else if (map->s_partition_type == UDF_VIRTUAL_MAP20) {
>  		vati = UDF_I(sbi->s_vat_inode);
> +		if (sbi->s_vat_inode->i_size <
> +			sizeof(struct virtualAllocationTable20) ||
> +		    (vati->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB &&
> +		     vati->i_lenAlloc <
> +			sizeof(struct virtualAllocationTable20))) {
> +			udf_err(sb, "Too short VAT inode size %lld\n",
> +				sbi->s_vat_inode->i_size);
> +			return -EFSCORRUPTED;
> +		}
>  		if (vati->i_alloc_type != ICBTAG_FLAG_AD_IN_ICB) {
>  			int err = 0;
>  
> @@ -1269,8 +1278,15 @@ static int udf_load_vat(struct super_block *sb, int p_index, int type1_index)
>  
>  		map->s_type_specific.s_virtual.s_start_offset =
>  			le16_to_cpu(vat20->lengthHeader);
> -		if (map->s_type_specific.s_virtual.s_start_offset
> -		    > sbi->s_vat_inode->i_size) {
> +		if (map->s_type_specific.s_virtual.s_start_offset <
> +			sizeof(struct virtualAllocationTable20) ||
> +		    map->s_type_specific.s_virtual.s_start_offset >
> +			sb->s_blocksize ||
> +		    map->s_type_specific.s_virtual.s_start_offset >
> +			sbi->s_vat_inode->i_size ||
> +		    (vati->i_alloc_type == ICBTAG_FLAG_AD_IN_ICB &&
> +		     map->s_type_specific.s_virtual.s_start_offset >
> +			vati->i_lenAlloc)) {
>  			udf_err(sb, "Corrupted VAT header length %u (VAT inode size %lld)\n",
>  				map->s_type_specific.s_virtual.s_start_offset,
>  				sbi->s_vat_inode->i_size);
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR