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

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