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

[Gao Xiang]

On 2026/3/17 10:16, Gao Xiang wrote:
> 
> 
> On 2026/3/17 04:19, Utkal Singh wrote:
>> erofs_init_inode_xattrs() reads h_shared_count directly from the on-disk
>> xattr ibody header and uses it to size a heap allocation and drive a
>> read loop without checking whether the implied shared xattr array fits
>> within xattr_isize.
>>
>> A crafted EROFS image with a large h_shared_count but a minimal
>> xattr_isize causes the subsequent loop to read shared xattr entries
>> beyond the xattr ibody boundary, interpreting unrelated on-disk data
>> as shared xattr IDs.  This affects every library consumer -- dump.erofs,
>> erofsfuse, and the rebuild path (lib/rebuild.c) -- none of which call
>> the fsck-only erofs_verify_xattr() before reaching this code.
> 
> I don't think other than fsck tool, this must be checked, since it
> won't cause any harmful behavior but the filesystem image is already
> corrupted, and because of the corruption, the user should get the
> corrupted result, but it still have no impact to the whole system
> stablity.

Nevertheless, I'm fine if we try to harden this part, but the commit
message should clarify the impact: it actually has no stability impact
out of those images.

Also there are many threads with your contribution, it's hard for me
to follow those threads, now.

Would you mind raising a github issue
https://github.com/erofs/erofs-utils/issues

and list all your patches for merging (with meaningful topics are
preferred) ?

> 
>>
>> Validate that h_shared_count fits within the available xattr body space
>> before allocating or reading.  Use a division-based check to avoid any
>> theoretical overflow in the multiplication.
> 
> I don't think it will overflow according to the ondisk format.
> 
>>
>> The subtraction is safe because callers above already reject
>> xattr_isize < sizeof(struct erofs_xattr_ibody_header).
> 
> Please add a reproducible way.
> 
>>
>> Signed-off-by: Utkal Singh <singhutkal015 at gmail.com>
>> ---
>>   lib/xattr.c | 10 ++++++++++
>>   1 file changed, 10 insertions(+)
>>
>> diff --git a/lib/xattr.c b/lib/xattr.c
>> index 565070a..6891812 100644
>> --- a/lib/xattr.c
>> +++ b/lib/xattr.c
>> @@ -1182,6 +1182,16 @@ static int erofs_init_inode_xattrs(struct erofs_inode *vi)
>>       ih = it.kaddr;
>>       vi->xattr_shared_count = ih->h_shared_count;
>> +    /* validate h_shared_count fits within xattr_isize */
>> +    if (vi->xattr_shared_count >
>> +        (vi->xattr_isize - sizeof(struct erofs_xattr_ibody_header)) /
>> +            sizeof(u32)) {
> 
> Can we avoid division?
> 
>> +        erofs_err("bogus h_shared_count %u (xattr_isize %u) @ nid %llu",
>> +              vi->xattr_shared_count, vi->xattr_isize,
>> +              vi->nid | 0ULL);
>> +        erofs_put_metabuf(&it.buf);
>> +        return -EFSCORRUPTED;
>> +    }
>>       vi->xattr_shared_xattrs = malloc(vi->xattr_shared_count * sizeof(uint));
>>       if (!vi->xattr_shared_xattrs) {
>>           erofs_put_metabuf(&it.buf);
>