From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:37437) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Wkybu-0000OB-4K for qemu-devel@nongnu.org; Thu, 15 May 2014 12:34:26 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Wkybp-0003Ei-5G for qemu-devel@nongnu.org; Thu, 15 May 2014 12:34:22 -0400 Date: Thu, 15 May 2014 18:34:51 +0200 From: =?iso-8859-1?Q?Beno=EEt?= Canet Message-ID: <20140515163451.GM2812@irqsave.net> References: <1400163717-1898-1-git-send-email-kwolf@redhat.com> <1400163717-1898-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: <1400163717-1898-3-git-send-email-kwolf@redhat.com> Content-Transfer-Encoding: quoted-printable Subject: Re: [Qemu-devel] [PATCH v2 2/5] qcow1: Check maximum cluster size List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Kevin Wolf Cc: benoit.canet@irqsave.net, ppandit@redhat.com, qemu-devel@nongnu.org, stefanha@redhat.com, qemu-stable@nongnu.org The Thursday 15 May 2014 =E0 16:21:54 (+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 > Cc: qemu-stable@nongnu.org > Signed-off-by: Kevin Wolf > --- > block/qcow.c | 10 ++++++-- > tests/qemu-iotests/092 | 63 ++++++++++++++++++++++++++++++++++++++= ++++++++ > tests/qemu-iotests/092.out | 13 ++++++++++ > tests/qemu-iotests/group | 1 + > 4 files changed, 85 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..d060e6f > --- /dev/null > +++ b/tests/qemu-iotests/092 > @@ -0,0 +1,63 @@ > +#!/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 > + > +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 > +poke_file "$TEST_IMG" "$offset_cluster_bits" "\x08" > +{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filt= er_testdir > +poke_file "$TEST_IMG" "$offset_cluster_bits" "\x11" > +{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filt= er_testdir > + > +# 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..8bf8158 > --- /dev/null > +++ b/tests/qemu-iotests/092.out > @@ -0,0 +1,13 @@ > +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' > +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 2988cfd..0f07440 100644 > --- a/tests/qemu-iotests/group > +++ b/tests/qemu-iotests/group > @@ -98,3 +98,4 @@ > 089 rw auto quick > 090 rw auto quick > 091 rw auto > +092 rw auto quick > --=20 > 1.8.3.1 >=20 Reviewed-by: Benoit Canet