Re: [PATCH v6 20/31] ext4: disable online defrag when inode using iomap buffered I/O path

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Missing IS_DAX() check for donor_inode allows DAX semantics violation during online defrag
--

commit 72261d911fe966cebd22871f337efe46600e2fd2
Author: Zhang Yi <yi.zhang@huawei.com>

ext4: disable online defrag when inode using iomap buffered I/O path

Online defragmentation does not currently support inodes using the
iomap buffered I/O path. The existing implementation relies on
buffer_head for sub-folio block management and data=ordered mode for
data consistency, both of which are incompatible with the iomap path.

> diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
> index 3329b7ad5dbd8..948ef3f44df5e 100644
> --- a/fs/ext4/move_extent.c
> +++ b/fs/ext4/move_extent.c
> @@ -476,6 +476,17 @@ static int mext_check_validity(struct inode *orig_inode,
>  		return -EOPNOTSUPP;
>  	}
>  
> +	/*
> +	 * TODO: support online defrag for inodes that use the buffered
> +	 * I/O iomap path.
> +	 */
> +	if (ext4_inode_buffered_iomap(orig_inode) ||
> +	    ext4_inode_buffered_iomap(donor_inode)) {
> +		ext4_msg(sb, KERN_ERR,
> +			 "Online defrag not supported for inode with iomap buffered IO path");
> +		return -EOPNOTSUPP;
> +	}
> +

[Severity: High]
This isn't a bug introduced by this patch, but since you are correctly
validating both orig_inode and donor_inode here in mext_check_validity(),
I noticed a potential issue with the earlier DAX validation.

Currently, mext_check_validity() in fs/ext4/move_extent.c only checks if
the origin file is DAX:

	if (IS_DAX(orig_inode)) {
		ext4_msg(sb, KERN_ERR,
			 "Online defrag not supported with DAX");
		return -EOPNOTSUPP;
	}

If a local user calls the EXT4_IOC_MOVE_EXT ioctl with a non-DAX origin
file and a DAX donor file, the operation won't be rejected.

Could this allow ext4_move_extents() to proceed and call
mext_folio_double_lock(), incorrectly allocating page cache folios and
buffer heads for the DAX donor inode instead of following DAX block
semantics?

>  	if (donor_inode->i_mode & (S_ISUID|S_ISGID)) {
>  		ext4_debug("ext4 move extent: suid or sgid is set to donor file [ino:orig %llu, donor %llu]\n",
>  			   orig_inode->i_ino, donor_inode->i_ino);
>

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260903123543.2302999-1-yi.zhang@huaweicloud.com?part=20