Re: [PATCH] ext4: reject inline data flag when i_extra_isize is zero

["Theodore Ts'o" <tytso@mit.edu>]

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
>