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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20220831115517.qolsk27xh5djei7h@quack3>
Date:   Wed, 31 Aug 2022 13:55:17 +0200
From:   Jan Kara <jack@...e.cz>
To:     Jason Yan <yanaijie@...wei.com>
Cc:     tytso@....edu, adilger.kernel@...ger.ca, jack@...e.cz,
        ritesh.list@...il.com, lczerner@...hat.com,
        linux-ext4@...r.kernel.org
Subject: Re: [PATCH 09/13] ext4: factor out ext4_compat_feature_check()

On Tue 30-08-22 20:04:07, Jason Yan wrote:
> Factor out ext4_compat_feature_check(). No functional change.
> 
> Signed-off-by: Jason Yan <yanaijie@...wei.com>
> ---
>  fs/ext4/super.c | 144 ++++++++++++++++++++++++++----------------------
>  1 file changed, 77 insertions(+), 67 deletions(-)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 96cf23787bba..1e7d6eb6a3aa 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -4607,6 +4607,82 @@ static int ext4_handle_csum(struct super_block *sb, struct ext4_super_block *es)
>  	return 0;
>  }
>  
> +static int ext4_compat_feature_check(struct super_block *sb,
> +				     struct ext4_super_block *es,
> +				     int silent)

And here maybe ext4_check_feature_compatibility() might be a better name
because "compat_feature" is a name of a specific subset of ext4 features so
using it in function name is a bit confusing. Otherwise feel free to add:

Reviewed-by: Jan Kara <jack@...e.cz>

								Honza

> +{
> +	if (le32_to_cpu(es->s_rev_level) == EXT4_GOOD_OLD_REV &&
> +	    (ext4_has_compat_features(sb) ||
> +	     ext4_has_ro_compat_features(sb) ||
> +	     ext4_has_incompat_features(sb)))
> +		ext4_msg(sb, KERN_WARNING,
> +		       "feature flags set on rev 0 fs, "
> +		       "running e2fsck is recommended");
> +
> +	if (es->s_creator_os == cpu_to_le32(EXT4_OS_HURD)) {
> +		set_opt2(sb, HURD_COMPAT);
> +		if (ext4_has_feature_64bit(sb)) {
> +			ext4_msg(sb, KERN_ERR,
> +				 "The Hurd can't support 64-bit file systems");
> +			return -EINVAL;
> +		}
> +
> +		/*
> +		 * ea_inode feature uses l_i_version field which is not
> +		 * available in HURD_COMPAT mode.
> +		 */
> +		if (ext4_has_feature_ea_inode(sb)) {
> +			ext4_msg(sb, KERN_ERR,
> +				 "ea_inode feature is not supported for Hurd");
> +			return -EINVAL;
> +		}
> +	}
> +
> +	if (IS_EXT2_SB(sb)) {
> +		if (ext2_feature_set_ok(sb))
> +			ext4_msg(sb, KERN_INFO, "mounting ext2 file system "
> +				 "using the ext4 subsystem");
> +		else {
> +			/*
> +			 * If we're probing be silent, if this looks like
> +			 * it's actually an ext[34] filesystem.
> +			 */
> +			if (silent && ext4_feature_set_ok(sb, sb_rdonly(sb)))
> +				return -EINVAL;
> +			ext4_msg(sb, KERN_ERR, "couldn't mount as ext2 due "
> +				 "to feature incompatibilities");
> +			return -EINVAL;
> +		}
> +	}
> +
> +	if (IS_EXT3_SB(sb)) {
> +		if (ext3_feature_set_ok(sb))
> +			ext4_msg(sb, KERN_INFO, "mounting ext3 file system "
> +				 "using the ext4 subsystem");
> +		else {
> +			/*
> +			 * If we're probing be silent, if this looks like
> +			 * it's actually an ext4 filesystem.
> +			 */
> +			if (silent && ext4_feature_set_ok(sb, sb_rdonly(sb)))
> +				return -EINVAL;
> +			ext4_msg(sb, KERN_ERR, "couldn't mount as ext3 due "
> +				 "to feature incompatibilities");
> +			return -EINVAL;
> +		}
> +	}
> +
> +	/*
> +	 * Check feature flags regardless of the revision level, since we
> +	 * previously didn't change the revision level when setting the flags,
> +	 * so there is a chance incompat flags are set on a rev 0 filesystem.
> +	 */
> +	if (!ext4_feature_set_ok(sb, (sb_rdonly(sb))))
> +		return -EINVAL;
> +
> +	return 0;
> +}
> +
>  static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>  {
>  	struct buffer_head *bh, **group_desc;
> @@ -4761,73 +4837,7 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>  	sb->s_flags = (sb->s_flags & ~SB_POSIXACL) |
>  		(test_opt(sb, POSIX_ACL) ? SB_POSIXACL : 0);
>  
> -	if (le32_to_cpu(es->s_rev_level) == EXT4_GOOD_OLD_REV &&
> -	    (ext4_has_compat_features(sb) ||
> -	     ext4_has_ro_compat_features(sb) ||
> -	     ext4_has_incompat_features(sb)))
> -		ext4_msg(sb, KERN_WARNING,
> -		       "feature flags set on rev 0 fs, "
> -		       "running e2fsck is recommended");
> -
> -	if (es->s_creator_os == cpu_to_le32(EXT4_OS_HURD)) {
> -		set_opt2(sb, HURD_COMPAT);
> -		if (ext4_has_feature_64bit(sb)) {
> -			ext4_msg(sb, KERN_ERR,
> -				 "The Hurd can't support 64-bit file systems");
> -			goto failed_mount;
> -		}
> -
> -		/*
> -		 * ea_inode feature uses l_i_version field which is not
> -		 * available in HURD_COMPAT mode.
> -		 */
> -		if (ext4_has_feature_ea_inode(sb)) {
> -			ext4_msg(sb, KERN_ERR,
> -				 "ea_inode feature is not supported for Hurd");
> -			goto failed_mount;
> -		}
> -	}
> -
> -	if (IS_EXT2_SB(sb)) {
> -		if (ext2_feature_set_ok(sb))
> -			ext4_msg(sb, KERN_INFO, "mounting ext2 file system "
> -				 "using the ext4 subsystem");
> -		else {
> -			/*
> -			 * If we're probing be silent, if this looks like
> -			 * it's actually an ext[34] filesystem.
> -			 */
> -			if (silent && ext4_feature_set_ok(sb, sb_rdonly(sb)))
> -				goto failed_mount;
> -			ext4_msg(sb, KERN_ERR, "couldn't mount as ext2 due "
> -				 "to feature incompatibilities");
> -			goto failed_mount;
> -		}
> -	}
> -
> -	if (IS_EXT3_SB(sb)) {
> -		if (ext3_feature_set_ok(sb))
> -			ext4_msg(sb, KERN_INFO, "mounting ext3 file system "
> -				 "using the ext4 subsystem");
> -		else {
> -			/*
> -			 * If we're probing be silent, if this looks like
> -			 * it's actually an ext4 filesystem.
> -			 */
> -			if (silent && ext4_feature_set_ok(sb, sb_rdonly(sb)))
> -				goto failed_mount;
> -			ext4_msg(sb, KERN_ERR, "couldn't mount as ext3 due "
> -				 "to feature incompatibilities");
> -			goto failed_mount;
> -		}
> -	}
> -
> -	/*
> -	 * Check feature flags regardless of the revision level, since we
> -	 * previously didn't change the revision level when setting the flags,
> -	 * so there is a chance incompat flags are set on a rev 0 filesystem.
> -	 */
> -	if (!ext4_feature_set_ok(sb, (sb_rdonly(sb))))
> +	if (ext4_compat_feature_check(sb, es, silent))
>  		goto failed_mount;
>  
>  	if (le16_to_cpu(sbi->s_es->s_reserved_gdt_blocks) > (blocksize / 4)) {
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR

Powered by blists - more mailing lists