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

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

[Deepanshu Kartikey]

ext4_read_inline_data() reads the location of an inline data xattr
value directly from entry->e_value_offs without validating it against
the actual bounds of the inode's xattr area. A corrupted filesystem
image can set e_value_offs to an out-of-range value, causing the
subsequent memcpy() to read from an address far outside the inode
buffer, including memory that has already been freed and reused for
something else. This mirrors the check already performed in
ext4_xattr_ibody_get(), which is missing here.

Add a bounds check on the computed source pointer against the end of
the inode's xattr area before the memcpy, and reject the read with
-EFSCORRUPTED if it would go out of bounds.

Fixes: 67cf5b09a46f ("ext4: add the basic function for inline data support")
Reported-by: syzbot+085a394c92518a04fd09@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=085a394c92518a04fd09
Tested-by: syzbot+085a394c92518a04fd09@syzkaller.appspotmail.com
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 fs/ext4/inline.c | 14 ++++++++++++--
 1 file changed, 12 insertions(+), 2 deletions(-)

diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
index ceee69a66482..2e60ab3e0db5 100644
--- a/fs/ext4/inline.c
+++ b/fs/ext4/inline.c
@@ -187,6 +187,8 @@ static int ext4_read_inline_data(struct inode *inode, void *buffer,
 	struct ext4_xattr_ibody_header *header;
 	int cp_len = 0;
 	struct ext4_inode *raw_inode;
+	void *end, *p;
+	u16 offset;
 
 	if (!len)
 		return 0;
@@ -205,13 +207,21 @@ static int ext4_read_inline_data(struct inode *inode, void *buffer,
 		goto out;
 
 	header = IHDR(inode, raw_inode);
+	end = ITAIL(inode, raw_inode);
 	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;
+	}
+
+	memcpy(buffer, p, len);
 	cp_len += len;
 
 out:
-- 
2.43.0

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

[sashiko-bot]

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

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

[Jan Kara]

On Fri 18-09-26 06:35:58, Deepanshu Kartikey wrote:
> ext4_read_inline_data() reads the location of an inline data xattr
> value directly from entry->e_value_offs without validating it against
> the actual bounds of the inode's xattr area. A corrupted filesystem
> image can set e_value_offs to an out-of-range value, causing the
> subsequent memcpy() to read from an address far outside the inode
> buffer, including memory that has already been freed and reused for
> something else. This mirrors the check already performed in
> ext4_xattr_ibody_get(), which is missing here.
> 
> Add a bounds check on the computed source pointer against the end of
> the inode's xattr area before the memcpy, and reject the read with
> -EFSCORRUPTED if it would go out of bounds.
> 
> Fixes: 67cf5b09a46f ("ext4: add the basic function for inline data support")
> Reported-by: syzbot+085a394c92518a04fd09@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=085a394c92518a04fd09
> Tested-by: syzbot+085a394c92518a04fd09@syzkaller.appspotmail.com
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>

Looks good. Feel free to add:

Reviewed-by: Jan Kara <jack@suse.cz>

								Honza

> ---
>  fs/ext4/inline.c | 14 ++++++++++++--
>  1 file changed, 12 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/ext4/inline.c b/fs/ext4/inline.c
> index ceee69a66482..2e60ab3e0db5 100644
> --- a/fs/ext4/inline.c
> +++ b/fs/ext4/inline.c
> @@ -187,6 +187,8 @@ static int ext4_read_inline_data(struct inode *inode, void *buffer,
>  	struct ext4_xattr_ibody_header *header;
>  	int cp_len = 0;
>  	struct ext4_inode *raw_inode;
> +	void *end, *p;
> +	u16 offset;
>  
>  	if (!len)
>  		return 0;
> @@ -205,13 +207,21 @@ static int ext4_read_inline_data(struct inode *inode, void *buffer,
>  		goto out;
>  
>  	header = IHDR(inode, raw_inode);
> +	end = ITAIL(inode, raw_inode);
>  	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;
> +	}
> +
> +	memcpy(buffer, p, len);
>  	cp_len += len;
>  
>  out:
> -- 
> 2.43.0
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR