linux-ext4.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2] ext4: detect invalid INLINE_DATA + EXTENTS flag combination
@ 2025-09-29 15:43 Deepanshu Kartikey
  2025-09-30  8:34 ` Zhang Yi
  0 siblings, 1 reply; 4+ messages in thread
From: Deepanshu Kartikey @ 2025-09-29 15:43 UTC (permalink / raw)
  To: tytso, adilger.kernel
  Cc: yi.zhang, linux-ext4, linux-kernel, Deepanshu Kartikey,
	syzbot+038b7bf43423e132b308, Zhang Yi

syzbot reported a BUG_ON in ext4_es_cache_extent() when opening a verity
file on a corrupted ext4 filesystem mounted without a journal.

The issue is that the filesystem has an inode with both the INLINE_DATA
and EXTENTS flags set:

    EXT4-fs error (device loop0): ext4_cache_extents:545: inode #15:
    comm syz.0.17: corrupted extent tree: lblk 0 < prev 66

Investigation revealed that the inode has both flags set:
    DEBUG: inode 15 - flag=1, i_inline_off=164, has_inline=1, extents_flag=1

This is an invalid combination since an inode should have either:
- INLINE_DATA: data stored directly in the inode
- EXTENTS: data stored in extent-mapped blocks

Having both flags causes ext4_has_inline_data() to return true, skipping
extent tree validation in __ext4_iget(). The unvalidated out-of-order
extents then trigger a BUG_ON in ext4_es_cache_extent() due to integer
underflow when calculating hole sizes.

Fix this by detecting this invalid flag combination early in ext4_iget()
and rejecting the corrupted inode.

Reported-and-tested-by: syzbot+038b7bf43423e132b308@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=038b7bf43423e132b308
Suggested-by: Zhang Yi <yi.zhang@huawei.com>
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
Changes in v2:
- Instead of adding validation in ext4_find_extent(), detect the invalid
  INLINE_DATA + EXTENTS flag combination in ext4_iget() as suggested by
  Zhang Yi to avoid redundant checks in the extent lookup path

 fs/ext4/inode.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 5b7a15db4953..71fa3faa1475 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5445,6 +5445,15 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
 	}
 
 	ret = 0;
+	/* Detect invalid flag combination - can't have both inline data and extents */
+	if (ext4_test_inode_flag(inode, EXT4_INODE_INLINE_DATA) &&
+		ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) {
+		ext4_error_inode(inode, __func__, __LINE__, 0,
+			"inode has both inline data and extents flags");
+		ret = -EFSCORRUPTED;
+		goto bad_inode;
+	}
+
 	if (ei->i_file_acl &&
 	    !ext4_inode_block_valid(inode, ei->i_file_acl, 1)) {
 		ext4_error_inode(inode, function, line, 0,
-- 
2.43.0

^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] ext4: detect invalid INLINE_DATA + EXTENTS flag combination
  2025-09-29 15:43 Deepanshu Kartikey
@ 2025-09-30  8:34 ` Zhang Yi
  0 siblings, 0 replies; 4+ messages in thread
From: Zhang Yi @ 2025-09-30  8:34 UTC (permalink / raw)
  To: Deepanshu Kartikey
  Cc: tytso, adilger.kernel, linux-ext4, linux-kernel,
	syzbot+038b7bf43423e132b308, Zhang Yi

On 9/29/2025 11:43 PM, Deepanshu Kartikey wrote:
> syzbot reported a BUG_ON in ext4_es_cache_extent() when opening a verity
> file on a corrupted ext4 filesystem mounted without a journal.
> 
> The issue is that the filesystem has an inode with both the INLINE_DATA
> and EXTENTS flags set:
> 
>     EXT4-fs error (device loop0): ext4_cache_extents:545: inode #15:
>     comm syz.0.17: corrupted extent tree: lblk 0 < prev 66
> 
> Investigation revealed that the inode has both flags set:
>     DEBUG: inode 15 - flag=1, i_inline_off=164, has_inline=1, extents_flag=1
> 
> This is an invalid combination since an inode should have either:
> - INLINE_DATA: data stored directly in the inode
> - EXTENTS: data stored in extent-mapped blocks
> 
> Having both flags causes ext4_has_inline_data() to return true, skipping
> extent tree validation in __ext4_iget(). The unvalidated out-of-order
> extents then trigger a BUG_ON in ext4_es_cache_extent() due to integer
> underflow when calculating hole sizes.
> 
> Fix this by detecting this invalid flag combination early in ext4_iget()
> and rejecting the corrupted inode.
> 
> Reported-and-tested-by: syzbot+038b7bf43423e132b308@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=038b7bf43423e132b308
> Suggested-by: Zhang Yi <yi.zhang@huawei.com>
> Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>

Thank you for debugging and thoroughly investigating this issue! This
patch overall looks good to me, with just a few minor suggestions.

> ---
> Changes in v2:
> - Instead of adding validation in ext4_find_extent(), detect the invalid
>   INLINE_DATA + EXTENTS flag combination in ext4_iget() as suggested by
>   Zhang Yi to avoid redundant checks in the extent lookup path
> 
>  fs/ext4/inode.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 5b7a15db4953..71fa3faa1475 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -5445,6 +5445,15 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
>  	}
>  
>  	ret = 0;
> +	/* Detect invalid flag combination - can't have both inline data and extents */
> +	if (ext4_test_inode_flag(inode, EXT4_INODE_INLINE_DATA) &&
> +		ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) {
	    ^^^
	    I'd recommended to maintain format alignment.

> +		ext4_error_inode(inode, __func__, __LINE__, 0,
		                        ^^^^^^^^^^^^^^^^^^
		                        function, line,> +			"inode has both inline data and extents flags");
> +		ret = -EFSCORRUPTED;
> +		goto bad_inode;
> +	}
> +

Additionally, I would prefer to move this check earlier, immediately
after setting the flags, that is, after ext4_set_inode_flags(). What
do you think?

Thanks,
Yi.

>  	if (ei->i_file_acl &&
>  	    !ext4_inode_block_valid(inode, ei->i_file_acl, 1)) {
>  		ext4_error_inode(inode, function, line, 0,


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] ext4: detect invalid INLINE_DATA + EXTENTS flag combination
@ 2025-09-30  9:02 Deepanshu Kartikey
  2025-09-30  9:15 ` Zhang Yi
  0 siblings, 1 reply; 4+ messages in thread
From: Deepanshu Kartikey @ 2025-09-30  9:02 UTC (permalink / raw)
  To: tytso, adilger.kernel, yi.zhang; +Cc: linux-ext4, linux-kernel

Zhang Yi,

Thank you for the review. Regarding the placement after ext4_set_inode_flags() - 
this would be too early. My debug shows that i_inline_off changes during inode 
initialization:

    After ext4_set_inode_flags(): flag=1, i_inline_off=0, has_inline=0
    Before my patch validation check: flag=1, i_inline_off=164, has_inline=1

At the earlier point, ext4_has_inline_data() returns false, so we wouldn't catch 
the corruption. The check needs to be after all inode fields are initialized.

I'll fix the alignment and use function/line variables as you suggested, but keep 
the check after "ret = 0;" where all inode fields are populated.

I'll send v3 with these fixes shortly.

Best regards,
Deepanshu

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] ext4: detect invalid INLINE_DATA + EXTENTS flag combination
  2025-09-30  9:02 [PATCH v2] ext4: detect invalid INLINE_DATA + EXTENTS flag combination Deepanshu Kartikey
@ 2025-09-30  9:15 ` Zhang Yi
  0 siblings, 0 replies; 4+ messages in thread
From: Zhang Yi @ 2025-09-30  9:15 UTC (permalink / raw)
  To: Deepanshu Kartikey; +Cc: tytso, adilger.kernel, linux-ext4, linux-kernel

On 9/30/2025 5:02 PM, Deepanshu Kartikey wrote:
> Zhang Yi,
> 
> Thank you for the review. Regarding the placement after ext4_set_inode_flags() - 
> this would be too early. My debug shows that i_inline_off changes during inode 
> initialization:
> 
>     After ext4_set_inode_flags(): flag=1, i_inline_off=0, has_inline=0
>     Before my patch validation check: flag=1, i_inline_off=164, has_inline=1
> 
> At the earlier point, ext4_has_inline_data() returns false, so we wouldn't catch 
> the corruption. The check needs to be after all inode fields are initialized.
> 

The return value of ext4_has_inline_data() changed since ext4_iget_extra_inode()
initialize the i_inline_off parameter from the ondisk inline xattr. However, in
your v2 patch, you checked the EXT4_INODE_INLINE_DATA flags directly instead of
the return value of ext4_has_inline_data(). The flags will not change, so it's
safe to move this check eralier, and it is more reasonable to directly check the
flags after getting flags from the disk and before checking inline xattr.

Thanks,
Yi.

> I'll fix the alignment and use function/line variables as you suggested, but keep 
> the check after "ret = 0;" where all inode fields are populated.
> 
> I'll send v3 with these fixes shortly.
> 
> Best regards,
> Deepanshu
> 


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-09-30  9:15 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-30  9:02 [PATCH v2] ext4: detect invalid INLINE_DATA + EXTENTS flag combination Deepanshu Kartikey
2025-09-30  9:15 ` Zhang Yi
  -- strict thread matches above, loose matches on Subject: below --
2025-09-29 15:43 Deepanshu Kartikey
2025-09-30  8:34 ` Zhang Yi

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).