* [Qemu-devel] [PATCH v2] vmdk: Fix integer overflow in offset calculation
@ 2014-09-15 2:32 Fam Zheng
2014-09-19 11:52 ` Max Reitz
0 siblings, 1 reply; 4+ messages in thread
From: Fam Zheng @ 2014-09-15 2:32 UTC (permalink / raw)
To: qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi, Max Reitz
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 <mark.cave-ayland@ilande.co.uk>
Signed-off-by: Fam Zheng <famz@redhat.com>
---
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=5368709120000
+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
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH v2] vmdk: Fix integer overflow in offset calculation
2014-09-15 2:32 [Qemu-devel] [PATCH v2] vmdk: Fix integer overflow in offset calculation Fam Zheng
@ 2014-09-19 11:52 ` Max Reitz
2014-09-19 11:58 ` Max Reitz
2014-09-22 7:03 ` Fam Zheng
0 siblings, 2 replies; 4+ messages in thread
From: Max Reitz @ 2014-09-19 11:52 UTC (permalink / raw)
To: Fam Zheng, qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi
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 <mark.cave-ayland@ilande.co.uk>
> Signed-off-by: Fam Zheng <famz@redhat.com>
> ---
> 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=5368709120000
> +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 <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH v2] vmdk: Fix integer overflow in offset calculation
2014-09-19 11:52 ` Max Reitz
@ 2014-09-19 11:58 ` Max Reitz
2014-09-22 7:03 ` Fam Zheng
1 sibling, 0 replies; 4+ messages in thread
From: Max Reitz @ 2014-09-19 11:58 UTC (permalink / raw)
To: Fam Zheng, qemu-devel; +Cc: Kevin Wolf, Stefan Hajnoczi
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 <mark.cave-ayland@ilande.co.uk>
>> Signed-off-by: Fam Zheng <famz@redhat.com>
>> ---
>> 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=5368709120000
>> +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 <mreitz@redhat.com>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [Qemu-devel] [PATCH v2] vmdk: Fix integer overflow in offset calculation
2014-09-19 11:52 ` Max Reitz
2014-09-19 11:58 ` Max Reitz
@ 2014-09-22 7:03 ` Fam Zheng
1 sibling, 0 replies; 4+ messages in thread
From: Fam Zheng @ 2014-09-22 7:03 UTC (permalink / raw)
To: Max Reitz; +Cc: Kevin Wolf, qemu-devel, Stefan Hajnoczi
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 <mark.cave-ayland@ilande.co.uk>
> >Signed-off-by: Fam Zheng <famz@redhat.com>
> >---
> > 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=5368709120000
> >+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
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-09-22 7:03 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-15 2:32 [Qemu-devel] [PATCH v2] vmdk: Fix integer overflow in offset calculation Fam Zheng
2014-09-19 11:52 ` Max Reitz
2014-09-19 11:58 ` Max Reitz
2014-09-22 7:03 ` Fam Zheng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).