Subject: Re: [PATCH 7/7] ext4: improve inode table blocks counting in
 ext4_num_overhead_clusters
To:     Jan Kara <jack@suse.cz>
Cc:     tytso@mit.edu, adilger.kernel@dilger.ca,
        linux-ext4@vger.kernel.org, linux-kernel@vger.kernel.org
References: <20230221115919.1918161-1-shikemeng@huaweicloud.com>
 <20230221115919.1918161-8-shikemeng@huaweicloud.com>
 <20230320124444.kkp4es2wyke7vqgx@quack3>
From:   Kemeng Shi <shikemeng@huaweicloud.com>
Message-ID: <afde9290-39d4-0273-40de-153a53f60e31@huaweicloud.com>
Date:   Mon, 20 Mar 2023 21:19:37 +0800
User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:60.0) Gecko/20100101
 Thunderbird/60.5.0
MIME-Version: 1.0
In-Reply-To: <20230320124444.kkp4es2wyke7vqgx@quack3>
Content-Type: text/plain; charset=gbk
Content-Transfer-Encoding: 7bit
Precedence: bulk


on 3/20/2023 8:44 PM, Jan Kara wrote:
> On Tue 21-02-23 19:59:19, Kemeng Shi wrote:
>> As inode table blocks are contiguous, inode table blocks inside the
>> block_group can be represented as range [itbl_cluster_start,
>> itbl_cluster_last]. Then we can simply account inode table cluters and
>> check cluster overlap with [itbl_cluster_start, itbl_cluster_last] instead
>> of traverse each block of inode table.
>> By the way, this patch fixes code style problem of comment for
>> ext4_num_overhead_clusters.
>>
>> Signed-off-by: Kemeng Shi <shikemeng@huaweicloud.com>
> 
> FWIW this is triggering Coverity warning:
> 
> *** CID 1536792:  Uninitialized variables  (UNINIT)
> /fs/ext4/balloc.c: 153 in ext4_num_overhead_clusters()
> 147                     inode_cluster = EXT4_B2C(sbi,
> 148                                              ext4_inode_bitmap(sb, gdp) - st
> 149                     /*
> 150                      * Additional check if inode bitmap is in just accounted
> 151                      * block_cluster
> 152                      */
>>>>     CID 1536792:  Uninitialized variables  (UNINIT)
>>>>     Using uninitialized value "block_cluster".
> 153                     if (inode_cluster != block_cluster &&
> 154                         inode_cluster >= base_clusters &&
> 155                         (inode_cluster < itbl_cluster_start ||
> 156                         inode_cluster > itbl_cluster_end))
> 157                             num_clusters++;
> 158             }
> 
> which actually looks valid AFAICT.
Yes, there is a risk to access uninitialized block_cluster if block bitmap block
and inode bitmap block are in different groups. Patch to fix is just sent. Thanks!

-- 
Best wishes
Kemeng Shi