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

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

[Utkal Singh]

`u8 h_shared_count` indicates the shared xattr count of an inode. It is
read from the on-disk xattr ibody header, which should be corrupted if
the size of the shared xattr array exceeds the space available in
`xattr_isize`.

It does not cause harmful consequence (e.g. crashes), since the image is
already considered corrupted, it indeed results in the silent processing
of garbage metadata.

Let's harden it to report -EFSCORRUPTED earlier.

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 | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/lib/xattr.c b/lib/xattr.c
index 565070a..9d52a18 100644
--- a/lib/xattr.c
+++ b/lib/xattr.c
@@ -1182,6 +1182,13 @@ 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 * sizeof(__le32) >
+	    vi->xattr_isize - sizeof(struct erofs_xattr_ibody_header)) {
+		erofs_err("invalid h_shared_count %u in nid %llu",
+			  vi->xattr_shared_count, 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);
-- 
2.43.0

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

[Gao Xiang]

On 2026/3/17 23:15, Utkal Singh wrote:
> `u8 h_shared_count` indicates the shared xattr count of an inode. It is
> read from the on-disk xattr ibody header, which should be corrupted if
> the size of the shared xattr array exceeds the space available in
> `xattr_isize`.
> 
> It does not cause harmful consequence (e.g. crashes), since the image is
> already considered corrupted, it indeed results in the silent processing
> of garbage metadata.
> 
> Let's harden it to report -EFSCORRUPTED earlier.
> 
> 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 | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/lib/xattr.c b/lib/xattr.c
> index 565070a..9d52a18 100644
> --- a/lib/xattr.c
> +++ b/lib/xattr.c
> @@ -1182,6 +1182,13 @@ 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 * sizeof(__le32) >
> +	    vi->xattr_isize - sizeof(struct erofs_xattr_ibody_header)) {
> +		erofs_err("invalid h_shared_count %u in nid %llu",

I think this line is incorrect (did you compile at least?), also
you need to Cc LKML <linux-kernel@vger.kernel.org> for all
kernel patches.

Thanks,
Gao Xiang

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

[Gao Xiang]

On 2026/3/17 23:25, Gao Xiang wrote:
> 
> 
> On 2026/3/17 23:15, Utkal Singh wrote:
>> `u8 h_shared_count` indicates the shared xattr count of an inode. It is
>> read from the on-disk xattr ibody header, which should be corrupted if
>> the size of the shared xattr array exceeds the space available in
>> `xattr_isize`.
>>
>> It does not cause harmful consequence (e.g. crashes), since the image is
>> already considered corrupted, it indeed results in the silent processing
>> of garbage metadata.
>>
>> Let's harden it to report -EFSCORRUPTED earlier.
>>
>> 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 | 7 +++++++
>>   1 file changed, 7 insertions(+)
>>
>> diff --git a/lib/xattr.c b/lib/xattr.c
>> index 565070a..9d52a18 100644
>> --- a/lib/xattr.c
>> +++ b/lib/xattr.c
>> @@ -1182,6 +1182,13 @@ 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 * sizeof(__le32) >
>> +        vi->xattr_isize - sizeof(struct erofs_xattr_ibody_header)) {
>> +        erofs_err("invalid h_shared_count %u in nid %llu",
> 
> I think this line is incorrect (did you compile at least?), also
> you need to Cc LKML <linux-kernel@vger.kernel.org> for all
> kernel patches.

Sorry, I misread it, but the patch version number is quite confusing.

Thanks,
Gao Xiang

> 
> Thanks,
> Gao Xiang