[Qemu-devel] [PATCH EMBARGOED 1/7] vmdk: Make VMDK3Header QEMU_PACKED

[Qemu-devel] [PATCH EMBARGOED 1/7] vmdk: Make VMDK3Header QEMU_PACKED

[Fam Zheng]

Although the fields are all uint32_t, it's best to make it consistent
that all on disk structures are QEMU_PACKED.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/vmdk.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 3756333..8ff43b9 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -62,7 +62,7 @@ typedef struct {
     uint32_t cylinders;
     uint32_t heads;
     uint32_t sectors_per_track;
-} VMDK3Header;
+} QEMU_PACKED VMDK3Header;
 
 typedef struct {
     uint32_t version;
-- 
1.8.3.4

[Qemu-devel] [PATCH EMBARGOED 2/7] vmdk: use unsigned values for on disk header fields

[Fam Zheng]

The size and offset fields are all non-negative values, use uint64_t for
them to avoid getting negative in memory value by int overflow.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/vmdk.c | 16 ++++++++--------
 1 file changed, 8 insertions(+), 8 deletions(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index 8ff43b9..3c6fa47 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -67,14 +67,14 @@ typedef struct {
 typedef struct {
     uint32_t version;
     uint32_t flags;
-    int64_t capacity;
-    int64_t granularity;
-    int64_t desc_offset;
-    int64_t desc_size;
-    int32_t num_gtes_per_gte;
-    int64_t rgd_offset;
-    int64_t gd_offset;
-    int64_t grain_offset;
+    uint64_t capacity;
+    uint64_t granularity;
+    uint64_t desc_offset;
+    uint64_t desc_size;
+    uint32_t num_gtes_per_gte;
+    uint64_t rgd_offset;
+    uint64_t gd_offset;
+    uint64_t grain_offset;
     char filler[1];
     char check_bytes[4];
     uint16_t compressAlgorithm;
-- 
1.8.3.4

[Qemu-devel] [PATCH EMBARGOED 3/7] qemu-iotests: add empty test case for vmdk

[Fam Zheng]

Will add vmdk specific tests later here.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 tests/qemu-iotests/059     | 51 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/059.out |  2 ++
 tests/qemu-iotests/group   |  1 +
 3 files changed, 54 insertions(+)
 create mode 100755 tests/qemu-iotests/059
 create mode 100644 tests/qemu-iotests/059.out

diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
new file mode 100755
index 0000000..996d85c
--- /dev/null
+++ b/tests/qemu-iotests/059
@@ -0,0 +1,51 @@
+#!/bin/bash
+#
+# Test case for vmdk
+#
+# Copyright (C) 2011 Red Hat, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2 of the License, or
+# (at your option) any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+#
+
+# creator
+owner=famz@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+
+_cleanup()
+{
+	_cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+# This tests qcow2-specific low-level functionality
+_supported_fmt vmdk
+_supported_proto generic
+_supported_os Linux
+
+granularity_offset=16
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
new file mode 100644
index 0000000..4ca7f29
--- /dev/null
+++ b/tests/qemu-iotests/059.out
@@ -0,0 +1,2 @@
+QA output created by 059
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 1a1a30a..732dff8 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -65,3 +65,4 @@
 056 rw auto backing
 057 rw auto
 058 rw auto
+059 rw auto
-- 
1.8.3.4

[Qemu-devel] [PATCH EMBARGOED 4/7] vmdk: check granularity field in opening

[Fam Zheng]

Granularity is used to calculate the cluster size and allocate r/w
buffer. Check the value from image before using it, so we don't abort()
for unbounded memory allocation.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/vmdk.c           | 13 +++++++++++++
 tests/qemu-iotests/059 |  6 ++++++
 2 files changed, 19 insertions(+)

diff --git a/block/vmdk.c b/block/vmdk.c
index 3c6fa47..dbedff7 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -475,6 +475,13 @@ static int vmdk_open_vmdk3(BlockDriverState *bs,
     if (ret < 0) {
         return ret;
     }
+
+    if (le32_to_cpu(header.granularity) > 0x200000) {
+        /* 1GB for one cluster is unrealistic */
+        error_report("invalid granularity, image may be corrupt");
+        return -EINVAL;
+    }
+
     extent = vmdk_add_extent(bs,
                              bs->file, false,
                              le32_to_cpu(header.disk_sectors),
@@ -561,6 +568,12 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
         header = footer.header;
     }
 
+    if (le32_to_cpu(header.granularity) > 0x200000) {
+        /* 1GB for one cluster is unrealistic */
+        error_report("invalid granularity, image may be corrupt");
+        return -EINVAL;
+    }
+
     if (le32_to_cpu(header.version) >= 3) {
         char buf[64];
         snprintf(buf, sizeof(buf), "VMDK version %d",
diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
index 996d85c..18807d3 100755
--- a/tests/qemu-iotests/059
+++ b/tests/qemu-iotests/059
@@ -45,6 +45,12 @@ _supported_os Linux
 
 granularity_offset=16
 
+echo "=== Testing invalid granularity ==="
+echo
+_make_test_img 64M
+poke_file "$TEST_IMG" "$granularity_offset" "\xff\xff\xff\xff\xff\xff\xff\xff"
+$QEMU_IO -c "read 0 512" $TEST_IMG 2>&1 | _filter_qemu_io | _filter_testdir
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
-- 
1.8.3.4

[Qemu-devel] [PATCH EMBARGOED 5/7] vmdk: check cluster size in vmdk_open

[Fam Zheng]

VMDK specification doesn't give upper limit of cluster size
(header.grainSize). But we need to avoid unbounded memory allocation.
Check the value when opening and refuse too big value (>512MB cluster
size).

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/vmdk.c               | 1 +
 tests/qemu-iotests/059     | 2 +-
 tests/qemu-iotests/059.out | 6 ++++++
 3 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/block/vmdk.c b/block/vmdk.c
index dbedff7..1d6bc39 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -41,6 +41,7 @@
 #define VMDK4_GD_AT_END 0xffffffffffffffffULL
 
 #define VMDK_GTE_ZEROED 0x1
+#define VMDK_GRANULARITY_MAX 0x100000 /* limit of cluster_sectors */
 
 /* VMDK internal error codes */
 #define VMDK_OK      0
diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
index 18807d3..f14c41b 100755
--- a/tests/qemu-iotests/059
+++ b/tests/qemu-iotests/059
@@ -43,7 +43,7 @@ _supported_fmt vmdk
 _supported_proto generic
 _supported_os Linux
 
-granularity_offset=16
+granularity_offset=20
 
 echo "=== Testing invalid granularity ==="
 echo
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index 4ca7f29..380ca3d 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -1,2 +1,8 @@
 QA output created by 059
+=== Testing invalid granularity ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+invalid granularity, image may be corrupt
+qemu-io: can't open device TEST_DIR/t.vmdk
+no file open, try 'help open'
 *** done
-- 
1.8.3.4

[Qemu-devel] [PATCH EMBARGOED 6/7] vmdk: check l2 table size when opening

[Fam Zheng]

header.num_gtes_per_gte determines size for L2 table. Check for too big
value before using it. Limit to 512M entries (2GB per one L2 table).

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/vmdk.c               | 5 +++++
 tests/qemu-iotests/059     | 7 +++++++
 tests/qemu-iotests/059.out | 6 ++++++
 3 files changed, 18 insertions(+)

diff --git a/block/vmdk.c b/block/vmdk.c
index 1d6bc39..e28bfc8 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -584,6 +584,11 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
         return -ENOTSUP;
     }
 
+    if (le64_to_cpu(header.num_gtes_per_gte) > 512 * 1024 * 1024) {
+        error_report("L2 table size too big");
+        return -EINVAL;
+    }
+
     l1_entry_sectors = le32_to_cpu(header.num_gtes_per_gte)
                         * le64_to_cpu(header.granularity);
     if (l1_entry_sectors == 0) {
diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
index f14c41b..aa9eb57 100755
--- a/tests/qemu-iotests/059
+++ b/tests/qemu-iotests/059
@@ -44,6 +44,7 @@ _supported_proto generic
 _supported_os Linux
 
 granularity_offset=20
+grain_table_size_offset=44
 
 echo "=== Testing invalid granularity ==="
 echo
@@ -51,6 +52,12 @@ _make_test_img 64M
 poke_file "$TEST_IMG" "$granularity_offset" "\xff\xff\xff\xff\xff\xff\xff\xff"
 $QEMU_IO -c "read 0 512" $TEST_IMG 2>&1 | _filter_qemu_io | _filter_testdir
 
+echo "=== Testing too big L2 table size ==="
+echo
+_make_test_img 64M
+poke_file "$TEST_IMG" "$grain_table_size_offset" "\xff\xff\xff\xff"
+$QEMU_IO -c "read 0 512" $TEST_IMG 2>&1 | _filter_qemu_io | _filter_testdir
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index 380ca3d..583955f 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -5,4 +5,10 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 invalid granularity, image may be corrupt
 qemu-io: can't open device TEST_DIR/t.vmdk
 no file open, try 'help open'
+=== Testing too big L2 table size ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+L2 table size too big
+qemu-io: can't open device TEST_DIR/t.vmdk
+no file open, try 'help open'
 *** done
-- 
1.8.3.4

[Qemu-devel] [PATCH EMBARGOED 7/7] vmdk: check l1 size before opening image

[Fam Zheng]

L1 table size is calculated from capacity, granularity and l2 table
size. If capacity is too big or later two are too small, the L1 table
will be too big to allocate in memory. Limit it to a reasonable range.

Signed-off-by: Fam Zheng <famz@redhat.com>
---
 block/vmdk.c               | 8 ++++++++
 tests/qemu-iotests/059     | 8 ++++++++
 tests/qemu-iotests/059.out | 6 ++++++
 3 files changed, 22 insertions(+)

diff --git a/block/vmdk.c b/block/vmdk.c
index e28bfc8..2114ef6 100644
--- a/block/vmdk.c
+++ b/block/vmdk.c
@@ -596,6 +596,14 @@ static int vmdk_open_vmdk4(BlockDriverState *bs,
     }
     l1_size = (le64_to_cpu(header.capacity) + l1_entry_sectors - 1)
                 / l1_entry_sectors;
+    if (l1_size > 512 * 1024 * 1024) {
+        /* although with big capacity and small l1_entry_sectors, we can get a
+         * big l1_size, we don't want unbounded value to allocate the table.
+         * Limit it to 512M, which is 16PB for default cluster and L2 table
+         * size */
+        error_report("L1 size too big");
+        return -EFBIG;
+    }
     if (le32_to_cpu(header.flags) & VMDK4_FLAG_RGD) {
         l1_backup_offset = le64_to_cpu(header.rgd_offset) << 9;
     }
diff --git a/tests/qemu-iotests/059 b/tests/qemu-iotests/059
index aa9eb57..e09e041 100755
--- a/tests/qemu-iotests/059
+++ b/tests/qemu-iotests/059
@@ -43,6 +43,7 @@ _supported_fmt vmdk
 _supported_proto generic
 _supported_os Linux
 
+capacity_offset=16
 granularity_offset=20
 grain_table_size_offset=44
 
@@ -58,6 +59,13 @@ _make_test_img 64M
 poke_file "$TEST_IMG" "$grain_table_size_offset" "\xff\xff\xff\xff"
 $QEMU_IO -c "read 0 512" $TEST_IMG 2>&1 | _filter_qemu_io | _filter_testdir
 
+echo "=== Testing too big L1 table size ==="
+echo
+_make_test_img 64M
+poke_file "$TEST_IMG" "$capacity_offset" "\xff\xff\xff\xff"
+poke_file "$TEST_IMG" "$grain_table_size_offset" "\x01\x00\x00\x00"
+$QEMU_IO -c "read 0 512" $TEST_IMG 2>&1 | _filter_qemu_io | _filter_testdir
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/059.out b/tests/qemu-iotests/059.out
index 583955f..9e715e5 100644
--- a/tests/qemu-iotests/059.out
+++ b/tests/qemu-iotests/059.out
@@ -11,4 +11,10 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 L2 table size too big
 qemu-io: can't open device TEST_DIR/t.vmdk
 no file open, try 'help open'
+=== Testing too big L1 table size ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
+L1 size too big
+qemu-io: can't open device TEST_DIR/t.vmdk
+no file open, try 'help open'
 *** done
-- 
1.8.3.4