| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <klbs735aa2hewiugz474b35b4o7yxxsnkiqjwvka7x6eefphht@l3hdn3hjaisr>
Date: Wed, 8 Oct 2025 13:47:14 +0200
From: Jan Kara <jack@...e.cz>
To: Zhang Yi <yi.zhang@...weicloud.com>
Cc: linux-ext4@...r.kernel.org, linux-fsdevel@...r.kernel.org,
linux-kernel@...r.kernel.org, tytso@....edu, adilger.kernel@...ger.ca, jack@...e.cz,
yi.zhang@...wei.com, libaokun1@...wei.com, yukuai3@...wei.com, yangerkun@...wei.com
Subject: Re: [PATCH v2 07/13] ext4: add mext_check_validity() to do basic
check
On Thu 25-09-25 17:26:03, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@...wei.com>
>
> Currently, the basic validation checks during the move extent operation
> are scattered across __ext4_ioctl() and ext4_move_extents(), which makes
> the code somewhat disorganized. Introduce a new helper,
> mext_check_validity(), to handle these checks. This change involves only
> code relocation without any logical modifications.
>
> Signed-off-by: Zhang Yi <yi.zhang@...wei.com>
Looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@...e.cz>
Honza
> ---
> fs/ext4/ioctl.c | 10 -----
> fs/ext4/move_extent.c | 102 +++++++++++++++++++++++++++---------------
> 2 files changed, 65 insertions(+), 47 deletions(-)
>
> diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
> index 84e3c73952d7..a0d3a951ae85 100644
> --- a/fs/ext4/ioctl.c
> +++ b/fs/ext4/ioctl.c
> @@ -1349,16 +1349,6 @@ static long __ext4_ioctl(struct file *filp, unsigned int cmd, unsigned long arg)
> if (!(fd_file(donor)->f_mode & FMODE_WRITE))
> return -EBADF;
>
> - if (ext4_has_feature_bigalloc(sb)) {
> - ext4_msg(sb, KERN_ERR,
> - "Online defrag not supported with bigalloc");
> - return -EOPNOTSUPP;
> - } else if (IS_DAX(inode)) {
> - ext4_msg(sb, KERN_ERR,
> - "Online defrag not supported with DAX");
> - return -EOPNOTSUPP;
> - }
> -
> err = mnt_want_write_file(filp);
> if (err)
> return err;
> diff --git a/fs/ext4/move_extent.c b/fs/ext4/move_extent.c
> index 6175906c7119..cdd175d5c1f3 100644
> --- a/fs/ext4/move_extent.c
> +++ b/fs/ext4/move_extent.c
> @@ -442,6 +442,68 @@ move_extent_per_page(struct file *o_filp, struct inode *donor_inode,
> goto unlock_folios;
> }
>
> +/*
> + * Check the validity of the basic filesystem environment and the
> + * inodes' support status.
> + */
> +static int mext_check_validity(struct inode *orig_inode,
> + struct inode *donor_inode)
> +{
> + struct super_block *sb = orig_inode->i_sb;
> +
> + /* origin and donor should be different inodes */
> + if (orig_inode == donor_inode) {
> + ext4_debug("ext4 move extent: The argument files should not be same inode [ino:orig %lu, donor %lu]\n",
> + orig_inode->i_ino, donor_inode->i_ino);
> + return -EINVAL;
> + }
> +
> + /* origin and donor should belone to the same filesystem */
> + if (orig_inode->i_sb != donor_inode->i_sb) {
> + ext4_debug("ext4 move extent: The argument files should be in same FS [ino:orig %lu, donor %lu]\n",
> + orig_inode->i_ino, donor_inode->i_ino);
> + return -EINVAL;
> + }
> +
> + /* Regular file check */
> + if (!S_ISREG(orig_inode->i_mode) || !S_ISREG(donor_inode->i_mode)) {
> + ext4_debug("ext4 move extent: The argument files should be regular file [ino:orig %lu, donor %lu]\n",
> + orig_inode->i_ino, donor_inode->i_ino);
> + return -EINVAL;
> + }
> +
> + if (ext4_has_feature_bigalloc(sb)) {
> + ext4_msg(sb, KERN_ERR,
> + "Online defrag not supported with bigalloc");
> + return -EOPNOTSUPP;
> + }
> +
> + if (IS_DAX(orig_inode)) {
> + ext4_msg(sb, KERN_ERR,
> + "Online defrag not supported with DAX");
> + return -EOPNOTSUPP;
> + }
> +
> + /*
> + * TODO: it's not obvious how to swap blocks for inodes with full
> + * journaling enabled.
> + */
> + if (ext4_should_journal_data(orig_inode) ||
> + ext4_should_journal_data(donor_inode)) {
> + ext4_msg(sb, KERN_ERR,
> + "Online defrag not supported with data journaling");
> + return -EOPNOTSUPP;
> + }
> +
> + if (IS_ENCRYPTED(orig_inode) || IS_ENCRYPTED(donor_inode)) {
> + ext4_msg(sb, KERN_ERR,
> + "Online defrag not supported for encrypted files");
> + return -EOPNOTSUPP;
> + }
> +
> + return 0;
> +}
> +
> /**
> * mext_check_arguments - Check whether move extent can be done
> *
> @@ -567,43 +629,9 @@ ext4_move_extents(struct file *o_filp, struct file *d_filp, __u64 orig_blk,
> ext4_lblk_t d_start = donor_blk;
> int ret;
>
> - if (orig_inode->i_sb != donor_inode->i_sb) {
> - ext4_debug("ext4 move extent: The argument files "
> - "should be in same FS [ino:orig %lu, donor %lu]\n",
> - orig_inode->i_ino, donor_inode->i_ino);
> - return -EINVAL;
> - }
> -
> - /* orig and donor should be different inodes */
> - if (orig_inode == donor_inode) {
> - ext4_debug("ext4 move extent: The argument files should not "
> - "be same inode [ino:orig %lu, donor %lu]\n",
> - orig_inode->i_ino, donor_inode->i_ino);
> - return -EINVAL;
> - }
> -
> - /* Regular file check */
> - if (!S_ISREG(orig_inode->i_mode) || !S_ISREG(donor_inode->i_mode)) {
> - ext4_debug("ext4 move extent: The argument files should be "
> - "regular file [ino:orig %lu, donor %lu]\n",
> - orig_inode->i_ino, donor_inode->i_ino);
> - return -EINVAL;
> - }
> -
> - /* TODO: it's not obvious how to swap blocks for inodes with full
> - journaling enabled */
> - if (ext4_should_journal_data(orig_inode) ||
> - ext4_should_journal_data(donor_inode)) {
> - ext4_msg(orig_inode->i_sb, KERN_ERR,
> - "Online defrag not supported with data journaling");
> - return -EOPNOTSUPP;
> - }
> -
> - if (IS_ENCRYPTED(orig_inode) || IS_ENCRYPTED(donor_inode)) {
> - ext4_msg(orig_inode->i_sb, KERN_ERR,
> - "Online defrag not supported for encrypted files");
> - return -EOPNOTSUPP;
> - }
> + ret = mext_check_validity(orig_inode, donor_inode);
> + if (ret)
> + return ret;
>
> /* Protect orig and donor inodes against a truncate */
> lock_two_nondirectories(orig_inode, donor_inode);
> --
> 2.46.1
>
--
Jan Kara <jack@...e.com>
SUSE Labs, CR
Powered by blists - more mailing lists