Re: [PATCH] erofs-utils: lib: validate z_extents against inode size

[Gao Xiang <hsiangkao@linux.alibaba.com>]

On 2026/3/17 16:21, Utkal Singh wrote:
> z_extents is read from on-disk metadata and used as the upper bound
> for extent lookups in z_erofs_map_blocks_ext().  A corrupted value
> can be arbitrarily large (up to 2^48-1), causing erofs_read_metabuf()
> to access offsets far beyond the actual extent table.  The resulting
> garbage is parsed as z_erofs_extent records, leading to wrong physical
> addresses used for I/O, silent data corruption, or crashes.

No, I don't think it needs to be fixed since it won't cause any
harmful behavior if the image is already corrupted.

Please don't submit any patches like this if you don't have any
reproducer and i_size is too long can cause overly long time,
but the image can be valid.


> 
> Since each extent covers at least one logical cluster, the extent
> count cannot exceed DIV_ROUND_UP(i_size, 1 << z_lclusterbits).
> Validate z_extents against this bound at inode initialization time
> and reject invalid values with -EFSCORRUPTED.
> 
> Signed-off-by: Utkal Singh <singhutkal015@gmail.com>
> ---
>   lib/zmap.c | 18 ++++++++++++++++++
>   1 file changed, 18 insertions(+)
> 
> diff --git a/lib/zmap.c b/lib/zmap.c
> index 0e7af4e..2f679b7 100644
> --- a/lib/zmap.c
> +++ b/lib/zmap.c
> @@ -675,8 +675,26 @@ static int z_erofs_fill_inode_lazy(struct erofs_inode *vi)
>   	vi->z_lclusterbits = sbi->blkszbits + (h->h_clusterbits & 15);
>   	if (vi->datalayout == EROFS_INODE_COMPRESSED_FULL &&
>   	    (vi->z_advise & Z_EROFS_ADVISE_EXTENTS)) {
> +		u64 max_extents;
> +
>   		vi->z_extents = le32_to_cpu(h->h_extents_lo) |
>   			((u64)le16_to_cpu(h->h_extents_hi) << 32);
> +
> +		/*
> +		 * Each extent covers at least one logical cluster, so
> +		 * the extent count must not exceed the number of lclusters.
> +		 * Reject bogus values to prevent out-of-bounds metadata
> +		 * reads in z_erofs_map_blocks_ext().
> +		 */
> +		max_extents = DIV_ROUND_UP(vi->i_size,
> +					   1ULL << vi->z_lclusterbits);

extents can be within 1ULL << vi->z_lclusterbits instead,
I don't think it's valid.

`up to 2^48-1 extents` is fine and valid, the users can
always interrupt this at any time.

Thanks,
Gao Xiang

> +		if (vi->z_extents > max_extents) {
> +			erofs_err("bogus z_extents %llu (max %llu) for nid %llu",
> +				  vi->z_extents | 0ULL, max_extents | 0ULL,
> +				  vi->nid | 0ULL);
> +			err = -EFSCORRUPTED;
> +			goto out_put_metabuf;
> +		}
>   		goto done;
>   	}
>   	vi->z_algorithmtype[0] = h->h_algorithmtype & 15;