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: <20220908090025.7aw2nsfnp5mzgewn@riteshh-domain>
Date:   Thu, 8 Sep 2022 14:30:25 +0530
From:   "Ritesh Harjani (IBM)" <ritesh.list@...il.com>
To:     Jason Yan <yanaijie@...wei.com>
Cc:     tytso@....edu, adilger.kernel@...ger.ca, jack@...e.cz,
        lczerner@...hat.com, linux-ext4@...r.kernel.org
Subject: Re: [PATCH v2 10/13] ext4: factor out ext4_geometry_check()

On 22/09/03 11:01AM, Jason Yan wrote:
> Factor out ext4_geometry_check(). No functional change.
> 
> Signed-off-by: Jason Yan <yanaijie@...wei.com>
> Reviewed-by: Jan Kara <jack@...e.cz>
> ---
>  fs/ext4/super.c | 111 ++++++++++++++++++++++++++----------------------
>  1 file changed, 61 insertions(+), 50 deletions(-)
> 
> diff --git a/fs/ext4/super.c b/fs/ext4/super.c
> index 5f0e7c5188a3..69921a850644 100644
> --- a/fs/ext4/super.c
> +++ b/fs/ext4/super.c
> @@ -4683,6 +4683,66 @@ static int ext4_check_feature_compatibility(struct super_block *sb,
>  	return 0;
>  }
>  
> +static int ext4_geometry_check(struct super_block *sb,
> +			       struct ext4_super_block *es)
> +{
> +	struct ext4_sb_info *sbi = EXT4_SB(sb);
> +	__u64 blocks_count;
> +
> +	/* check blocks count against device size */
> +	blocks_count = sb_bdev_nr_blocks(sb);
> +	if (blocks_count && ext4_blocks_count(es) > blocks_count) {
> +		ext4_msg(sb, KERN_WARNING, "bad geometry: block count %llu "
> +		       "exceeds size of device (%llu blocks)",
> +		       ext4_blocks_count(es), blocks_count);
> +		return -EINVAL;
> +	}
> +
> +	/*
> +	 * It makes no sense for the first data block to be beyond the end
> +	 * of the filesystem.
> +	 */
> +	if (le32_to_cpu(es->s_first_data_block) >= ext4_blocks_count(es)) {
> +		ext4_msg(sb, KERN_WARNING, "bad geometry: first data "
> +			 "block %u is beyond end of filesystem (%llu)",
> +			 le32_to_cpu(es->s_first_data_block),
> +			 ext4_blocks_count(es));
> +		return -EINVAL;
> +	}
> +	if ((es->s_first_data_block == 0) && (es->s_log_block_size == 0) &&
> +	    (sbi->s_cluster_ratio == 1)) {
> +		ext4_msg(sb, KERN_WARNING, "bad geometry: first data "
> +			 "block is 0 with a 1k block and cluster size");
> +		return -EINVAL;
> +	}
> +
> +	blocks_count = (ext4_blocks_count(es) -
> +			le32_to_cpu(es->s_first_data_block) +
> +			EXT4_BLOCKS_PER_GROUP(sb) - 1);
> +	do_div(blocks_count, EXT4_BLOCKS_PER_GROUP(sb));
> +	if (blocks_count > ((uint64_t)1<<32) - EXT4_DESC_PER_BLOCK(sb)) {
> +		ext4_msg(sb, KERN_WARNING, "groups count too large: %llu "
> +		       "(block count %llu, first data block %u, "
> +		       "blocks per group %lu)", blocks_count,
> +		       ext4_blocks_count(es),
> +		       le32_to_cpu(es->s_first_data_block),
> +		       EXT4_BLOCKS_PER_GROUP(sb));
> +		return -EINVAL;
> +	}
> +	sbi->s_groups_count = blocks_count;
> +	sbi->s_blockfile_groups = min_t(ext4_group_t, sbi->s_groups_count,
> +			(EXT4_MAX_BLOCK_FILE_PHYS / EXT4_BLOCKS_PER_GROUP(sb)));
> +	if (((u64)sbi->s_groups_count * sbi->s_inodes_per_group) !=
> +	    le32_to_cpu(es->s_inodes_count)) {
> +		ext4_msg(sb, KERN_ERR, "inodes count not valid: %u vs %llu",
> +			 le32_to_cpu(es->s_inodes_count),
> +			 ((u64)sbi->s_groups_count * sbi->s_inodes_per_group));
> +		return -EINVAL;
> +	}
> +
> +	return 0;
> +}
> +
>  static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>  {
>  	struct buffer_head *bh, **group_desc;
> @@ -4698,7 +4758,6 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>  	unsigned int db_count;
>  	unsigned int i;
>  	int needs_recovery, has_huge_files;
> -	__u64 blocks_count;
>  	int err = 0;
>  	ext4_group_t first_not_zeroed;
>  	struct ext4_fs_context *ctx = fc->fs_private;
> @@ -4984,57 +5043,9 @@ static int __ext4_fill_super(struct fs_context *fc, struct super_block *sb)
>  		goto failed_mount;
>  	}
>  
> -	/* check blocks count against device size */
> -	blocks_count = sb_bdev_nr_blocks(sb);
> -	if (blocks_count && ext4_blocks_count(es) > blocks_count) {
> -		ext4_msg(sb, KERN_WARNING, "bad geometry: block count %llu "
> -		       "exceeds size of device (%llu blocks)",
> -		       ext4_blocks_count(es), blocks_count);
> +	if (ext4_geometry_check(sb, es))
>  		goto failed_mount;
> -	}
>  
> -	/*
> -	 * It makes no sense for the first data block to be beyond the end
> -	 * of the filesystem.
> -	 */
> -	if (le32_to_cpu(es->s_first_data_block) >= ext4_blocks_count(es)) {
> -		ext4_msg(sb, KERN_WARNING, "bad geometry: first data "
> -			 "block %u is beyond end of filesystem (%llu)",
> -			 le32_to_cpu(es->s_first_data_block),
> -			 ext4_blocks_count(es));
> -		goto failed_mount;
> -	}
> -	if ((es->s_first_data_block == 0) && (es->s_log_block_size == 0) &&
> -	    (sbi->s_cluster_ratio == 1)) {
> -		ext4_msg(sb, KERN_WARNING, "bad geometry: first data "
> -			 "block is 0 with a 1k block and cluster size");
> -		goto failed_mount;
> -	}
> -
> -	blocks_count = (ext4_blocks_count(es) -
> -			le32_to_cpu(es->s_first_data_block) +
> -			EXT4_BLOCKS_PER_GROUP(sb) - 1);
> -	do_div(blocks_count, EXT4_BLOCKS_PER_GROUP(sb));
> -	if (blocks_count > ((uint64_t)1<<32) - EXT4_DESC_PER_BLOCK(sb)) {
> -		ext4_msg(sb, KERN_WARNING, "groups count too large: %llu "
> -		       "(block count %llu, first data block %u, "
> -		       "blocks per group %lu)", blocks_count,
> -		       ext4_blocks_count(es),
> -		       le32_to_cpu(es->s_first_data_block),
> -		       EXT4_BLOCKS_PER_GROUP(sb));
> -		goto failed_mount;
> -	}
> -	sbi->s_groups_count = blocks_count;
> -	sbi->s_blockfile_groups = min_t(ext4_group_t, sbi->s_groups_count,
> -			(EXT4_MAX_BLOCK_FILE_PHYS / EXT4_BLOCKS_PER_GROUP(sb)));
> -	if (((u64)sbi->s_groups_count * sbi->s_inodes_per_group) !=
> -	    le32_to_cpu(es->s_inodes_count)) {
> -		ext4_msg(sb, KERN_ERR, "inodes count not valid: %u vs %llu",
> -			 le32_to_cpu(es->s_inodes_count),
> -			 ((u64)sbi->s_groups_count * sbi->s_inodes_per_group));
> -		ret = -EINVAL;

This had initially confused me. But then I saw we by default always have
ret = -EINVAL.

This patch looks good to me. 
Reviewed-by: Ritesh Harjani (IBM) <ritesh.list@...il.com>


> -		goto failed_mount;
> -	}
>  	db_count = (sbi->s_groups_count + EXT4_DESC_PER_BLOCK(sb) - 1) /
>  		   EXT4_DESC_PER_BLOCK(sb);
>  	if (ext4_has_feature_meta_bg(sb)) {
> -- 
> 2.31.1
> 

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ