From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:44393) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WTg7A-00047i-29 for qemu-devel@nongnu.org; Fri, 28 Mar 2014 19:23:13 -0400 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WTg75-0004tR-7D for qemu-devel@nongnu.org; Fri, 28 Mar 2014 19:23:08 -0400 Message-ID: <53360452.6090805@redhat.com> Date: Sat, 29 Mar 2014 00:22:58 +0100 From: Max Reitz MIME-Version: 1.0 References: <1395835569-21193-1-git-send-email-stefanha@redhat.com> <1395835569-21193-40-git-send-email-stefanha@redhat.com> In-Reply-To: <1395835569-21193-40-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 39/47] block: vdi bounds check qemu-io tests List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: Stefan Hajnoczi , qemu-devel@nongnu.org Cc: Kevin Wolf , Jeff Cody , pmatouse@redhat.com, qemu-stable@nongnu.org On 26.03.2014 13:06, Stefan Hajnoczi wrote: > From: Jeff Cody > > This test checks for proper bounds checking of some VDI input > headers. The following is checked: > > 1. Max image size (1024TB) with the appropriate Blocks In Image > value (0x3fffffff) is detected as valid. > > 2. Image size exceeding max (1024TB) is seen as invalid > > 3. Valid image size but with Blocks In Image value that is too > small fails > > 4. Blocks In Image size exceeding max (0x3fffffff) is seen as invalid > > 5. 64MB image, with 64 Blocks In Image, and 1MB Block Size is seen > as valid > > 6. Block Size < 1MB not supported > > 7. Block Size > 1MB not supported > > Signed-off-by: Jeff Cody > Reviewed-by: Stefan Hajnoczi > Signed-off-by: Kevin Wolf > --- > tests/qemu-iotests/084 | 104 +++++++++++++++++++++++++++++++++++++++++++++ > tests/qemu-iotests/084.out | 33 ++++++++++++++ > tests/qemu-iotests/group | 1 + > 3 files changed, 138 insertions(+) > create mode 100755 tests/qemu-iotests/084 > create mode 100644 tests/qemu-iotests/084.out > > diff --git a/tests/qemu-iotests/084 b/tests/qemu-iotests/084 > new file mode 100755 > index 0000000..10a5a65 > --- /dev/null > +++ b/tests/qemu-iotests/084 > @@ -0,0 +1,104 @@ > +#!/bin/bash > +# > +# Test case for VDI header corruption; image too large, and too many blocks > +# > +# Copyright (C) 2013 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=jcody@redhat.com > + > +seq=`basename $0` > +echo "QA output created by $seq" > + > +here=`pwd` > +tmp=/tmp/$$ > +status=1 # failure is the default! > + > +_cleanup() > +{ > + _cleanup_test_img > +} > +trap "_cleanup; exit \$status" 0 1 2 3 15 > + > +# get standard environment, filters and checks > +. ./common.rc > +. ./common.filter > + > +# This tests vdi-specific header fields > +_supported_fmt vdi > +_supported_proto generic > +_supported_os Linux > + > +ds_offset=368 # disk image size field offset > +bs_offset=376 # block size field offset > +bii_offset=384 # block in image field offset > + > +echo > +echo "=== Testing image size bounds ===" > +echo > +_make_test_img 64M > + > +# check for image size too large > +# poke max image size, and appropriate blocks_in_image value > +echo "Test 1: Maximum size (1024 TB):" > +poke_file "$TEST_IMG" "$ds_offset" "\x00\x00\xf0\xff\xff\xff\x03\x00" > +poke_file "$TEST_IMG" "$bii_offset" "\xff\xff\xff\x3f" > +_img_info > + > +echo > +echo "Test 2: Size too large (1024TB + 1)" > +# This should be too large (-EINVAL): > +poke_file "$TEST_IMG" "$ds_offset" "\x00\x00\xf1\xff\xff\xff\x03\x00" > +_img_info > + > +echo > +echo "Test 3: Size valid (64M), but Blocks In Image too small (63)" > +# This sets the size to 64M, but with a blocks_in_image size that is > +# too small > +poke_file "$TEST_IMG" "$ds_offset" "\x00\x00\x00\x04\x00\x00\x00\x00" > +# For a 64M image, we would need a blocks_in_image value of at least 64, > +# so 63 should be too small and give us -ENOTSUP > +poke_file "$TEST_IMG" "$bii_offset" "\x3f\x00\x00\x00" > +_img_info > + > +echo > +echo "Test 4: Size valid (64M), but Blocks In Image exceeds max allowed" > +# Now check the bounds of blocks_in_image - 0x3fffffff should be the max > +# value here, and we should get -ENOTSUP > +poke_file "$TEST_IMG" "$bii_offset" "\x00\x00\x00\x40" > +_img_info > + > +# Finally, 1MB is the only block size supported. Verify that > +# a value != 1MB results in error, both smaller and larger > +echo > +echo "Test 5: Valid Image: 64MB, Blocks In Image 64, Block Size 1MB" > +poke_file "$TEST_IMG" "$bii_offset" "\x40\x00\x00\x00" # reset bii to valid > +poke_file "$TEST_IMG" "$bs_offset" "\x00\x00\x10\x00" # valid > +_img_info > +echo > +echo "Test 6: Block Size != 1MB; too small test (1MB - 1)" > +poke_file "$TEST_IMG" "$bs_offset" "\xff\xff\x0f\x00" # invalid (too small) > +_img_info > +echo > +echo "Test 7: Block Size != 1MB; too large test (1MB + 1)" > +poke_file "$TEST_IMG" "$bs_offset" "\x00\x00\x11\x00" # invalid (too large) 0x110000 is not 1 MB + 1. > +_img_info > +# success, all done > +echo > +echo "*** done" > +rm -f $seq.full > +status=0 > diff --git a/tests/qemu-iotests/084.out b/tests/qemu-iotests/084.out > new file mode 100644 > index 0000000..1e320f5 > --- /dev/null > +++ b/tests/qemu-iotests/084.out > @@ -0,0 +1,33 @@ > +QA output created by 084 > + > +=== Testing image size bounds === > + > +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 > +Test 1: Maximum size (1024 TB): > +image: TEST_DIR/t.IMGFMT > +file format: IMGFMT > +virtual size: 1024T (1125899905794048 bytes) > +cluster_size: 1048576 > + > +Test 2: Size too large (1024TB + 1) > +qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Could not open 'TEST_DIR/t.IMGFMT': Invalid argument Hm, maybe E2BIG would be better here. > + > +Test 3: Size valid (64M), but Blocks In Image too small (63) > +qemu-img: Could not open 'TEST_DIR/t.IMGFMT': unsupported VDI image (disk size 67108864, image bitmap has room for 66060288) > + > +Test 4: Size valid (64M), but Blocks In Image exceeds max allowed > +qemu-img: Could not open 'TEST_DIR/t.IMGFMT': unsupported VDI image (too many blocks) > + > +Test 5: Valid Image: 64MB, Blocks In Image 64, Block Size 1MB > +image: TEST_DIR/t.IMGFMT > +file format: IMGFMT > +virtual size: 64M (67108864 bytes) > +cluster_size: 1048576 > + > +Test 6: Block Size != 1MB; too small test (1MB - 1) > +qemu-img: Could not open 'TEST_DIR/t.IMGFMT': unsupported VDI image (sector size 1048575 is not 1048576) > + > +Test 7: Block Size != 1MB; too large test (1MB + 1) > +qemu-img: Could not open 'TEST_DIR/t.IMGFMT': unsupported VDI image (sector size 1114112 is not 1048576) As can be seen here, 0x110000 really does not equal 1 MB + 1. > + > +*** done > diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group > index ed44f35..c51640c 100644 > --- a/tests/qemu-iotests/group > +++ b/tests/qemu-iotests/group > @@ -89,6 +89,7 @@ > 081 rw auto > 082 rw auto quick > 083 rw auto > +084 img auto > 085 rw auto > 086 rw auto quick > 087 rw auto Albeit the comment being wrong (which should be fixed) and EINVAL being a little bit confusing for too big images (there should be a follow-up patch for this): Reviewed-by: Max Reitz