public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] ext4: reject inline data flag when i_extra_isize is zero
@ 2025-10-02 10:41 Deepanshu Kartikey
  2025-10-02 19:57 ` Theodore Ts'o
  0 siblings, 1 reply; 3+ messages in thread
From: Deepanshu Kartikey @ 2025-10-02 10:41 UTC (permalink / raw)
  To: tytso, adilger.kernel
  Cc: linux-ext4, linux-kernel, Deepanshu Kartikey,
	syzbot+3ee481e21fd75e14c397

Prevent use-after-free in ext4_search_dir by rejecting inodes that
claim to have inline data but have no extra inode space allocated.

ext4 inline data is stored in the extra inode space beyond the
standard 128-byte inode structure. This requires i_extra_isize to be
non-zero to provide space for the system.data xattr that stores the
inline directory entries or file data.

However, a corrupted filesystem can craft an inode with both:
- i_extra_isize == 0 (no extra space)
- EXT4_INODE_INLINE_DATA flag set (claims to use extra space)

This creates a fundamental inconsistency. When i_extra_isize is zero,
ext4_iget() skips calling ext4_iget_extra_inode(), which means the
inline xattr validation in check_xattrs() never runs. Later, when
ext4_find_inline_entry() attempts to access the inline data, it reads
unvalidated and potentially corrupt xattr structures, leading to
out-of-bounds memory access and use-after-free.

Fix this by validating in ext4_iget() that if an inode has the
EXT4_INODE_INLINE_DATA flag set, i_extra_isize must be non-zero.
This catches the corruption at inode load time before any inline
data operations are attempted.

Reported-by: syzbot+3ee481e21fd75e14c397@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=3ee481e21fd75e14c397
Tested-by: syzbot+3ee481e21fd75e14c397@syzkaller.appspotmail.com
Signed-off-by: Deepanshu Kartikey <kartikey406@gmail.com>
---
 fs/ext4/inode.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 5b7a15db4953..d082fff675ac 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5417,6 +5417,12 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
 
 	if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE) {
 		if (ei->i_extra_isize == 0) {
+			if (ext4_has_inline_data(inode)) {
+				ext4_error_inode(inode, function, line, 0,
+						 "inline data flag set but i_extra_isize is zero");
+				ret = -EFSCORRUPTED;
+				goto bad_inode;
+			}
 			/* The extra space is currently unused. Use it. */
 			BUILD_BUG_ON(sizeof(struct ext4_inode) & 3);
 			ei->i_extra_isize = sizeof(struct ext4_inode) -
-- 
2.43.0


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

* Re: [PATCH] ext4: reject inline data flag when i_extra_isize is zero
  2025-10-02 10:41 [PATCH] ext4: reject inline data flag when i_extra_isize is zero Deepanshu Kartikey
@ 2025-10-02 19:57 ` Theodore Ts'o
  0 siblings, 0 replies; 3+ messages in thread
From: Theodore Ts'o @ 2025-10-02 19:57 UTC (permalink / raw)
  To: Deepanshu Kartikey
  Cc: adilger.kernel, linux-ext4, linux-kernel,
	syzbot+3ee481e21fd75e14c397

On Thu, Oct 02, 2025 at 04:11:51PM +0530, Deepanshu Kartikey wrote:
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 5b7a15db4953..d082fff675ac 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -5417,6 +5417,12 @@ struct inode *__ext4_iget(struct super_block *sb, unsigned long ino,
>  
>  	if (EXT4_INODE_SIZE(inode->i_sb) > EXT4_GOOD_OLD_INODE_SIZE) {
>  		if (ei->i_extra_isize == 0) {
> +			if (ext4_has_inline_data(inode)) {

I'm not sure how we would ever enter this code code branch?
ext4_has_inline_data() is defind as follows:

	return ext4_test_inode_flag(inode, EXT4_INODE_INLINE_DATA) &&
	       EXT4_I(inode)->i_inline_off;

Sure, the inode can have the INLINE_DATA flag set, and if i_extra_size
is zero, that's an impossible situation modulo a deliberately,
maliciously corrupted file system.

But there's also the requiremnt that i_inline_off is non-zero, and at
this point in ext4_iget(), i_inline_off should still be 0.  So how
does this work?

If the instead of ext4_has_inline_data(inode), this were changed to
ext4_test_inode_flag(inode, EXT4_INODE_INLINE_ATA), this would make
sense to me.  But given that you tested this with sybot and aparently
it prevented the reprducer from triggering the issue --- this worries
me, and makes me wonder what we're missing?

We should also make sure that a test file system with this corruption
is also repaired by e2fsck.

					- Ted


> +				ext4_error_inode(inode, function, line, 0,
> +						 "inline data flag set but i_extra_isize is zero");
> +				ret = -EFSCORRUPTED;
> +				goto bad_inode;
> +			}
>  			/* The extra space is currently unused. Use it. */
>  			BUILD_BUG_ON(sizeof(struct ext4_inode) & 3);
>  			ei->i_extra_isize = sizeof(struct ext4_inode) -
> -- 
> 2.43.0
> 

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

* Re: [PATCH] ext4: reject inline data flag when i_extra_isize is zero
@ 2025-10-03  0:51 Deepanshu Kartikey
  0 siblings, 0 replies; 3+ messages in thread
From: Deepanshu Kartikey @ 2025-10-03  0:51 UTC (permalink / raw)
  To: tytso; +Cc: adilger.kernel, linux-ext4, linux-kernel


Looking at the syzbot logs more carefully, I see the initial 'acceptance' was 
because the filesystem failed to mount ('VFS: Can't find ext4 filesystem'), 
so the bug couldn't be triggered at all. My patches were never actually tested 
against the real bug.

I apologize for the confusion. I still cannot test locally to determine what 
the actual corruption scenario is. Today I have re-tested my same patch in 
syzbot and it is failing.

Best regards,
Deepanshu

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

end of thread, other threads:[~2025-10-03  0:51 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-02 10:41 [PATCH] ext4: reject inline data flag when i_extra_isize is zero Deepanshu Kartikey
2025-10-02 19:57 ` Theodore Ts'o
  -- strict thread matches above, loose matches on Subject: below --
2025-10-03  0:51 Deepanshu Kartikey

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox