From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:39100) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WXsS0-0004qp-GN for qemu-devel@nongnu.org; Wed, 09 Apr 2014 09:22:06 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WXsRt-0003CD-UD for qemu-devel@nongnu.org; Wed, 09 Apr 2014 09:22:00 -0400 Received: from mx1.redhat.com ([209.132.183.28]:44205) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WXsRt-0003Bu-MH for qemu-devel@nongnu.org; Wed, 09 Apr 2014 09:21:53 -0400 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id s39DLrBr005262 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK) for ; Wed, 9 Apr 2014 09:21:53 -0400 Message-ID: <5345496E.5000904@redhat.com> Date: Wed, 09 Apr 2014 15:21:50 +0200 From: Laszlo Ersek MIME-Version: 1.0 References: <1397042053-1323-1-git-send-email-kwolf@redhat.com> In-Reply-To: <1397042053-1323-1-git-send-email-kwolf@redhat.com> Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH] bochs: Fix catalog size check List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf , qemu-devel@nongnu.org Cc: stefanha@redhat.com On 04/09/14 13:14, Kevin Wolf wrote: > The old check was off by a factor of 512 and didn't consider cases where > we don't get an exact division. This could lead to an out-of-bounds > array access in seek_to_sector(). > > Signed-off-by: Kevin Wolf > --- > block/bochs.c | 14 +++++++++++--- > tests/qemu-iotests/078 | 6 +++++- > tests/qemu-iotests/078.out | 6 ++++-- > 3 files changed, 20 insertions(+), 6 deletions(-) > > diff --git a/block/bochs.c b/block/bochs.c > index 50b84a9..eacf956 100644 > --- a/block/bochs.c > +++ b/block/bochs.c > @@ -148,8 +148,14 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags, > s->extent_blocks = 1 + (le32_to_cpu(bochs.extent) - 1) / 512; > > s->extent_size = le32_to_cpu(bochs.extent); > - if (s->extent_size == 0) { > - error_setg(errp, "Extent size may not be zero"); > + if (s->extent_size < BDRV_SECTOR_SIZE) { > + /* bximage actually never creates extents smaller than 4k */ > + error_setg(errp, "Extent size must be at least 512"); > + ret = -EINVAL; > + goto fail; > + } else if (!is_power_of_2(s->extent_size)) { > + error_setg(errp, "Extent size %" PRIu32 " is not a power of two", > + s->extent_size); > ret = -EINVAL; > goto fail; > } else if (s->extent_size > 0x800000) { OK, so this is based on your other [PATCH] bochs: Fix memory leak in bochs_open() error path ; good thus far. > @@ -159,7 +165,9 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags, > goto fail; > } > > - if (s->catalog_size < bs->total_sectors / s->extent_size) { > + if (s->catalog_size < DIV_ROUND_UP(bs->total_sectors, > + s->extent_size / BDRV_SECTOR_SIZE)) > + { > error_setg(errp, "Catalog size is too small for this disk size"); > ret = -EINVAL; > goto fail; Nice! If I may voice my liking. Reviewed-by: Laszlo Ersek Thanks Laszlo > diff --git a/tests/qemu-iotests/078 b/tests/qemu-iotests/078 > index 872e734..d4d6da7 100755 > --- a/tests/qemu-iotests/078 > +++ b/tests/qemu-iotests/078 > @@ -69,10 +69,14 @@ _use_sample_img empty.bochs.bz2 > poke_file "$TEST_IMG" "$disk_size_offset" "\x00\xc0\x0f\x00\x00\x00\x00\x7f" > { $QEMU_IO -c "read 2T 4k" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir > > +_use_sample_img empty.bochs.bz2 > +poke_file "$TEST_IMG" "$catalog_size_offset" "\x10\x00\x00\x00" > +{ $QEMU_IO -c "read 0xfbe00 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir > + > echo > echo "== Negative extent size ==" > _use_sample_img empty.bochs.bz2 > -poke_file "$TEST_IMG" "$extent_size_offset" "\xff\xff\xff\xff" > +poke_file "$TEST_IMG" "$extent_size_offset" "\x00\x00\x00\x80" > { $QEMU_IO -c "read 768k 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir > > echo > diff --git a/tests/qemu-iotests/078.out b/tests/qemu-iotests/078.out > index ea95ffd..ca18d2e 100644 > --- a/tests/qemu-iotests/078.out > +++ b/tests/qemu-iotests/078.out > @@ -15,12 +15,14 @@ no file open, try 'help open' > == Too small catalog bitmap for image size == > qemu-io: can't open device TEST_DIR/empty.bochs: Catalog size is too small for this disk size > no file open, try 'help open' > +qemu-io: can't open device TEST_DIR/empty.bochs: Catalog size is too small for this disk size > +no file open, try 'help open' > > == Negative extent size == > -qemu-io: can't open device TEST_DIR/empty.bochs: Extent size 4294967295 is too large > +qemu-io: can't open device TEST_DIR/empty.bochs: Extent size 2147483648 is too large > no file open, try 'help open' > > == Zero extent size == > -qemu-io: can't open device TEST_DIR/empty.bochs: Extent size may not be zero > +qemu-io: can't open device TEST_DIR/empty.bochs: Extent size must be at least 512 > no file open, try 'help open' > *** done >