Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 28 Feb 2022 10:03:59 +0100
From:   Jan Kara <jack@suse.cz>
To:     Zhang Yi <yi.zhang@huawei.com>
Cc:     Jan Kara <jack@suse.cz>, linux-ext4@vger.kernel.org, tytso@mit.edu,
        adilger.kernel@dilger.ca, yukuai3@huawei.com
Subject: Re: [PATCH v2] ext4: fix underflow in ext4_max_bitmap_size()
Message-ID: <20220228090359.dcqajobuol3ripur@quack3.lan>
References: <20220225102837.3048196-1-yi.zhang@huawei.com>
 <20220225123851.flahv2nlvpqq3d33@quack3.lan>
 <3335eb5d-76c0-0b01-3dca-b2e2ccdf91c0@huawei.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <3335eb5d-76c0-0b01-3dca-b2e2ccdf91c0@huawei.com>
Precedence: bulk

On Sat 26-02-22 10:30:31, Zhang Yi wrote:
> On 2022/2/25 20:38, Jan Kara wrote:
> > On Fri 25-02-22 18:28:37, Zhang Yi wrote:
> >> +	/* Compute how many metadata blocks are needed */
> >> +	meta_blocks = 1;
> >> +	meta_blocks += 1 + ppb;
> >> +	meta_blocks += 1 + ppb + ppb * ppb;
> >> +	/* Does block tree limit file size? */
> >> +	if (res + meta_blocks <= upper_limit)
> >> +		goto check_lfs;
> >> +
> >> +	res = upper_limit;
> >> +	/* How many metadata blocks are needed for addressing upper_limit? */
> >> +	upper_limit -= EXT4_NDIR_BLOCKS;
> >> +	/* indirect blocks */
> >> +	meta_blocks = 1;
> >> +	upper_limit -= ppb;
> >> +	/* double indirect blocks */
> >> +	if (upper_limit < ppb * ppb) {
> >> +		meta_blocks += 1 + DIV_ROUND_UP_ULL(upper_limit, ppb);
> >> +		res -= meta_blocks;
> >> +		goto check_lfs;
> >> +	}
> >> +	meta_blocks += 1 + ppb;
> >> +	upper_limit -= ppb * ppb;
> >> +	/* tripple indirect blocks for the rest */
> >> +	meta_blocks += 1 + DIV_ROUND_UP_ULL(upper_limit, ppb) +
> >> +		DIV_ROUND_UP_ULL(upper_limit, ppb*ppb);
> >> +	res -= meta_blocks;
> >> +check_lfs:
> >>  	res <<= bits;
> > 
> > Cannot this overflow loff_t again? I mean if upper_limit == (1 << 48) - 1
> > and we have 64k blocksize, 'res' will be larger than (1 << 47) and thus 
> > res << 16 will be greater than 1 << 63 => negative... Am I missing
> > something?
> > 
> 
> If upper_limit==(1 << 48) - 1, we could address the whole data blocks, the 'res'
> is equal to EXT4_NDIR_BLOCKS + ppb + ppb*ppb + ((long long)ppb)*ppb*ppb, it's
> smaller than (1 << 43) - 1, so res << 16 is still smaller 1 << 59, so it cannot
> overflow loff_t again.

Indeed, sorry for confusion. Not sure where I did mistake in my math
previously.

								Honza
-- 
Jan Kara <jack@suse.com>
SUSE Labs, CR