Re: [PATCH] erofs-utils: lib: validate h_shared_count in erofs_init_inode_xattrs()

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

On 2026/3/17 19:17, Utkal Singh wrote:
> erofs_init_inode_xattrs() reads h_shared_count from the on-disk xattr
> ibody header and uses it to size a malloc and drive a loop that reads
> shared xattr IDs.  If h_shared_count exceeds the space available
> within xattr_isize, the loop reads past the intended ibody region
> and the malloc is oversized.
> 
> Validate that h_shared_count does not exceed the number of __le32
> entries that fit after the ibody header.  Return -EFSCORRUPTED with
> a diagnostic message on failure.
> 
> Reproducer:
>    mkdir testdir && echo hello > testdir/a.txt
>    setfattr -n user.test -v val testdir/a.txt
>    mkfs.erofs test.img testdir
>    # corrupt h_shared_count (offset = nid*32 + inode_size + 4) to 0xFF
>    # then: fsck.erofs --extract=/tmp/out --xattrs test_corrupted.img
>    # Without patch: silently processes invalid shared xattr IDs
>    # With patch: returns -EFSCORRUPTED
> 
> Signed-off-by: Utkal Singh <singhutkal015@gmail.com>
> ---
>   lib/xattr.c | 8 ++++++++
>   1 file changed, 8 insertions(+)
> 
> diff --git a/lib/xattr.c b/lib/xattr.c
> index 565070a..5888602 100644
> --- a/lib/xattr.c
> +++ b/lib/xattr.c
> @@ -1182,6 +1182,14 @@ static int erofs_init_inode_xattrs(struct erofs_inode *vi)
>   
>   	ih = it.kaddr;
>   	vi->xattr_shared_count = ih->h_shared_count;
> +	if (vi->xattr_shared_count >
> +	    (vi->xattr_isize - sizeof(struct erofs_xattr_ibody_header)) /
> +	    sizeof(__le32)) {
> +		erofs_err("invalid h_shared_count %u in nid %llu",
> +			  vi->xattr_shared_count, vi->nid | 0ULL);
> +		erofs_put_metabuf(&it.buf);
> +		return -EFSCORRUPTED;
> +	}

Again, why not `vi->xattr_shared_count * sizeof(__le32) >
	vi->xattr_isize - sizeof(struct erofs_xattr_ibody_header)`?

Thanks,
Gao Xiang