| Message ID | 9845459389d245fcaca2c017c27be8bc@h3c.com |
|---|---|
| State | New |
| Headers | show |
| Series | qcow2-bitmap: Fix uint64_t left-shift overflow | expand |
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 <tu.guoyi@h3c.com> > --- > 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? 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). > { > 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
> -----邮件原件----- > 发件人: Vladimir Sementsov-Ogievskiy [mailto:vsementsov@virtuozzo.com] > 发送时间: 2019年10月27日 0:50 > 收件人: tuguoyi (Cloud) <tu.guoyi@h3c.com>; kwolf@redhat.com; > mreitz@redhat.com; qemu-block@nongnu.org > 抄送: chengchiwen (Cloud) <chengchiwen@h3c.com>; > qemu-devel@nongnu.org; wangyongqing (Cloud) <w_yongqing@h3c.com>; > changlimin (Cloud) <changlimin@h3c.com>; gaoliang (Cloud) > <liang_gao@h3c.com>; wangyong (Cloud) <wang.yongD@h3c.com> > 主题: 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 <tu.guoyi@h3c.com> > > --- > > 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
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)) + ((len >> granularity_bits) > (uint64_t)BME_MAX_TABLE_SIZE * + s->cluster_size)) { error_setg(errp, "Too much space will be occupied by the bitmap. " "Use larger granularity");
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 <tu.guoyi@h3c.com> --- block/qcow2-bitmap.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) -- 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!