Re: [PATCH v2] udf: validate extent partition references in udf_current_aext()

[Jan Kara]

On Fri 12-06-26 15:58:46, Kyle Zeng wrote:
> Long allocation descriptors carry an on-disk
> extLocation.partitionReferenceNum. udf_current_aext() copies that value
> into a kernel_lb_addr and returns it to several consumers.
> 
> If the partition reference is outside s_partitions, callers can later
> index s_partmaps out of bounds. The truncate/free path can pass such an
> extent to udf_free_blocks(), where the invalid partition reference
> causes a slab out-of-bounds read.
> 
> Validate eloc->partitionReferenceNum in udf_current_aext() before
> returning a decoded extent. This rejects invalid file extents and
> indirect allocation descriptor extents in the common parser, so callers
> do not need to duplicate the partition-map bounds check.
> 
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Kyle Zeng <kylebot@xxxxxxxxxx>

Thanks! I've added the patch to my tree (just deleting the spurious sbi
variable addition).

								Honza

> ---
>  fs/udf/inode.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/fs/udf/inode.c b/fs/udf/inode.c
> index 67bcf83..3a65b95 100644
> --- a/fs/udf/inode.c
> +++ b/fs/udf/inode.c
> @@ -2151,6 +2151,7 @@ void udf_write_aext(struct inode *inode, struct extent_position *epos,
>  	struct short_ad *sad;
>  	struct long_ad *lad;
>  	struct udf_inode_info *iinfo = UDF_I(inode);
> +	struct udf_sb_info *sbi = UDF_SB(inode->i_sb);
>  
>  	if (!epos->bh)
>  		ptr = iinfo->i_data + epos->offset -
> @@ -2299,6 +2300,12 @@ int udf_current_aext(struct inode *inode, struct extent_position *epos,
>  		return -EINVAL;
>  	}
>  
> +	if (eloc->partitionReferenceNum >= sbi->s_partitions) {
> +		udf_debug("invalid partition reference %u (partitions %u)\n",
> +			  eloc->partitionReferenceNum, sbi->s_partitions);
> +		return -EFSCORRUPTED;
> +	}
> +
>  	return 1;
>  }
>  
> -- 
> 2.54.0
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR