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

[Max Reitz]

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