On Sat, Mar 29, 2014 at 12:22:58AM +0100, Max Reitz wrote: > On 26.03.2014 13:06, Stefan Hajnoczi wrote: > >From: Jeff Cody <jc...@redhat.com> > > > >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 <jc...@redhat.com> > >Reviewed-by: Stefan Hajnoczi <stefa...@redhat.com> > >Signed-off-by: Kevin Wolf <kw...@redhat.com> > >--- > > 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 <http://www.gnu.org/licenses/>. > >+# > >+ > >+# creator > >+owner=jc...@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. >
Indeed it is not. I changed the test, but forgot to update the comment. I already submitted a v2; Stefan, do you want me to submit a v3 with a comment change, or do you want to fix up the comment when applying the patch (note you'd need to change the .out file as well)? > >+_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. > In the v2 series, this was changed to ENOTSUP, since we consciously don't support the larger size, even though the size may be valid. > >+ > >+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 <mre...@redhat.com>
