Re: [Qemu-devel] [PATCH v2] vmdk: Fix integer overflow in offset calculation

[Max Reitz]

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 <address@hidden>
> Signed-off-by: Fam Zheng <address@hidden>
> ---
>   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 <address@hidden>