Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7;
Date:   Thu, 19 Oct 2023 13:21:42 +0200
From:   Jan Kara <jack@suse.cz>
To:     Baokun Li <libaokun1@huawei.com>
Cc:     linux-ext4@vger.kernel.org, tytso@mit.edu,
        adilger.kernel@dilger.ca, jack@suse.cz, ritesh.list@gmail.com,
        linux-kernel@vger.kernel.org, yi.zhang@huawei.com,
        yangerkun@huawei.com, yukuai3@huawei.com
Subject: Re: [PATCH 1/4] ext4: unify the type of flexbg_size to unsigned int
Message-ID: <20231019112142.4s6jebebztqaunnt@quack3>
References: <20231018114221.441526-1-libaokun1@huawei.com>
 <20231018114221.441526-2-libaokun1@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20231018114221.441526-2-libaokun1@huawei.com>
Precedence: bulk

On Wed 18-10-23 19:42:18, Baokun Li wrote:
> The maximum value of flexbg_size is 2^31, but the maximum value of int
> is (2^31 - 1), so overflow may occur when the type of flexbg_size is
> declared as int.
> 
> For example, when uninit_mask is initialized in ext4_alloc_group_tables(),
> if flexbg_size == 2^31, the initialized uninit_mask is incorrect, and this
> may causes set_flexbg_block_bitmap() to trigger a BUG_ON().
> 
> Therefore, the flexbg_size type is declared as unsigned int to avoid
> overflow and memory waste.
> 
> Signed-off-by: Baokun Li <libaokun1@huawei.com>

Looks good. Feel free to add:

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

								Honza

> ---
>  fs/ext4/resize.c | 13 +++++++------
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/ext4/resize.c b/fs/ext4/resize.c
> index 4fe061edefdd..c6d4539d4c1f 100644
> --- a/fs/ext4/resize.c
> +++ b/fs/ext4/resize.c
> @@ -228,7 +228,7 @@ struct ext4_new_flex_group_data {
>   *
>   * Returns NULL on failure otherwise address of the allocated structure.
>   */
> -static struct ext4_new_flex_group_data *alloc_flex_gd(unsigned long flexbg_size)
> +static struct ext4_new_flex_group_data *alloc_flex_gd(unsigned int flexbg_size)
>  {
>  	struct ext4_new_flex_group_data *flex_gd;
>  
> @@ -283,7 +283,7 @@ static void free_flex_gd(struct ext4_new_flex_group_data *flex_gd)
>   */
>  static int ext4_alloc_group_tables(struct super_block *sb,
>  				struct ext4_new_flex_group_data *flex_gd,
> -				int flexbg_size)
> +				unsigned int flexbg_size)
>  {
>  	struct ext4_new_group_data *group_data = flex_gd->groups;
>  	ext4_fsblk_t start_blk;
> @@ -384,12 +384,12 @@ static int ext4_alloc_group_tables(struct super_block *sb,
>  		group = group_data[0].group;
>  
>  		printk(KERN_DEBUG "EXT4-fs: adding a flex group with "
> -		       "%d groups, flexbg size is %d:\n", flex_gd->count,
> +		       "%u groups, flexbg size is %u:\n", flex_gd->count,
>  		       flexbg_size);
>  
>  		for (i = 0; i < flex_gd->count; i++) {
>  			ext4_debug(
> -			       "adding %s group %u: %u blocks (%d free, %d mdata blocks)\n",
> +			       "adding %s group %u: %u blocks (%u free, %u mdata blocks)\n",
>  			       ext4_bg_has_super(sb, group + i) ? "normal" :
>  			       "no-super", group + i,
>  			       group_data[i].blocks_count,
> @@ -1606,7 +1606,7 @@ static int ext4_flex_group_add(struct super_block *sb,
>  static int ext4_setup_next_flex_gd(struct super_block *sb,
>  				    struct ext4_new_flex_group_data *flex_gd,
>  				    ext4_fsblk_t n_blocks_count,
> -				    unsigned long flexbg_size)
> +				    unsigned int flexbg_size)
>  {
>  	struct ext4_sb_info *sbi = EXT4_SB(sb);
>  	struct ext4_super_block *es = sbi->s_es;
> @@ -1990,8 +1990,9 @@ int ext4_resize_fs(struct super_block *sb, ext4_fsblk_t n_blocks_count)
>  	ext4_fsblk_t o_blocks_count;
>  	ext4_fsblk_t n_blocks_count_retry = 0;
>  	unsigned long last_update_time = 0;
> -	int err = 0, flexbg_size = 1 << sbi->s_log_groups_per_flex;
> +	int err = 0;
>  	int meta_bg;
> +	unsigned int flexbg_size = ext4_flex_bg_size(sbi);
>  
>  	/* See if the device is actually as big as what was requested */
>  	bh = ext4_sb_bread(sb, n_blocks_count - 1, 0);
> -- 
> 2.31.1
> 
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR