* [Qemu-devel] [PATCH] vmdk: Fix integer overflow in offset calculation
@ 2014-09-12 9:51 Fam Zheng
2014-09-13 18:01 ` Max Reitz
0 siblings, 1 reply; 3+ messages in thread
From: Fam Zheng @ 2014-09-12 9:51 UTC (permalink / raw)
To: qemu-devel
Cc: Kevin Wolf, Fam Zheng, Jeff Cody, Peter Lieven, Stefan Hajnoczi,
Paolo Bonzini
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/059 | 6 ++++++
tests/qemu-iotests/059.out | 7 +++++++
3 files changed, 14 insertions(+), 1 deletion(-)
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/059 b/tests/qemu-iotests/059
index 3c053c2..6ed2a25 100755
--- a/tests/qemu-iotests/059
+++ b/tests/qemu-iotests/059
@@ -126,6 +126,12 @@ _img_info
$QEMU_IO -c "write -P 0xa 900G 512" "$TEST_IMG" | _filter_qemu_io
$QEMU_IO -c "read -v 900G 1024" "$TEST_IMG" | _filter_qemu_io
+echo
+echo "=== Testing reading and writing at big offset ==="
+_make_test_img 4T
+$QEMU_IO -c "write -P 0xa 1T 1024" "$TEST_IMG" | _filter_qemu_io
+$QEMU_IO -c "read -P 0xa 1T 1024" "$TEST_IMG" | _filter_qemu_io
+
# success, all done
echo "*** done"
rm -f $seq.full
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index 0dadba6..51c72a6 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -2332,4 +2332,11 @@ e1000003e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
e1000003f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
read 1024/1024 bytes at offset 966367641600
1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+=== Testing reading and writing at big offset ===
+Formatting 'TEST_DIR/iotest-version3.IMGFMT', fmt=IMGFMT size=4398046511104
+wrote 1024/1024 bytes at offset 1099511627776
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 1024/1024 bytes at offset 1099511627776
+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
*** done
--
1.9.3
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] vmdk: Fix integer overflow in offset calculation
2014-09-12 9:51 [Qemu-devel] [PATCH] vmdk: Fix integer overflow in offset calculation Fam Zheng
@ 2014-09-13 18:01 ` Max Reitz
2014-09-15 2:27 ` Fam Zheng
0 siblings, 1 reply; 3+ messages in thread
From: Max Reitz @ 2014-09-13 18:01 UTC (permalink / raw)
To: Fam Zheng, qemu-devel
Cc: Kevin Wolf, Paolo Bonzini, Jeff Cody, Peter Lieven,
Stefan Hajnoczi
On 12.09.2014 11:51, 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/059 | 6 ++++++
> tests/qemu-iotests/059.out | 7 +++++++
> 3 files changed, 14 insertions(+), 1 deletion(-)
>
> 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;
The fix is okay.
> diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
> index 3c053c2..6ed2a25 100755
> --- a/tests/qemu-iotests/059
> +++ b/tests/qemu-iotests/059
> @@ -126,6 +126,12 @@ _img_info
> $QEMU_IO -c "write -P 0xa 900G 512" "$TEST_IMG" | _filter_qemu_io
> $QEMU_IO -c "read -v 900G 1024" "$TEST_IMG" | _filter_qemu_io
>
> +echo
> +echo "=== Testing reading and writing at big offset ==="
> +_make_test_img 4T
> +$QEMU_IO -c "write -P 0xa 1T 1024" "$TEST_IMG" | _filter_qemu_io
> +$QEMU_IO -c "read -P 0xa 1T 1024" "$TEST_IMG" | _filter_qemu_io
> +
> # success, all done
> echo "*** done"
> rm -f $seq.full
> diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
> index 0dadba6..51c72a6 100644
> --- a/tests/qemu-iotests/059.out
> +++ b/tests/qemu-iotests/059.out
> @@ -2332,4 +2332,11 @@ e1000003e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> e1000003f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> read 1024/1024 bytes at offset 966367641600
> 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +
> +=== Testing reading and writing at big offset ===
> +Formatting 'TEST_DIR/iotest-version3.IMGFMT', fmt=IMGFMT size=4398046511104
> +wrote 1024/1024 bytes at offset 1099511627776
> +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> +read 1024/1024 bytes at offset 1099511627776
> +1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> *** done
This test is not. It works even without the fix for me. The problem is
that this does not depend on the guest disk size or the guest offset,
but on the host offset. So the problem only appears if the image has
been filled enough so that the host offsets grow large enough
(apparently, at least).
However, the first host offset written to does depend on the image size.
For 2 GB images, it's 0x90000; for 4 TB images, it's 0x20110000; and for
16 TB images, it finally overflows, *independently* on the guest offset
written to. So I suggest you create a 16 TB image here (or maybe an
image which is as large as possible, for testing purposes) and then you
can write anywhere (maybe once at the very beginning and once at the
very end, if that works).
Max
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [Qemu-devel] [PATCH] vmdk: Fix integer overflow in offset calculation
2014-09-13 18:01 ` Max Reitz
@ 2014-09-15 2:27 ` Fam Zheng
0 siblings, 0 replies; 3+ messages in thread
From: Fam Zheng @ 2014-09-15 2:27 UTC (permalink / raw)
To: Max Reitz
Cc: Kevin Wolf, Jeff Cody, Peter Lieven, qemu-devel, Stefan Hajnoczi,
Paolo Bonzini
On Sat, 09/13 20:01, Max Reitz wrote:
> On 12.09.2014 11:51, 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/059 | 6 ++++++
> > tests/qemu-iotests/059.out | 7 +++++++
> > 3 files changed, 14 insertions(+), 1 deletion(-)
> >
> >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;
>
> The fix is okay.
>
> >diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
> >index 3c053c2..6ed2a25 100755
> >--- a/tests/qemu-iotests/059
> >+++ b/tests/qemu-iotests/059
> >@@ -126,6 +126,12 @@ _img_info
> > $QEMU_IO -c "write -P 0xa 900G 512" "$TEST_IMG" | _filter_qemu_io
> > $QEMU_IO -c "read -v 900G 1024" "$TEST_IMG" | _filter_qemu_io
> >+echo
> >+echo "=== Testing reading and writing at big offset ==="
> >+_make_test_img 4T
> >+$QEMU_IO -c "write -P 0xa 1T 1024" "$TEST_IMG" | _filter_qemu_io
> >+$QEMU_IO -c "read -P 0xa 1T 1024" "$TEST_IMG" | _filter_qemu_io
> >+
> > # success, all done
> > echo "*** done"
> > rm -f $seq.full
> >diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
> >index 0dadba6..51c72a6 100644
> >--- a/tests/qemu-iotests/059.out
> >+++ b/tests/qemu-iotests/059.out
> >@@ -2332,4 +2332,11 @@ e1000003e0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> > e1000003f0: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 ................
> > read 1024/1024 bytes at offset 966367641600
> > 1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> >+
> >+=== Testing reading and writing at big offset ===
> >+Formatting 'TEST_DIR/iotest-version3.IMGFMT', fmt=IMGFMT size=4398046511104
> >+wrote 1024/1024 bytes at offset 1099511627776
> >+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> >+read 1024/1024 bytes at offset 1099511627776
> >+1 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
> > *** done
>
> This test is not. It works even without the fix for me. The problem is that
> this does not depend on the guest disk size or the guest offset, but on the
> host offset. So the problem only appears if the image has been filled enough
> so that the host offsets grow large enough (apparently, at least).
>
> However, the first host offset written to does depend on the image size. For
> 2 GB images, it's 0x90000; for 4 TB images, it's 0x20110000; and for 16 TB
> images, it finally overflows, *independently* on the guest offset written
> to. So I suggest you create a 16 TB image here (or maybe an image which is
> as large as possible, for testing purposes) and then you can write anywhere
> (maybe once at the very beginning and once at the very end, if that works).
Ah yes, thanks for pointing out! I will respin and do what you suggest in 005.
Fam
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-09-15 2:27 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-12 9:51 [Qemu-devel] [PATCH] vmdk: Fix integer overflow in offset calculation Fam Zheng
2014-09-13 18:01 ` Max Reitz
2014-09-15 2:27 ` 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).