From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:53679) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WSu2X-0008EU-88 for qemu-devel@nongnu.org; Wed, 26 Mar 2014 16:03:15 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WSu2R-0000ck-5H for qemu-devel@nongnu.org; Wed, 26 Mar 2014 16:03:09 -0400 Message-ID: <53333270.2000704@redhat.com> Date: Wed, 26 Mar 2014 21:02:56 +0100 From: Max Reitz MIME-Version: 1.0 References: <1395835569-21193-1-git-send-email-stefanha@redhat.com> <1395835569-21193-11-git-send-email-stefanha@redhat.com> In-Reply-To: <1395835569-21193-11-git-send-email-stefanha@redhat.com> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit Subject: Re: [Qemu-devel] [PATCH for-2.0 10/47] bochs: Use unsigned variables for offsets and sizes (CVE-2014-0147) List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi , qemu-devel@nongnu.org Cc: Kevin Wolf , pmatouse@redhat.com, qemu-stable@nongnu.org On 26.03.2014 13:05, Stefan Hajnoczi wrote: > From: Kevin Wolf > > Gets us rid of integer overflows resulting in negative sizes which > aren't correctly checked. > > Signed-off-by: Kevin Wolf > Reviewed-by: Stefan Hajnoczi > --- > block/bochs.c | 16 ++++++++-------- > tests/qemu-iotests/078 | 8 ++++++++ > tests/qemu-iotests/078.out | 4 ++++ > 3 files changed, 20 insertions(+), 8 deletions(-) > > diff --git a/block/bochs.c b/block/bochs.c > index ef8e381..e923eed 100644 > --- a/block/bochs.c > +++ b/block/bochs.c > @@ -67,13 +67,13 @@ struct bochs_header { > typedef struct BDRVBochsState { > CoMutex lock; > uint32_t *catalog_bitmap; > - int catalog_size; > + uint32_t catalog_size; > > - int data_offset; > + uint32_t data_offset; > > - int bitmap_blocks; > - int extent_blocks; > - int extent_size; > + uint32_t bitmap_blocks; > + uint32_t extent_blocks; > + uint32_t extent_size; > } BDRVBochsState; > > static int bochs_probe(const uint8_t *buf, int buf_size, const char *filename) > @@ -97,7 +97,7 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags, > Error **errp) > { > BDRVBochsState *s = bs->opaque; > - int i; > + uint32_t i; > struct bochs_header bochs; > int ret; > > @@ -153,8 +153,8 @@ fail: > static int64_t seek_to_sector(BlockDriverState *bs, int64_t sector_num) > { > BDRVBochsState *s = bs->opaque; > - int64_t offset = sector_num * 512; > - int64_t extent_index, extent_offset, bitmap_offset; > + uint64_t offset = sector_num * 512; > + uint64_t extent_index, extent_offset, bitmap_offset; > char bitmap_entry; > > // seek to sector > diff --git a/tests/qemu-iotests/078 b/tests/qemu-iotests/078 > index f55f46d..73b573a 100755 > --- a/tests/qemu-iotests/078 > +++ b/tests/qemu-iotests/078 > @@ -42,11 +42,19 @@ _supported_fmt bochs > _supported_proto generic > _supported_os Linux > > +catalog_size_offset=$((0x48)) > + > echo > echo "== Read from a valid image ==" > _use_sample_img empty.bochs.bz2 > { $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir > > +echo > +echo "== Negative catalog size ==" > +_use_sample_img empty.bochs.bz2 > +poke_file "$TEST_IMG" "$catalog_size_offset" "\xff\xff\xff\xff" > +{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir > + > # success, all done > echo "*** done" > rm -f $seq.full > diff --git a/tests/qemu-iotests/078.out b/tests/qemu-iotests/078.out > index 25d37c5..ef8c42d 100644 > --- a/tests/qemu-iotests/078.out > +++ b/tests/qemu-iotests/078.out > @@ -3,4 +3,8 @@ QA output created by 078 > == Read from a valid image == > read 512/512 bytes at offset 0 > 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) > + > +== Negative catalog size == > +qemu-io: can't open device TEST_DIR/empty.bochs: Could not open 'TEST_DIR/empty.bochs': Interrupted system call This does not sound like the error message we'd like, but it is fixed by the next patch. > +no file open, try 'help open' > *** done Reviewed-by: Max Reitz