Re: [PATCH] ext4: clear EXT4_STATE_MAY_INLINE_DATA in swap_inode_boot_loader()

[Jan Kara]

On Sat 19-09-26 20:48:12, Hui Peng wrote:
> swap_inode_boot_loader() (EXT4_IOC_SWAP_BOOT) checks
> !ext4_has_inline_data(inode), but neither checks
> ext4_has_inline_data(inode_bl) nor clears EXT4_STATE_MAY_INLINE_DATA on
> inode and inode_bl after swapping their inode data.
> 
> When a newly created empty file (which has EXT4_STATE_MAY_INLINE_DATA
> set in memory) is swapped with a non-empty EXT4_BOOT_LOADER_INO that
> already has extent blocks, the target inode retains
> EXT4_STATE_MAY_INLINE_DATA == 1 alongside extent blocks. A subsequent
> ftruncate() + small pwrite() + fsync() dirties folio 0 while converting
> the inode to inline data, hitting
> BUG_ON(ext4_test_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA)) in
> ext4_writepages().
> 
> Reject inline data on inode_bl and clear EXT4_STATE_MAY_INLINE_DATA on
> both inodes in swap_inode_boot_loader().
> 
> Fixes: 67a11611e1a5 ("ext4: fix check of inode in swap_inode_boot_loader")
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@xxxxxxxxx>
> ---
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -397,7 +397,7 @@ static long swap_inode_boot_loader(struct super_block *sb,
>  	if (inode->i_nlink != 1 || !S_ISREG(inode->i_mode) ||
>  	    IS_SWAPFILE(inode) || IS_ENCRYPTED(inode) ||
>  	    (EXT4_I(inode)->i_flags & EXT4_JOURNAL_DATA_FL) ||
> -	    ext4_has_inline_data(inode)) {
> +	    ext4_has_inline_data(inode) || ext4_has_inline_data(inode_bl)) {
>  		err = -EINVAL;
>  		goto journal_err_out;
>  	}

I guess instead we should add a check to check_igot_inode() that inode
doesn't have inline data if EXT4_IGET_SPECIAL flag is set.

> @@ -461,6 +461,8 @@ static long swap_inode_boot_loader(struct super_block *sb,
>  	size_bl = (qsize_t)(inode_bl->i_blocks) * (1 << 9) + inode_bl->i_bytes;
>  	diff = size - size_bl;
>  	swap_inode_data(inode, inode_bl);
> +	ext4_clear_inode_state(inode, EXT4_STATE_MAY_INLINE_DATA);
> +	ext4_clear_inode_state(inode_bl, EXT4_STATE_MAY_INLINE_DATA);

Please don't touch inode_bl. That one should never have
EXT4_STATE_MAY_INLINE_DATA set (and nobody can manipulate it directly
anyway). For 'inode' I agree it will get blocks from inode_bl so better
just clear EXT4_STATE_MAY_INLINE_DATA.

								Honza
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR