[Qemu-devel] [PATCH v2] vmdk: Fix integer overflow in offset calculation
This fixes the bug introduced by commit c6ac36e (vmdk: Optimize cluster allocation). $ ~/build/master/qemu-io /stor/vm/arch.vmdk -c 'write 2G 1k' write failed: Invalid argument Reported-by: Mark Cave-Ayland Signed-off-by: Fam Zheng --- block/vmdk.c | 2 +- tests/qemu-iotests/005 | 10 +- tests/qemu-iotests/005.out | 10 +- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index a1cb911..3fd7738 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1113,7 +1113,7 @@ static int get_cluster_offset(BlockDriverState *bs, uint32_t min_count, *l2_table; bool zeroed = false; int64_t ret; -int32_t cluster_sector; +int64_t cluster_sector; if (m_data) { m_data->valid = 0; diff --git a/tests/qemu-iotests/005 b/tests/qemu-iotests/005 index ba1236d..fc8944c 100755 --- a/tests/qemu-iotests/005 +++ b/tests/qemu-iotests/005 @@ -59,7 +59,7 @@ fi echo echo "creating large image" -_make_test_img 5000G +_make_test_img 16T echo echo "small read" @@ -69,6 +69,14 @@ echo echo "small write" $QEMU_IO -c "write 8192 4096" "$TEST_IMG" | _filter_qemu_io +echo +echo "small read at high offset" +$QEMU_IO -c "read 4T 4096" "$TEST_IMG" | _filter_qemu_io + +echo +echo "small write at high offset" +$QEMU_IO -c "write 4T 4096" "$TEST_IMG" | _filter_qemu_io + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/005.out b/tests/qemu-iotests/005.out index 2d3e7df..fd6aed9 100644 --- a/tests/qemu-iotests/005.out +++ b/tests/qemu-iotests/005.out @@ -1,7 +1,7 @@ QA output created by 005 creating large image -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=536870912 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=17592186044416 small read read 4096/4096 bytes at offset 1024 @@ -10,4 +10,12 @@ read 4096/4096 bytes at offset 1024 small write wrote 4096/4096 bytes at offset 8192 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +small read at high offset +read 4096/4096 bytes at offset 4398046511104 +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +small write at high offset +wrote 4096/4096 bytes at offset 4398046511104 +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) *** done -- 1.9.3
Re: [Qemu-devel] [PATCH v2] vmdk: Fix integer overflow in offset calculation
On 15.09.2014 04:32, Fam Zheng wrote: This fixes the bug introduced by commit c6ac36e (vmdk: Optimize cluster allocation). $ ~/build/master/qemu-io /stor/vm/arch.vmdk -c 'write 2G 1k' write failed: Invalid argument Reported-by: Mark Cave-Ayland Signed-off-by: Fam Zheng --- block/vmdk.c | 2 +- tests/qemu-iotests/005 | 10 +- tests/qemu-iotests/005.out | 10 +- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index a1cb911..3fd7738 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1113,7 +1113,7 @@ static int get_cluster_offset(BlockDriverState *bs, uint32_t min_count, *l2_table; bool zeroed = false; int64_t ret; -int32_t cluster_sector; +int64_t cluster_sector; if (m_data) { m_data->valid = 0; diff --git a/tests/qemu-iotests/005 b/tests/qemu-iotests/005 index ba1236d..fc8944c 100755 --- a/tests/qemu-iotests/005 +++ b/tests/qemu-iotests/005 @@ -59,7 +59,7 @@ fi echo echo "creating large image" -_make_test_img 5000G +_make_test_img 16T echo echo "small read" @@ -69,6 +69,14 @@ echo echo "small write" $QEMU_IO -c "write 8192 4096" "$TEST_IMG" | _filter_qemu_io +echo +echo "small read at high offset" +$QEMU_IO -c "read 4T 4096" "$TEST_IMG" | _filter_qemu_io + +echo +echo "small write at high offset" +$QEMU_IO -c "write 4T 4096" "$TEST_IMG" | _filter_qemu_io + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/005.out b/tests/qemu-iotests/005.out index 2d3e7df..fd6aed9 100644 --- a/tests/qemu-iotests/005.out +++ b/tests/qemu-iotests/005.out @@ -1,7 +1,7 @@ QA output created by 005 creating large image -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=536870912 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=17592186044416 small read read 4096/4096 bytes at offset 1024 @@ -10,4 +10,12 @@ read 4096/4096 bytes at offset 1024 small write wrote 4096/4096 bytes at offset 8192 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +small read at high offset +read 4096/4096 bytes at offset 4398046511104 +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +small write at high offset +wrote 4096/4096 bytes at offset 4398046511104 +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) *** done Okay, this test works for VMDK. However, now this test no longer works with raw, at least not on my system (ftruncate() fails). So we could either exempt raw from this test like vpc (which is probably fine since I don't see the point in trying to create such huge raw images; if it works for other image formats, that should be fine) or we (you) cannot reuse this test. In case you opt for the former (exempt raw like vpc): Reviewed-by: Max Reitz
Re: [Qemu-devel] [PATCH v2] vmdk: Fix integer overflow in offset calculation
On 19.09.2014 13:52, Max Reitz wrote: On 15.09.2014 04:32, Fam Zheng wrote: This fixes the bug introduced by commit c6ac36e (vmdk: Optimize cluster allocation). $ ~/build/master/qemu-io /stor/vm/arch.vmdk -c 'write 2G 1k' write failed: Invalid argument Reported-by: Mark Cave-Ayland Signed-off-by: Fam Zheng --- block/vmdk.c | 2 +- tests/qemu-iotests/005 | 10 +- tests/qemu-iotests/005.out | 10 +- 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/block/vmdk.c b/block/vmdk.c index a1cb911..3fd7738 100644 --- a/block/vmdk.c +++ b/block/vmdk.c @@ -1113,7 +1113,7 @@ static int get_cluster_offset(BlockDriverState *bs, uint32_t min_count, *l2_table; bool zeroed = false; int64_t ret; -int32_t cluster_sector; +int64_t cluster_sector; if (m_data) { m_data->valid = 0; diff --git a/tests/qemu-iotests/005 b/tests/qemu-iotests/005 index ba1236d..fc8944c 100755 --- a/tests/qemu-iotests/005 +++ b/tests/qemu-iotests/005 @@ -59,7 +59,7 @@ fi echo echo "creating large image" -_make_test_img 5000G +_make_test_img 16T echo echo "small read" @@ -69,6 +69,14 @@ echo echo "small write" $QEMU_IO -c "write 8192 4096" "$TEST_IMG" | _filter_qemu_io +echo +echo "small read at high offset" +$QEMU_IO -c "read 4T 4096" "$TEST_IMG" | _filter_qemu_io + +echo +echo "small write at high offset" +$QEMU_IO -c "write 4T 4096" "$TEST_IMG" | _filter_qemu_io + # success, all done echo "*** done" rm -f $seq.full diff --git a/tests/qemu-iotests/005.out b/tests/qemu-iotests/005.out index 2d3e7df..fd6aed9 100644 --- a/tests/qemu-iotests/005.out +++ b/tests/qemu-iotests/005.out @@ -1,7 +1,7 @@ QA output created by 005 creating large image -Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=536870912 +Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=17592186044416 small read read 4096/4096 bytes at offset 1024 @@ -10,4 +10,12 @@ read 4096/4096 bytes at offset 1024 small write wrote 4096/4096 bytes at offset 8192 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +small read at high offset +read 4096/4096 bytes at offset 4398046511104 +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) + +small write at high offset +wrote 4096/4096 bytes at offset 4398046511104 +4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) *** done Okay, this test works for VMDK. However, now this test no longer works with raw, at least not on my system (ftruncate() fails). So we could either exempt raw from this test like vpc (which is probably fine since I don't see the point in trying to create such huge raw images; if it works for other image formats, that should be fine) or we (you) cannot reuse this test. Oh, I forgot to add: I only tested qcow2, vmdk and raw; so there might be other image formats which no longer work with this test. I'm completely fine with excluding all of them from this, because failure to pass it would then be format-specific and no longer a general problem of the block layer (which this generic test is probably for). Max In case you opt for the former (exempt raw like vpc): Reviewed-by: Max Reitz
Re: [Qemu-devel] [PATCH v2] vmdk: Fix integer overflow in offset calculation
On Fri, 09/19 13:52, Max Reitz wrote: > On 15.09.2014 04:32, Fam Zheng wrote: > >This fixes the bug introduced by commit c6ac36e (vmdk: Optimize cluster > >allocation). > > > >$ ~/build/master/qemu-io /stor/vm/arch.vmdk -c 'write 2G 1k' > >write failed: Invalid argument > > > >Reported-by: Mark Cave-Ayland > >Signed-off-by: Fam Zheng > >--- > > block/vmdk.c | 2 +- > > tests/qemu-iotests/005 | 10 +- > > tests/qemu-iotests/005.out | 10 +- > > 3 files changed, 19 insertions(+), 3 deletions(-) > > > >diff --git a/block/vmdk.c b/block/vmdk.c > >index a1cb911..3fd7738 100644 > >--- a/block/vmdk.c > >+++ b/block/vmdk.c > >@@ -1113,7 +1113,7 @@ static int get_cluster_offset(BlockDriverState *bs, > > uint32_t min_count, *l2_table; > > bool zeroed = false; > > int64_t ret; > >-int32_t cluster_sector; > >+int64_t cluster_sector; > > if (m_data) { > > m_data->valid = 0; > >diff --git a/tests/qemu-iotests/005 b/tests/qemu-iotests/005 > >index ba1236d..fc8944c 100755 > >--- a/tests/qemu-iotests/005 > >+++ b/tests/qemu-iotests/005 > >@@ -59,7 +59,7 @@ fi > > echo > > echo "creating large image" > >-_make_test_img 5000G > >+_make_test_img 16T > > echo > > echo "small read" > >@@ -69,6 +69,14 @@ echo > > echo "small write" > > $QEMU_IO -c "write 8192 4096" "$TEST_IMG" | _filter_qemu_io > >+echo > >+echo "small read at high offset" > >+$QEMU_IO -c "read 4T 4096" "$TEST_IMG" | _filter_qemu_io > >+ > >+echo > >+echo "small write at high offset" > >+$QEMU_IO -c "write 4T 4096" "$TEST_IMG" | _filter_qemu_io > >+ > > # success, all done > > echo "*** done" > > rm -f $seq.full > >diff --git a/tests/qemu-iotests/005.out b/tests/qemu-iotests/005.out > >index 2d3e7df..fd6aed9 100644 > >--- a/tests/qemu-iotests/005.out > >+++ b/tests/qemu-iotests/005.out > >@@ -1,7 +1,7 @@ > > QA output created by 005 > > creating large image > >-Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=536870912 > >+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=17592186044416 > > small read > > read 4096/4096 bytes at offset 1024 > >@@ -10,4 +10,12 @@ read 4096/4096 bytes at offset 1024 > > small write > > wrote 4096/4096 bytes at offset 8192 > > 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) > >+ > >+small read at high offset > >+read 4096/4096 bytes at offset 4398046511104 > >+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) > >+ > >+small write at high offset > >+wrote 4096/4096 bytes at offset 4398046511104 > >+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec) > > *** done > > Okay, this test works for VMDK. However, now this test no longer works with > raw, at least not on my system (ftruncate() fails). So we could either > exempt raw from this test like vpc (which is probably fine since I don't see > the point in trying to create such huge raw images; if it works for other > image formats, that should be fine) or we (you) cannot reuse this test. > I tested raw on my system and it passed. Maybe it's because of the file system. Let's keep existing test and add the new case in a separate file. Fam