答复: [PATCH] qcow2-bitmap: Fix uint64_t left-shift overflow

[Tuguoyi]

> -----邮件原件-----
> 发件人: Vladimir Sementsov-Ogievskiy [mailto:address@hidden]
> 发送时间: 2019年10月27日 0:50
> 收件人: tuguoyi (Cloud) <address@hidden>; address@hidden;
> address@hidden; address@hidden
> 抄送: chengchiwen (Cloud) <address@hidden>;
> address@hidden; wangyongqing (Cloud) <address@hidden>;
> changlimin (Cloud) <address@hidden>; gaoliang (Cloud)
> <address@hidden>; wangyong (Cloud) <address@hidden>
> 主题: Re: [PATCH] qcow2-bitmap: Fix uint64_t left-shift overflow
> 
> 26.10.2019 12:19, Tuguoyi wrote:
> > In check_constraints_on_bitmap(), the sanity check on the granularity
> > will cause uint64_t integer left-shift overflow when cluster_size is
> > 2M and the granularity is bigger than 32K which is even smaller than
> > the default value for a qcow2 disk with cluster_size set to 64k or
> > bigger. This patch fix the issue by right-shift @len instead.
> >
> > Signed-off-by: Guoyi Tu <address@hidden>
> > ---
> >   block/qcow2-bitmap.c | 4 ++--
> >   1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/block/qcow2-bitmap.c b/block/qcow2-bitmap.c index
> > 98294a7..2a1d789 100644
> > --- a/block/qcow2-bitmap.c
> > +++ b/block/qcow2-bitmap.c
> > @@ -172,8 +172,8 @@ static int
> check_constraints_on_bitmap(BlockDriverState *bs,
> >       }
> >
> >       if ((len > (uint64_t)BME_MAX_PHYS_SIZE << granularity_bits) ||
> > -        (len > (uint64_t)BME_MAX_TABLE_SIZE * s->cluster_size <<
> > -               granularity_bits))
> 
> Hmm.
> BME_MAX_TABLE_SIZE = 0x8000000
> 
> 0x8000000 * 1024 * 1024 * 2 << 16 = 2 ** 64, so for 64k granularity it
> owerflows..
> But for 32k doesn't. Or am I wrong?

You are right, it doesn't overflow for 32K.

> 
> Anyway, thanks for fixing!
> 
> > +        ((len >> granularity_bits) > (uint64_t)BME_MAX_TABLE_SIZE *
> > +                s->cluster_size))
> 
> It's a bit incorrect, as len may be unaligned, we need ((len + granularity - 
> 1) >>
> granularity_bits) on the left, or better DIV_ROUNTD_UP(len, granularity).

Yes, @len should be ROUND-UP, thanks for pointing it out, and I'll fix it and 
send another patch

> 
> >       {
> >           error_setg(errp, "Too much space will be occupied by the
> bitmap. "
> >                      "Use larger granularity");
> > --
> > 2.7.4
> > ----------------------------------------------------------------------
> > ---------------------------------------------------------------
> > 本邮件及其附件含有新华三集团的保密信息，仅限于发送给上面地址中
> 列出
> > 的个人或群组。禁止任何其他人以任何形式使用（包括但不限于全部或
> 部分地泄露、复制、
> > 或散发）本邮件中的信息。如果您错收了本邮件，请您立即电话或邮件
> 通知发件人并删除本
> > 邮件！
> > This e-mail and its attachments contain confidential information from
> > New H3C, which is intended only for the person or entity whose address
> > is listed above. Any use of the information contained herein in any
> > way (including, but not limited to, total or partial disclosure,
> > reproduction, or dissemination) by persons other than the intended
> > recipient(s) is prohibited. If you receive this e-mail in error,
> > please notify the sender by phone or email immediately and delete it!
> >
> 
> Not sure that this is possible, as it's automatically available here:
> https://lists.gnu.org/archive/html/qemu-devel/2019-10/msg07336.html
> 
> 
> --
> Best regards,
> Vladimir

Thanks a lot for reviewing it