From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:50359) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wjrhw-000527-PQ for qemu-devel@nongnu.org; Mon, 12 May 2014 11:00:05 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wjrhr-0000Pp-US for qemu-devel@nongnu.org; Mon, 12 May 2014 11:00:00 -0400 Received: from lputeaux-656-01-25-125.w80-12.abo.wanadoo.fr ([80.12.84.125]:47807 helo=paradis.irqsave.net) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wjrhr-0000Pd-Lp for qemu-devel@nongnu.org; Mon, 12 May 2014 10:59:55 -0400 Date: Mon, 12 May 2014 17:00:26 +0200 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140512150026.GE7858@irqsave.net> References: <1399899851-5641-1-git-send-email-kwolf@redhat.com> <1399899851-5641-3-git-send-email-kwolf@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline In-Reply-To: <1399899851-5641-3-git-send-email-kwolf@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH 2/5] qcow1: Check maximum cluster size List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: qemu-devel@nongnu.org, stefanha@redhat.com, ppandit@redhat.com The Monday 12 May 2014 =E0 15:04:08 (+0200), Kevin Wolf wrote : > Huge values for header.cluster_bits cause unbounded allocations (e.g. > for s->cluster_cache) and crash qemu this way. Less huge values may > survive those allocations, but can cause integer overflows later on. >=20 > The only cluster sizes that qemu can create are 4k (for standalone > images) and 512 (for images with backing files), so we can limit it > to 64k. >=20 > Signed-off-by: Kevin Wolf > --- > block/qcow.c | 10 ++++++-- > tests/qemu-iotests/092 | 60 ++++++++++++++++++++++++++++++++++++++= ++++++++ > tests/qemu-iotests/092.out | 9 +++++++ > tests/qemu-iotests/group | 1 + > 4 files changed, 78 insertions(+), 2 deletions(-) > create mode 100755 tests/qemu-iotests/092 > create mode 100644 tests/qemu-iotests/092.out >=20 > diff --git a/block/qcow.c b/block/qcow.c > index 3684794..e60df23 100644 > --- a/block/qcow.c > +++ b/block/qcow.c > @@ -128,11 +128,17 @@ static int qcow_open(BlockDriverState *bs, QDict = *options, int flags, > goto fail; > } > =20 > - if (header.size <=3D 1 || header.cluster_bits < 9) { > - error_setg(errp, "invalid value in qcow header"); > + if (header.size <=3D 1) { > + error_setg(errp, "Image size is too small (must be at least 2 = bytes)"); > ret =3D -EINVAL; > goto fail; > } > + if (header.cluster_bits < 9 || header.cluster_bits > 16) { > + error_setg(errp, "Cluster size must be between 512 and 64k"); > + ret =3D -EINVAL; > + goto fail; > + } > + > if (header.crypt_method > QCOW_CRYPT_AES) { > error_setg(errp, "invalid encryption method in qcow header"); > ret =3D -EINVAL; > diff --git a/tests/qemu-iotests/092 b/tests/qemu-iotests/092 > new file mode 100755 > index 0000000..b0f04e3 > --- /dev/null > +++ b/tests/qemu-iotests/092 > @@ -0,0 +1,60 @@ > +#!/bin/bash > +# > +# qcow1 format input validation tests > +# > +# Copyright (C) 2014 Red Hat, Inc. > +# > +# This program is free software; you can redistribute it and/or modify > +# it under the terms of the GNU General Public License as published by > +# the Free Software Foundation; either version 2 of the License, or > +# (at your option) any later version. > +# > +# This program is distributed in the hope that it will be useful, > +# but WITHOUT ANY WARRANTY; without even the implied warranty of > +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the > +# GNU General Public License for more details. > +# > +# You should have received a copy of the GNU General Public License > +# along with this program. If not, see = . > +# > + > +# creator > +owner=3Dkwolf@redhat.com > + > +seq=3D`basename $0` > +echo "QA output created by $seq" > + > +here=3D`pwd` > +tmp=3D/tmp/$$ > +status=3D1 # failure is the default! > + > +_cleanup() > +{ > + rm -f $TEST_IMG.snap > + _cleanup_test_img > +} > +trap "_cleanup; exit \$status" 0 1 2 3 15 > + > +# get standard environment, filters and checks > +. ./common.rc > +. ./common.filter > + > +_supported_fmt qcow > +_supported_proto generic > +_supported_os Linux > + > +offset_cluster_bits=3D32 > +offset_l2_bits=3D33 This seems to be an extra. > + > +echo > +echo "=3D=3D Invalid cluster size =3D=3D" > +_make_test_img 64M > +poke_file "$TEST_IMG" "$offset_cluster_bits" "\xff" > +{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filt= er_testdir > +poke_file "$TEST_IMG" "$offset_cluster_bits" "\x1f" > +{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filt= er_testdir >>From the code " + if (header.cluster_bits < 9 || header.cluster_bits >= 16) {" Shouldn't the hex values be "\x08" and "\x11" ? > + > +# success, all done > +echo "*** done" > +rm -f $seq.full > +status=3D0 > diff --git a/tests/qemu-iotests/092.out b/tests/qemu-iotests/092.out > new file mode 100644 > index 0000000..9e7367a > --- /dev/null > +++ b/tests/qemu-iotests/092.out > @@ -0,0 +1,9 @@ > +QA output created by 092 > + > +=3D=3D Invalid cluster size =3D=3D > +Formatting 'TEST_DIR/t.IMGFMT', fmt=3DIMGFMT size=3D67108864=20 > +qemu-io: can't open device TEST_DIR/t.qcow: Cluster size must be betwe= en 512 and 64k > +no file open, try 'help open' > +qemu-io: can't open device TEST_DIR/t.qcow: Cluster size must be betwe= en 512 and 64k > +no file open, try 'help open' > +*** done > diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group > index cd3e4d2..ca39ab3 100644 > --- a/tests/qemu-iotests/group > +++ b/tests/qemu-iotests/group > @@ -97,3 +97,4 @@ > 088 rw auto > 090 rw auto quick > 091 rw auto > +092 rw auto quick > --=20 > 1.8.3.1 >=20 >=20