Re: [PATCH] ext4: add bounds check for e_value_offs in ext4_read_inline_data

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] ext4: information leak due to unhandled error in ext4_read_inline_page()
- [Low] ext4: undefined macro ITAIL causes compilation failure
--

commit 61b118bf302b061ec089fd01a981cb9991fa3a62
Author: Deepanshu Kartikey <kartikey406@gmail.com>
ext4: add bounds check for e_value_offs in ext4_read_inline_data

This patch adds a bounds check to ext4_read_inline_data() to validate the
location of inline data xattr values against the actual bounds of the inode's
xattr area. If the value is out of bounds, it returns -EFSCORRUPTED to
prevent reading out-of-bounds memory.

Closes: https://syzkaller.appspot.com/bug?extid=085a394c92518a04fd09

> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index caa51473207d7..7d5592030ec32 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c

[ ... ]

> @@ -188,13 +190,21 @@ static int ext4_read_inline_data(struct inode *inode, void *buffer,
>  		goto out;
>  
>  	header = IHDR(inode, raw_inode);
> +	end = ITAIL(inode, raw_inode);

[Severity: Low]
Does this code compile? ITAIL() appears to be undefined in this context,
which would break the build.

>  	entry = (struct ext4_xattr_entry *)((void *)raw_inode +
>  					    EXT4_I(inode)->i_inline_off);
>  	len = min_t(unsigned int, len,
>  		    (unsigned int)le32_to_cpu(entry->e_value_size));
>  
> -	memcpy(buffer,
> -	       (void *)IFIRST(header) + le16_to_cpu(entry->e_value_offs), len);
> +	offset = le16_to_cpu(entry->e_value_offs);
> +	p = (void *)IFIRST(header) + offset;
> +
> +	if (unlikely(p + len > end)) {
> +		EXT4_ERROR_INODE(inode, "corrupt inline xattr entry");
> +		return -EFSCORRUPTED;

[Severity: High]
Does returning an error code here cause an information leak?

Looking at ext4_read_inline_page(), it calls this function but does not
check if the return value is negative before unconditionally zeroing the tail
and marking the page as up-to-date:

fs/ext4/inline.c:ext4_read_inline_page() {
    ...
    ret = ext4_read_inline_data(inode, kaddr, len, &iloc);
    flush_dcache_page(page);
    kunmap_atomic(kaddr);
    zero_user_segment(page, len, PAGE_SIZE);
    SetPageUptodate(page);
    ...
}

If this returns -EFSCORRUPTED, will the page be left with uninitialized
kernel memory that gets exposed to userspace?

> +	}
> +
> +	memcpy(buffer, p, len);
>  	cp_len += len;
>  
>  out:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260918010558.8210-1-kartikey406@gmail.com?part=1