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

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

[Hui Peng]

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@gmail.com>
---
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);

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@gmail.com>

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@suse.com>
SUSE Labs, CR