[Qemu-devel] [PULL for-2.0 00/51] Block patches

[Qemu-devel] [PULL for-2.0 00/51] Block patches

[Stefan Hajnoczi]

This pull request for QEMU 2.0 contains the block layer CVE security fixes and
additional bug fixes by Markus and Prasad.

The following changes since commit 63678e17cf399ff81b93417fe7bee8d6ef6b6b1b:

  configure: add option to disable -fstack-protector flags (2014-03-31 20:16:02 +0100)

are available in the git repository at:

  git://github.com/stefanha/qemu.git tags/block-pull-request

for you to fetch changes up to c792707f54aa445cfb63a42411c66594b52b8f79:

  qcow2: link all L2 meta updates in preallocate() (2014-04-01 15:22:35 +0200)

----------------------------------------------------------------
Block pull request

----------------------------------------------------------------
Fam Zheng (1):
      curl: check data size before memcpy to local buffer. (CVE-2014-0144)

Jeff Cody (4):
      vpc/vhd: add bounds check for max_table_entries and block_size (CVE-2014-0144)
      vdi: add bounds checks for blocks_in_image and disk_size header fields (CVE-2014-0144)
      vhdx: Bounds checking for block_size and logical_sector_size (CVE-2014-0148)
      block: vdi bounds check qemu-io tests

Kevin Wolf (28):
      qemu-iotests: Support for bochs format
      bochs: Unify header structs and make them QEMU_PACKED
      bochs: Use unsigned variables for offsets and sizes (CVE-2014-0147)
      bochs: Check catalog_size header field (CVE-2014-0143)
      bochs: Check extent_size header field (CVE-2014-0142)
      bochs: Fix bitmap offset calculation
      vpc: Validate block size (CVE-2014-0142)
      qcow2: Check header_length (CVE-2014-0144)
      qcow2: Check backing_file_offset (CVE-2014-0144)
      qcow2: Check refcount table size (CVE-2014-0144)
      qcow2: Validate refcount table offset
      qcow2: Validate snapshot table offset/size (CVE-2014-0144)
      qcow2: Validate active L1 table offset and size (CVE-2014-0144)
      qcow2: Fix backing file name length check
      qcow2: Don't rely on free_cluster_index in alloc_refcount_block() (CVE-2014-0147)
      qcow2: Avoid integer overflow in get_refcount (CVE-2014-0143)
      qcow2: Check new refcount table size on growth
      qcow2: Fix types in qcow2_alloc_clusters and alloc_clusters_noref
      qcow2: Protect against some integer overflows in bdrv_check
      qcow2: Fix new L1 table size check (CVE-2014-0143)
      block: Limit request size (CVE-2014-0143)
      qcow2: Fix copy_sectors() with VM state
      qcow2: Fix NULL dereference in qcow2_open() error path (CVE-2014-0146)
      qcow2: Fix L1 allocation size in qcow2_snapshot_load_tmp() (CVE-2014-0145)
      qcow2: Check maximum L1 size in qcow2_snapshot_load_tmp() (CVE-2014-0143)
      qcow2: Limit snapshot table size
      parallels: Fix catalog size integer overflow (CVE-2014-0143)
      parallels: Sanity check for s->tracks (CVE-2014-0142)

Markus Armbruster (1):
      vvfat: Fix :floppy: option to suppress partition table

Prasad Joshi (2):
      qemu-img: Release reference to BlockDriverState
      qcow2: fix two memory leaks in qcow2_open error code path

Stefan Hajnoczi (15):
      qemu-iotests: add ./check -cloop support
      qemu-iotests: add cloop input validation tests
      block/cloop: validate block_size header field (CVE-2014-0144)
      block/cloop: prevent offsets_size integer overflow (CVE-2014-0143)
      block/cloop: refuse images with huge offsets arrays (CVE-2014-0144)
      block/cloop: refuse images with bogus offsets (CVE-2014-0144)
      block/cloop: fix offsets[] size off-by-one
      dmg: coding style and indentation cleanup
      dmg: prevent out-of-bounds array access on terminator
      dmg: drop broken bdrv_pread() loop
      dmg: use appropriate types when reading chunks
      dmg: sanitize chunk length and sectorcount (CVE-2014-0145)
      dmg: use uint64_t consistently for sectors and lengths
      dmg: prevent chunk buffer overflow (CVE-2014-0145)
      qcow2: link all L2 meta updates in preallocate()

 block.c                                            |   4 +
 block/bochs.c                                      | 109 ++++----
 block/cloop.c                                      |  81 +++++-
 block/curl.c                                       |   5 +
 block/dmg.c                                        | 275 +++++++++++++--------
 block/parallels.c                                  |  14 +-
 block/qcow2-cluster.c                              |  11 +-
 block/qcow2-refcount.c                             | 111 +++++----
 block/qcow2-snapshot.c                             |  50 ++--
 block/qcow2.c                                      | 145 +++++++++--
 block/qcow2.h                                      |  52 +++-
 block/vdi.c                                        |  37 ++-
 block/vhdx.c                                       |  12 +-
 block/vpc.c                                        |  32 ++-
 block/vvfat.c                                      |   2 +-
 qemu-img.c                                         |   1 +
 tests/qemu-iotests/026.out                         |   6 +-
 tests/qemu-iotests/029                             |  40 ++-
 tests/qemu-iotests/029.out                         |  17 ++
 tests/qemu-iotests/044.out                         |   2 +-
 tests/qemu-iotests/075                             | 106 ++++++++
 tests/qemu-iotests/075.out                         |  38 +++
 tests/qemu-iotests/076                             |  76 ++++++
 tests/qemu-iotests/076.out                         |  18 ++
 tests/qemu-iotests/078                             |  87 +++++++
 tests/qemu-iotests/078.out                         |  26 ++
 tests/qemu-iotests/080                             | 180 ++++++++++++++
 tests/qemu-iotests/080.out                         |  83 +++++++
 tests/qemu-iotests/084                             | 104 ++++++++
 tests/qemu-iotests/084.out                         |  33 +++
 tests/qemu-iotests/088                             |  64 +++++
 tests/qemu-iotests/088.out                         |  17 ++
 tests/qemu-iotests/common                          |  21 ++
 tests/qemu-iotests/common.rc                       |   3 +
 tests/qemu-iotests/group                           |   6 +
 tests/qemu-iotests/sample_images/empty.bochs.bz2   | Bin 0 -> 118 bytes
 .../qemu-iotests/sample_images/fake.parallels.bz2  | Bin 0 -> 141 bytes
 .../sample_images/simple-pattern.cloop.bz2         | Bin 0 -> 488 bytes
 38 files changed, 1565 insertions(+), 303 deletions(-)
 create mode 100755 tests/qemu-iotests/075
 create mode 100644 tests/qemu-iotests/075.out
 create mode 100755 tests/qemu-iotests/076
 create mode 100644 tests/qemu-iotests/076.out
 create mode 100755 tests/qemu-iotests/078
 create mode 100644 tests/qemu-iotests/078.out
 create mode 100755 tests/qemu-iotests/080
 create mode 100644 tests/qemu-iotests/080.out
 create mode 100755 tests/qemu-iotests/084
 create mode 100644 tests/qemu-iotests/084.out
 create mode 100755 tests/qemu-iotests/088
 create mode 100644 tests/qemu-iotests/088.out
 create mode 100644 tests/qemu-iotests/sample_images/empty.bochs.bz2
 create mode 100644 tests/qemu-iotests/sample_images/fake.parallels.bz2
 create mode 100644 tests/qemu-iotests/sample_images/simple-pattern.cloop.bz2

-- 
1.9.0

[Qemu-devel] [PULL for-2.0 01/51] qemu-img: Release reference to BlockDriverState

[Stefan Hajnoczi]

From: Prasad Joshi <prasadjoshi.linux@gmail.com>

Signed-off-by: Prasad Joshi <prasadjoshi.linux@gmail.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 qemu-img.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/qemu-img.c b/qemu-img.c
index 77d946b..8455994 100644
--- a/qemu-img.c
+++ b/qemu-img.c
@@ -1809,6 +1809,7 @@ static ImageInfoList *collect_image_info_list(const char *filename,
         if (err) {
             error_report("%s", error_get_pretty(err));
             error_free(err);
+            bdrv_unref(bs);
             goto err;
         }
 
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 02/51] vvfat: Fix :floppy: option to suppress partition table

[Stefan Hajnoczi]

From: Markus Armbruster <armbru@redhat.com>

Regressed in commit 7ad9be6, v1.5.0.

Reported-by: Kiyokazu SUTO <suto@ks-and-ks.ne.jp>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/vvfat.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/vvfat.c b/block/vvfat.c
index f966ea5..1978c9e 100644
--- a/block/vvfat.c
+++ b/block/vvfat.c
@@ -1119,6 +1119,7 @@ DLOG(if (stderr == NULL) {
         if (!s->fat_type) {
             s->fat_type = 16;
         }
+        s->first_sectors_number = 0x40;
         cyls = s->fat_type == 12 ? 64 : 1024;
         heads = 16;
         secs = 63;
@@ -1146,7 +1147,6 @@ DLOG(if (stderr == NULL) {
 
     s->current_cluster=0xffffffff;
 
-    s->first_sectors_number=0x40;
     /* read only is the default for safety */
     bs->read_only = 1;
     s->qcow = s->write_target = NULL;
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 03/51] qcow2: fix two memory leaks in qcow2_open error code path

[Stefan Hajnoczi]

From: Prasad Joshi <prasadjoshi.linux@gmail.com>

Signed-off-by: Prasad Joshi <prasadjoshi.linux@gmail.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index b9dc960..10eccf9 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -506,6 +506,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
                                    s->incompatible_features &
                                    ~QCOW2_INCOMPAT_MASK);
         ret = -ENOTSUP;
+        g_free(feature_table);
         goto fail;
     }
 
@@ -745,6 +746,9 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     if (s->l2_table_cache) {
         qcow2_cache_destroy(bs, s->l2_table_cache);
     }
+    if (s->refcount_block_cache) {
+        qcow2_cache_destroy(bs, s->refcount_block_cache);
+    }
     g_free(s->cluster_cache);
     qemu_vfree(s->cluster_data);
     return ret;
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 04/51] qemu-iotests: add ./check -cloop support

[Stefan Hajnoczi]

Add the cloop block driver to qemu-iotests.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qemu-iotests/common    | 7 +++++++
 tests/qemu-iotests/common.rc | 3 +++
 2 files changed, 10 insertions(+)

diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
index 5795358..37e3bed 100644
--- a/tests/qemu-iotests/common
+++ b/tests/qemu-iotests/common
@@ -137,6 +137,7 @@ common options
 check options
     -raw                test raw (default)
     -cow                test cow
+    -cloop              test cloop
     -qcow               test qcow
     -qcow2              test qcow2
     -qed                test qed
@@ -178,6 +179,12 @@ testlist options
             xpand=false
             ;;
 
+        -cloop)
+            IMGFMT=cloop
+            IMGFMT_GENERIC=false
+            xpand=false
+            ;;
+
         -qcow)
             IMGFMT=qcow
             xpand=false
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 881079b..7f00883 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -364,6 +364,9 @@ _fail()
 #
 _supported_fmt()
 {
+    # "generic" is suitable for most image formats. For some formats it doesn't
+    # work, however (most notably read-only formats), so they can opt out by
+    # setting IMGFMT_GENERIC to false.
     for f; do
         if [ "$f" = "$IMGFMT" -o "$f" = "generic" -a "$IMGFMT_GENERIC" = "true" ]; then
             return
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 05/51] qemu-iotests: add cloop input validation tests

[Stefan Hajnoczi]

Add a cloop format-specific test case.  Later patches add tests for
input validation to the script.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qemu-iotests/075                             |  53 +++++++++++++++++++++
 tests/qemu-iotests/075.out                         |   6 +++
 tests/qemu-iotests/group                           |   1 +
 .../sample_images/simple-pattern.cloop.bz2         | Bin 0 -> 488 bytes
 4 files changed, 60 insertions(+)
 create mode 100755 tests/qemu-iotests/075
 create mode 100644 tests/qemu-iotests/075.out
 create mode 100644 tests/qemu-iotests/sample_images/simple-pattern.cloop.bz2

diff --git a/tests/qemu-iotests/075 b/tests/qemu-iotests/075
new file mode 100755
index 0000000..88ae8bb
--- /dev/null
+++ b/tests/qemu-iotests/075
@@ -0,0 +1,53 @@
+#!/bin/bash
+#
+# cloop format input validation tests
+#
+# Copyright (C) 2013 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=stefanha@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
+
+_supported_fmt cloop
+_supported_proto generic
+_supported_os Linux
+
+echo
+echo "== check that the first sector can be read =="
+_use_sample_img simple-pattern.cloop.bz2
+$QEMU_IO -c "read 0 512" $TEST_IMG 2>&1 | _filter_qemu_io | _filter_testdir
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/075.out b/tests/qemu-iotests/075.out
new file mode 100644
index 0000000..26661fa
--- /dev/null
+++ b/tests/qemu-iotests/075.out
@@ -0,0 +1,6 @@
+QA output created by 075
+
+== check that the first sector can be read ==
+read 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index ee09ebc..633e82d 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -81,6 +81,7 @@
 072 rw auto quick
 073 rw auto quick
 074 rw auto quick
+075 rw auto
 077 rw auto quick
 079 rw auto
 081 rw auto
diff --git a/tests/qemu-iotests/sample_images/simple-pattern.cloop.bz2 b/tests/qemu-iotests/sample_images/simple-pattern.cloop.bz2
new file mode 100644
index 0000000000000000000000000000000000000000..a02d2ee4c710f48e9c0151dcb25e0637e2067b9f
GIT binary patch
literal 488
zcmV<E0T=#4T4*^jL0KkKS$!c^s{jDG|NsC0_gDW9_d2OS$xc6a-+JT#;L<<<!!DqR
zoPPjAw}UVNSb~P4B=ixqo{^dp)X?;vpbaz)0j7f>3?Lc+41uQ7dV@xWL&_k@fD-@#
zFaQ7m34&k(U;qh;fB*@Jhy<dkwE)NlO)>*Ug9HEo0004?8UO~34GlC628%pmbhgch
zQ?C-kdNsT#_wF0s)BwwA2FQRmQ2|gLC{;2<2TA~GsDT1j=>&+yJOz05+voN44`{1d
zzJBs>VEI%;odJ<DA}T7yR63R#n!O(yOaYZb`VA~7GZW~D#85TOm-_%kMu-q!)tKq^
zERY4;S{;XR6rY6v(*H)9meFSz6lBU&gr7oE(l~G$83sv%a#T}w6gocKLIN6ITT|q-
znK{@X-6$nAs>rKXRk|8ZS4K7%5gzkAQ>1{(br+n41iF_h7v?hIp07p=N-CUc+@4Yf
zQ&XeXEn0Pf;=C$r^FkBM%zW*-#l~jMVhBQ-L?JW`fA{X9W`T7LT<?gFTO&*76!XLa
z9snznf(FbsrS0b8Ua%J@ynLKYN{c5AAK4%jagW79I1Su6Yx32d*aU&vvR5)d8pvqU
eYDAm_3G6V!^Ib0@xa$@_#oUoj6eJ%=RjR-T$jlD_

literal 0
HcmV?d00001

-- 
1.9.0

[Qemu-devel] [PULL for-2.0 06/51] block/cloop: validate block_size header field (CVE-2014-0144)

[Stefan Hajnoczi]

Avoid unbounded s->uncompressed_block memory allocation by checking that
the block_size header field has a reasonable value.  Also enforce the
assumption that the value is a non-zero multiple of 512.

These constraints conform to cloop 2.639's code so we accept existing
image files.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/cloop.c              | 23 +++++++++++++++++++++++
 tests/qemu-iotests/075     | 20 ++++++++++++++++++++
 tests/qemu-iotests/075.out | 12 ++++++++++++
 3 files changed, 55 insertions(+)

diff --git a/block/cloop.c b/block/cloop.c
index b907023..f021663 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -26,6 +26,9 @@
 #include "qemu/module.h"
 #include <zlib.h>
 
+/* Maximum compressed block size */
+#define MAX_BLOCK_SIZE (64 * 1024 * 1024)
+
 typedef struct BDRVCloopState {
     CoMutex lock;
     uint32_t block_size;
@@ -68,6 +71,26 @@ static int cloop_open(BlockDriverState *bs, QDict *options, int flags,
         return ret;
     }
     s->block_size = be32_to_cpu(s->block_size);
+    if (s->block_size % 512) {
+        error_setg(errp, "block_size %u must be a multiple of 512",
+                   s->block_size);
+        return -EINVAL;
+    }
+    if (s->block_size == 0) {
+        error_setg(errp, "block_size cannot be zero");
+        return -EINVAL;
+    }
+
+    /* cloop's create_compressed_fs.c warns about block sizes beyond 256 KB but
+     * we can accept more.  Prevent ridiculous values like 4 GB - 1 since we
+     * need a buffer this big.
+     */
+    if (s->block_size > MAX_BLOCK_SIZE) {
+        error_setg(errp, "block_size %u must be %u MB or less",
+                   s->block_size,
+                   MAX_BLOCK_SIZE / (1024 * 1024));
+        return -EINVAL;
+    }
 
     ret = bdrv_pread(bs->file, 128 + 4, &s->n_blocks, 4);
     if (ret < 0) {
diff --git a/tests/qemu-iotests/075 b/tests/qemu-iotests/075
index 88ae8bb..8f54a99 100755
--- a/tests/qemu-iotests/075
+++ b/tests/qemu-iotests/075
@@ -42,11 +42,31 @@ _supported_fmt cloop
 _supported_proto generic
 _supported_os Linux
 
+block_size_offset=128
+
 echo
 echo "== check that the first sector can be read =="
 _use_sample_img simple-pattern.cloop.bz2
 $QEMU_IO -c "read 0 512" $TEST_IMG 2>&1 | _filter_qemu_io | _filter_testdir
 
+echo
+echo "== block_size must be a multiple of 512 =="
+_use_sample_img simple-pattern.cloop.bz2
+poke_file "$TEST_IMG" "$block_size_offset" "\x00\x00\x02\x01"
+$QEMU_IO -c "read 0 512" $TEST_IMG 2>&1 | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== block_size cannot be zero =="
+_use_sample_img simple-pattern.cloop.bz2
+poke_file "$TEST_IMG" "$block_size_offset" "\x00\x00\x00\x00"
+$QEMU_IO -c "read 0 512" $TEST_IMG 2>&1 | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== huge block_size ==="
+_use_sample_img simple-pattern.cloop.bz2
+poke_file "$TEST_IMG" "$block_size_offset" "\xff\xff\xfe\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/075.out b/tests/qemu-iotests/075.out
index 26661fa..d362c95 100644
--- a/tests/qemu-iotests/075.out
+++ b/tests/qemu-iotests/075.out
@@ -3,4 +3,16 @@ QA output created by 075
 == check that the first sector can be read ==
 read 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== block_size must be a multiple of 512 ==
+qemu-io: can't open device TEST_DIR/simple-pattern.cloop: block_size 513 must be a multiple of 512
+no file open, try 'help open'
+
+== block_size cannot be zero ==
+qemu-io: can't open device TEST_DIR/simple-pattern.cloop: block_size cannot be zero
+no file open, try 'help open'
+
+== huge block_size ===
+qemu-io: can't open device TEST_DIR/simple-pattern.cloop: block_size 4294966784 must be 64 MB or less
+no file open, try 'help open'
 *** done
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 07/51] block/cloop: prevent offsets_size integer overflow (CVE-2014-0143)

[Stefan Hajnoczi]

The following integer overflow in offsets_size can lead to out-of-bounds
memory stores when n_blocks has a huge value:

    uint32_t n_blocks, offsets_size;
    [...]
    ret = bdrv_pread(bs->file, 128 + 4, &s->n_blocks, 4);
    [...]
    s->n_blocks = be32_to_cpu(s->n_blocks);

    /* read offsets */
    offsets_size = s->n_blocks * sizeof(uint64_t);
    s->offsets = g_malloc(offsets_size);

    [...]

    for(i=0;i<s->n_blocks;i++) {
        s->offsets[i] = be64_to_cpu(s->offsets[i]);

offsets_size can be smaller than n_blocks due to integer overflow.
Therefore s->offsets[] is too small when the for loop byteswaps offsets.

This patch refuses to open files if offsets_size would overflow.

Note that changing the type of offsets_size is not a fix since 32-bit
hosts still only have 32-bit size_t.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/cloop.c              | 7 +++++++
 tests/qemu-iotests/075     | 7 +++++++
 tests/qemu-iotests/075.out | 4 ++++
 3 files changed, 18 insertions(+)

diff --git a/block/cloop.c b/block/cloop.c
index f021663..563e916 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -99,6 +99,13 @@ static int cloop_open(BlockDriverState *bs, QDict *options, int flags,
     s->n_blocks = be32_to_cpu(s->n_blocks);
 
     /* read offsets */
+    if (s->n_blocks > UINT32_MAX / sizeof(uint64_t)) {
+        /* Prevent integer overflow */
+        error_setg(errp, "n_blocks %u must be %zu or less",
+                   s->n_blocks,
+                   UINT32_MAX / sizeof(uint64_t));
+        return -EINVAL;
+    }
     offsets_size = s->n_blocks * sizeof(uint64_t);
     s->offsets = g_malloc(offsets_size);
 
diff --git a/tests/qemu-iotests/075 b/tests/qemu-iotests/075
index 8f54a99..9ce6b1f 100755
--- a/tests/qemu-iotests/075
+++ b/tests/qemu-iotests/075
@@ -43,6 +43,7 @@ _supported_proto generic
 _supported_os Linux
 
 block_size_offset=128
+n_blocks_offset=132
 
 echo
 echo "== check that the first sector can be read =="
@@ -67,6 +68,12 @@ _use_sample_img simple-pattern.cloop.bz2
 poke_file "$TEST_IMG" "$block_size_offset" "\xff\xff\xfe\x00"
 $QEMU_IO -c "read 0 512" $TEST_IMG 2>&1 | _filter_qemu_io | _filter_testdir
 
+echo
+echo "== offsets_size overflow ==="
+_use_sample_img simple-pattern.cloop.bz2
+poke_file "$TEST_IMG" "$n_blocks_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/075.out b/tests/qemu-iotests/075.out
index d362c95..a771789 100644
--- a/tests/qemu-iotests/075.out
+++ b/tests/qemu-iotests/075.out
@@ -15,4 +15,8 @@ no file open, try 'help open'
 == huge block_size ===
 qemu-io: can't open device TEST_DIR/simple-pattern.cloop: block_size 4294966784 must be 64 MB or less
 no file open, try 'help open'
+
+== offsets_size overflow ===
+qemu-io: can't open device TEST_DIR/simple-pattern.cloop: n_blocks 4294967295 must be 536870911 or less
+no file open, try 'help open'
 *** done
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 08/51] block/cloop: refuse images with huge offsets arrays (CVE-2014-0144)

[Stefan Hajnoczi]

Limit offsets_size to 512 MB so that:

1. g_malloc() does not abort due to an unreasonable size argument.

2. offsets_size does not overflow the bdrv_pread() int size argument.

This limit imposes a maximum image size of 16 TB at 256 KB block size.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/cloop.c              | 9 +++++++++
 tests/qemu-iotests/075     | 6 ++++++
 tests/qemu-iotests/075.out | 4 ++++
 3 files changed, 19 insertions(+)

diff --git a/block/cloop.c b/block/cloop.c
index 563e916..844665e 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -107,6 +107,15 @@ static int cloop_open(BlockDriverState *bs, QDict *options, int flags,
         return -EINVAL;
     }
     offsets_size = s->n_blocks * sizeof(uint64_t);
+    if (offsets_size > 512 * 1024 * 1024) {
+        /* Prevent ridiculous offsets_size which causes memory allocation to
+         * fail or overflows bdrv_pread() size.  In practice the 512 MB
+         * offsets[] limit supports 16 TB images at 256 KB block size.
+         */
+        error_setg(errp, "image requires too many offsets, "
+                   "try increasing block size");
+        return -EINVAL;
+    }
     s->offsets = g_malloc(offsets_size);
 
     ret = bdrv_pread(bs->file, 128 + 4 + 4, s->offsets, offsets_size);
diff --git a/tests/qemu-iotests/075 b/tests/qemu-iotests/075
index 9ce6b1f..9c00fa8 100755
--- a/tests/qemu-iotests/075
+++ b/tests/qemu-iotests/075
@@ -74,6 +74,12 @@ _use_sample_img simple-pattern.cloop.bz2
 poke_file "$TEST_IMG" "$n_blocks_offset" "\xff\xff\xff\xff"
 $QEMU_IO -c "read 0 512" $TEST_IMG 2>&1 | _filter_qemu_io | _filter_testdir
 
+echo
+echo "== refuse images that require too many offsets ==="
+_use_sample_img simple-pattern.cloop.bz2
+poke_file "$TEST_IMG" "$n_blocks_offset" "\x04\x00\x00\x01"
+$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/075.out b/tests/qemu-iotests/075.out
index a771789..7cdaee1 100644
--- a/tests/qemu-iotests/075.out
+++ b/tests/qemu-iotests/075.out
@@ -19,4 +19,8 @@ no file open, try 'help open'
 == offsets_size overflow ===
 qemu-io: can't open device TEST_DIR/simple-pattern.cloop: n_blocks 4294967295 must be 536870911 or less
 no file open, try 'help open'
+
+== refuse images that require too many offsets ===
+qemu-io: can't open device TEST_DIR/simple-pattern.cloop: image requires too many offsets, try increasing block size
+no file open, try 'help open'
 *** done
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 09/51] block/cloop: refuse images with bogus offsets (CVE-2014-0144)

[Stefan Hajnoczi]

The offsets[] array allows efficient seeking and tells us the maximum
compressed data size.  If the offsets are bogus the maximum compressed
data size will be unrealistic.

This could cause g_malloc() to abort and bogus offsets mean the image is
broken anyway.  Therefore we should refuse such images.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/cloop.c              | 34 +++++++++++++++++++++++++++++-----
 tests/qemu-iotests/075     | 15 +++++++++++++++
 tests/qemu-iotests/075.out |  8 ++++++++
 3 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/block/cloop.c b/block/cloop.c
index 844665e..55a804f 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -124,12 +124,36 @@ static int cloop_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     for(i=0;i<s->n_blocks;i++) {
+        uint64_t size;
+
         s->offsets[i] = be64_to_cpu(s->offsets[i]);
-        if (i > 0) {
-            uint32_t size = s->offsets[i] - s->offsets[i - 1];
-            if (size > max_compressed_block_size) {
-                max_compressed_block_size = size;
-            }
+        if (i == 0) {
+            continue;
+        }
+
+        if (s->offsets[i] < s->offsets[i - 1]) {
+            error_setg(errp, "offsets not monotonically increasing at "
+                       "index %u, image file is corrupt", i);
+            ret = -EINVAL;
+            goto fail;
+        }
+
+        size = s->offsets[i] - s->offsets[i - 1];
+
+        /* Compressed blocks should be smaller than the uncompressed block size
+         * but maybe compression performed poorly so the compressed block is
+         * actually bigger.  Clamp down on unrealistic values to prevent
+         * ridiculous s->compressed_block allocation.
+         */
+        if (size > 2 * MAX_BLOCK_SIZE) {
+            error_setg(errp, "invalid compressed block size at index %u, "
+                       "image file is corrupt", i);
+            ret = -EINVAL;
+            goto fail;
+        }
+
+        if (size > max_compressed_block_size) {
+            max_compressed_block_size = size;
         }
     }
 
diff --git a/tests/qemu-iotests/075 b/tests/qemu-iotests/075
index 9c00fa8..d74fb33 100755
--- a/tests/qemu-iotests/075
+++ b/tests/qemu-iotests/075
@@ -44,6 +44,7 @@ _supported_os Linux
 
 block_size_offset=128
 n_blocks_offset=132
+offsets_offset=136
 
 echo
 echo "== check that the first sector can be read =="
@@ -80,6 +81,20 @@ _use_sample_img simple-pattern.cloop.bz2
 poke_file "$TEST_IMG" "$n_blocks_offset" "\x04\x00\x00\x01"
 $QEMU_IO -c "read 0 512" $TEST_IMG 2>&1 | _filter_qemu_io | _filter_testdir
 
+echo
+echo "== refuse images with non-monotonically increasing offsets =="
+_use_sample_img simple-pattern.cloop.bz2
+poke_file "$TEST_IMG" "$offsets_offset" "\x00\x00\x00\x00\xff\xff\xff\xff"
+poke_file "$TEST_IMG" $((offsets_offset + 8)) "\x00\x00\x00\x00\xff\xfe\x00\x00"
+$QEMU_IO -c "read 0 512" $TEST_IMG 2>&1 | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== refuse images with invalid compressed block size =="
+_use_sample_img simple-pattern.cloop.bz2
+poke_file "$TEST_IMG" "$offsets_offset" "\x00\x00\x00\x00\x00\x00\x00\x00"
+poke_file "$TEST_IMG" $((offsets_offset + 8)) "\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
diff --git a/tests/qemu-iotests/075.out b/tests/qemu-iotests/075.out
index 7cdaee1..911cd3b 100644
--- a/tests/qemu-iotests/075.out
+++ b/tests/qemu-iotests/075.out
@@ -23,4 +23,12 @@ no file open, try 'help open'
 == refuse images that require too many offsets ===
 qemu-io: can't open device TEST_DIR/simple-pattern.cloop: image requires too many offsets, try increasing block size
 no file open, try 'help open'
+
+== refuse images with non-monotonically increasing offsets ==
+qemu-io: can't open device TEST_DIR/simple-pattern.cloop: offsets not monotonically increasing at index 1, image file is corrupt
+no file open, try 'help open'
+
+== refuse images with invalid compressed block size ==
+qemu-io: can't open device TEST_DIR/simple-pattern.cloop: invalid compressed block size at index 1, image file is corrupt
+no file open, try 'help open'
 *** done
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 10/51] block/cloop: fix offsets[] size off-by-one

[Stefan Hajnoczi]

cloop stores the number of compressed blocks in the n_blocks header
field.  The file actually contains n_blocks + 1 offsets, where the extra
offset is the end-of-file offset.

The following line in cloop_read_block() results in an out-of-bounds
offsets[] access:

    uint32_t bytes = s->offsets[block_num + 1] - s->offsets[block_num];

This patch allocates and loads the extra offset so that
cloop_read_block() works correctly when the last block is accessed.

Notice that we must free s->offsets[] unconditionally now since there is
always an end-of-file offset.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/cloop.c              | 12 +++++-------
 tests/qemu-iotests/075     |  5 +++++
 tests/qemu-iotests/075.out |  4 ++++
 3 files changed, 14 insertions(+), 7 deletions(-)

diff --git a/block/cloop.c b/block/cloop.c
index 55a804f..b6ad50f 100644
--- a/block/cloop.c
+++ b/block/cloop.c
@@ -99,14 +99,14 @@ static int cloop_open(BlockDriverState *bs, QDict *options, int flags,
     s->n_blocks = be32_to_cpu(s->n_blocks);
 
     /* read offsets */
-    if (s->n_blocks > UINT32_MAX / sizeof(uint64_t)) {
+    if (s->n_blocks > (UINT32_MAX - 1) / sizeof(uint64_t)) {
         /* Prevent integer overflow */
         error_setg(errp, "n_blocks %u must be %zu or less",
                    s->n_blocks,
-                   UINT32_MAX / sizeof(uint64_t));
+                   (UINT32_MAX - 1) / sizeof(uint64_t));
         return -EINVAL;
     }
-    offsets_size = s->n_blocks * sizeof(uint64_t);
+    offsets_size = (s->n_blocks + 1) * sizeof(uint64_t);
     if (offsets_size > 512 * 1024 * 1024) {
         /* Prevent ridiculous offsets_size which causes memory allocation to
          * fail or overflows bdrv_pread() size.  In practice the 512 MB
@@ -123,7 +123,7 @@ static int cloop_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
-    for(i=0;i<s->n_blocks;i++) {
+    for (i = 0; i < s->n_blocks + 1; i++) {
         uint64_t size;
 
         s->offsets[i] = be64_to_cpu(s->offsets[i]);
@@ -243,9 +243,7 @@ static coroutine_fn int cloop_co_read(BlockDriverState *bs, int64_t sector_num,
 static void cloop_close(BlockDriverState *bs)
 {
     BDRVCloopState *s = bs->opaque;
-    if (s->n_blocks > 0) {
-        g_free(s->offsets);
-    }
+    g_free(s->offsets);
     g_free(s->compressed_block);
     g_free(s->uncompressed_block);
     inflateEnd(&s->zstream);
diff --git a/tests/qemu-iotests/075 b/tests/qemu-iotests/075
index d74fb33..40032c5 100755
--- a/tests/qemu-iotests/075
+++ b/tests/qemu-iotests/075
@@ -52,6 +52,11 @@ _use_sample_img simple-pattern.cloop.bz2
 $QEMU_IO -c "read 0 512" $TEST_IMG 2>&1 | _filter_qemu_io | _filter_testdir
 
 echo
+echo "== check that the last sector can be read =="
+_use_sample_img simple-pattern.cloop.bz2
+$QEMU_IO -c "read $((1024 * 1024 - 512)) 512" $TEST_IMG 2>&1 | _filter_qemu_io | _filter_testdir
+
+echo
 echo "== block_size must be a multiple of 512 =="
 _use_sample_img simple-pattern.cloop.bz2
 poke_file "$TEST_IMG" "$block_size_offset" "\x00\x00\x02\x01"
diff --git a/tests/qemu-iotests/075.out b/tests/qemu-iotests/075.out
index 911cd3b..5f1d6c1 100644
--- a/tests/qemu-iotests/075.out
+++ b/tests/qemu-iotests/075.out
@@ -4,6 +4,10 @@ QA output created by 075
 read 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
+== check that the last sector can be read ==
+read 512/512 bytes at offset 1048064
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
 == block_size must be a multiple of 512 ==
 qemu-io: can't open device TEST_DIR/simple-pattern.cloop: block_size 513 must be a multiple of 512
 no file open, try 'help open'
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 11/51] qemu-iotests: Support for bochs format

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qemu-iotests/078                           |  53 +++++++++++++++++++++++
 tests/qemu-iotests/078.out                       |   6 +++
 tests/qemu-iotests/common                        |   7 +++
 tests/qemu-iotests/group                         |   1 +
 tests/qemu-iotests/sample_images/empty.bochs.bz2 | Bin 0 -> 118 bytes
 5 files changed, 67 insertions(+)
 create mode 100755 tests/qemu-iotests/078
 create mode 100644 tests/qemu-iotests/078.out
 create mode 100644 tests/qemu-iotests/sample_images/empty.bochs.bz2

diff --git a/tests/qemu-iotests/078 b/tests/qemu-iotests/078
new file mode 100755
index 0000000..f55f46d
--- /dev/null
+++ b/tests/qemu-iotests/078
@@ -0,0 +1,53 @@
+#!/bin/bash
+#
+# bochs format input validation tests
+#
+# Copyright (C) 2013 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=kwolf@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
+
+_supported_fmt bochs
+_supported_proto generic
+_supported_os Linux
+
+echo
+echo "== Read from a valid image =="
+_use_sample_img empty.bochs.bz2
+{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/078.out b/tests/qemu-iotests/078.out
new file mode 100644
index 0000000..25d37c5
--- /dev/null
+++ b/tests/qemu-iotests/078.out
@@ -0,0 +1,6 @@
+QA output created by 078
+
+== Read from a valid image ==
+read 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+*** done
diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
index 37e3bed..a09d9c8 100644
--- a/tests/qemu-iotests/common
+++ b/tests/qemu-iotests/common
@@ -136,6 +136,7 @@ common options
 
 check options
     -raw                test raw (default)
+    -bochs              test bochs
     -cow                test cow
     -cloop              test cloop
     -qcow               test qcow
@@ -174,6 +175,12 @@ testlist options
             xpand=false
             ;;
 
+        -bochs)
+            IMGFMT=bochs
+            IMGFMT_GENERIC=false
+            xpand=false
+            ;;
+
         -cow)
             IMGFMT=cow
             xpand=false
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 633e82d..ecba432 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -83,6 +83,7 @@
 074 rw auto quick
 075 rw auto
 077 rw auto quick
+078 rw auto
 079 rw auto
 081 rw auto
 082 rw auto quick
diff --git a/tests/qemu-iotests/sample_images/empty.bochs.bz2 b/tests/qemu-iotests/sample_images/empty.bochs.bz2
new file mode 100644
index 0000000000000000000000000000000000000000..7a29c6ed763407f2de067d2618e6a60fb23812b8
GIT binary patch
literal 118
zcmZ>Y%CIzaj8qGbEHvXuW?+ykpYp%q0D}XA$OAzJz31~91b}P?js*-MrV1$+l1$i~
z-4}9X&0;tqx7fiX<AheW%Z6AkV-}vCX-a&`bK=V?6cx(PyWI7EdEsra!@~6!wUt9A
V{S18A!=S+OU%bfGB47f@9suImDUARC

literal 0
HcmV?d00001

-- 
1.9.0

[Qemu-devel] [PULL for-2.0 12/51] bochs: Unify header structs and make them QEMU_PACKED

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

This is an on-disk structure, so offsets must be accurate.

Before this patch, sizeof(bochs) != sizeof(header_v1), which makes the
memcpy() between both invalid. We're lucky enough that the destination
buffer happened to be the larger one, and the memcpy size to be taken
from the smaller one, so we didn't get a buffer overflow in practice.

This patch unifies the both structures, eliminating the need to do a
memcpy in the first place. The common fields are extracted to the top
level of the struct and the actually differing part gets a union of the
two versions.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/bochs.c | 67 ++++++++++++++++++++++-------------------------------------
 1 file changed, 25 insertions(+), 42 deletions(-)

diff --git a/block/bochs.c b/block/bochs.c
index 4d6403f..ef8e381 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -39,45 +39,30 @@
 // not allocated: 0xffffffff
 
 // always little-endian
-struct bochs_header_v1 {
-    char magic[32]; // "Bochs Virtual HD Image"
-    char type[16]; // "Redolog"
-    char subtype[16]; // "Undoable" / "Volatile" / "Growing"
-    uint32_t version;
-    uint32_t header; // size of header
-
-    union {
-	struct {
-	    uint32_t catalog; // num of entries
-	    uint32_t bitmap; // bitmap size
-	    uint32_t extent; // extent size
-	    uint64_t disk; // disk size
-	    char padding[HEADER_SIZE - 64 - 8 - 20];
-	} redolog;
-	char padding[HEADER_SIZE - 64 - 8];
-    } extra;
-};
-
-// always little-endian
 struct bochs_header {
-    char magic[32]; // "Bochs Virtual HD Image"
-    char type[16]; // "Redolog"
-    char subtype[16]; // "Undoable" / "Volatile" / "Growing"
+    char magic[32];     /* "Bochs Virtual HD Image" */
+    char type[16];      /* "Redolog" */
+    char subtype[16];   /* "Undoable" / "Volatile" / "Growing" */
     uint32_t version;
-    uint32_t header; // size of header
+    uint32_t header;    /* size of header */
+
+    uint32_t catalog;   /* num of entries */
+    uint32_t bitmap;    /* bitmap size */
+    uint32_t extent;    /* extent size */
 
     union {
-	struct {
-	    uint32_t catalog; // num of entries
-	    uint32_t bitmap; // bitmap size
-	    uint32_t extent; // extent size
-	    uint32_t reserved; // for ???
-	    uint64_t disk; // disk size
-	    char padding[HEADER_SIZE - 64 - 8 - 24];
-	} redolog;
-	char padding[HEADER_SIZE - 64 - 8];
+        struct {
+            uint32_t reserved;  /* for ??? */
+            uint64_t disk;      /* disk size */
+            char padding[HEADER_SIZE - 64 - 20 - 12];
+        } QEMU_PACKED redolog;
+        struct {
+            uint64_t disk;      /* disk size */
+            char padding[HEADER_SIZE - 64 - 20 - 8];
+        } QEMU_PACKED redolog_v1;
+        char padding[HEADER_SIZE - 64 - 20];
     } extra;
-};
+} QEMU_PACKED;
 
 typedef struct BDRVBochsState {
     CoMutex lock;
@@ -114,7 +99,6 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
     BDRVBochsState *s = bs->opaque;
     int i;
     struct bochs_header bochs;
-    struct bochs_header_v1 header_v1;
     int ret;
 
     bs->read_only = 1; // no write support yet
@@ -134,13 +118,12 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     if (le32_to_cpu(bochs.version) == HEADER_V1) {
-      memcpy(&header_v1, &bochs, sizeof(bochs));
-      bs->total_sectors = le64_to_cpu(header_v1.extra.redolog.disk) / 512;
+        bs->total_sectors = le64_to_cpu(bochs.extra.redolog_v1.disk) / 512;
     } else {
-      bs->total_sectors = le64_to_cpu(bochs.extra.redolog.disk) / 512;
+        bs->total_sectors = le64_to_cpu(bochs.extra.redolog.disk) / 512;
     }
 
-    s->catalog_size = le32_to_cpu(bochs.extra.redolog.catalog);
+    s->catalog_size = le32_to_cpu(bochs.catalog);
     s->catalog_bitmap = g_malloc(s->catalog_size * 4);
 
     ret = bdrv_pread(bs->file, le32_to_cpu(bochs.header), s->catalog_bitmap,
@@ -154,10 +137,10 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
 
     s->data_offset = le32_to_cpu(bochs.header) + (s->catalog_size * 4);
 
-    s->bitmap_blocks = 1 + (le32_to_cpu(bochs.extra.redolog.bitmap) - 1) / 512;
-    s->extent_blocks = 1 + (le32_to_cpu(bochs.extra.redolog.extent) - 1) / 512;
+    s->bitmap_blocks = 1 + (le32_to_cpu(bochs.bitmap) - 1) / 512;
+    s->extent_blocks = 1 + (le32_to_cpu(bochs.extent) - 1) / 512;
 
-    s->extent_size = le32_to_cpu(bochs.extra.redolog.extent);
+    s->extent_size = le32_to_cpu(bochs.extent);
 
     qemu_co_mutex_init(&s->lock);
     return 0;
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 13/51] bochs: Use unsigned variables for offsets and sizes (CVE-2014-0147)

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

Gets us rid of integer overflows resulting in negative sizes which
aren't correctly checked.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/bochs.c              | 16 ++++++++--------
 tests/qemu-iotests/078     |  8 ++++++++
 tests/qemu-iotests/078.out |  4 ++++
 3 files changed, 20 insertions(+), 8 deletions(-)

diff --git a/block/bochs.c b/block/bochs.c
index ef8e381..e923eed 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -67,13 +67,13 @@ struct bochs_header {
 typedef struct BDRVBochsState {
     CoMutex lock;
     uint32_t *catalog_bitmap;
-    int catalog_size;
+    uint32_t catalog_size;
 
-    int data_offset;
+    uint32_t data_offset;
 
-    int bitmap_blocks;
-    int extent_blocks;
-    int extent_size;
+    uint32_t bitmap_blocks;
+    uint32_t extent_blocks;
+    uint32_t extent_size;
 } BDRVBochsState;
 
 static int bochs_probe(const uint8_t *buf, int buf_size, const char *filename)
@@ -97,7 +97,7 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
                       Error **errp)
 {
     BDRVBochsState *s = bs->opaque;
-    int i;
+    uint32_t i;
     struct bochs_header bochs;
     int ret;
 
@@ -153,8 +153,8 @@ fail:
 static int64_t seek_to_sector(BlockDriverState *bs, int64_t sector_num)
 {
     BDRVBochsState *s = bs->opaque;
-    int64_t offset = sector_num * 512;
-    int64_t extent_index, extent_offset, bitmap_offset;
+    uint64_t offset = sector_num * 512;
+    uint64_t extent_index, extent_offset, bitmap_offset;
     char bitmap_entry;
 
     // seek to sector
diff --git a/tests/qemu-iotests/078 b/tests/qemu-iotests/078
index f55f46d..73b573a 100755
--- a/tests/qemu-iotests/078
+++ b/tests/qemu-iotests/078
@@ -42,11 +42,19 @@ _supported_fmt bochs
 _supported_proto generic
 _supported_os Linux
 
+catalog_size_offset=$((0x48))
+
 echo
 echo "== Read from a valid image =="
 _use_sample_img empty.bochs.bz2
 { $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
 
+echo
+echo "== Negative catalog size =="
+_use_sample_img empty.bochs.bz2
+poke_file "$TEST_IMG" "$catalog_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/078.out b/tests/qemu-iotests/078.out
index 25d37c5..ef8c42d 100644
--- a/tests/qemu-iotests/078.out
+++ b/tests/qemu-iotests/078.out
@@ -3,4 +3,8 @@ QA output created by 078
 == Read from a valid image ==
 read 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== Negative catalog size ==
+qemu-io: can't open device TEST_DIR/empty.bochs: Could not open 'TEST_DIR/empty.bochs': Interrupted system call
+no file open, try 'help open'
 *** done
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 14/51] bochs: Check catalog_size header field (CVE-2014-0143)

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

It should neither become negative nor allow unbounded memory
allocations. This fixes aborts in g_malloc() and an s->catalog_bitmap
buffer overflow on big endian hosts.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/bochs.c              | 13 +++++++++++++
 tests/qemu-iotests/078     | 13 +++++++++++++
 tests/qemu-iotests/078.out | 10 +++++++++-
 3 files changed, 35 insertions(+), 1 deletion(-)

diff --git a/block/bochs.c b/block/bochs.c
index e923eed..0ffa9c1 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -123,7 +123,14 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
         bs->total_sectors = le64_to_cpu(bochs.extra.redolog.disk) / 512;
     }
 
+    /* Limit to 1M entries to avoid unbounded allocation. This is what is
+     * needed for the largest image that bximage can create (~8 TB). */
     s->catalog_size = le32_to_cpu(bochs.catalog);
+    if (s->catalog_size > 0x100000) {
+        error_setg(errp, "Catalog size is too large");
+        return -EFBIG;
+    }
+
     s->catalog_bitmap = g_malloc(s->catalog_size * 4);
 
     ret = bdrv_pread(bs->file, le32_to_cpu(bochs.header), s->catalog_bitmap,
@@ -142,6 +149,12 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
 
     s->extent_size = le32_to_cpu(bochs.extent);
 
+    if (s->catalog_size < bs->total_sectors / s->extent_size) {
+        error_setg(errp, "Catalog size is too small for this disk size");
+        ret = -EINVAL;
+        goto fail;
+    }
+
     qemu_co_mutex_init(&s->lock);
     return 0;
 
diff --git a/tests/qemu-iotests/078 b/tests/qemu-iotests/078
index 73b573a..902ef0f 100755
--- a/tests/qemu-iotests/078
+++ b/tests/qemu-iotests/078
@@ -43,6 +43,7 @@ _supported_proto generic
 _supported_os Linux
 
 catalog_size_offset=$((0x48))
+disk_size_offset=$((0x58))
 
 echo
 echo "== Read from a valid image =="
@@ -55,6 +56,18 @@ _use_sample_img empty.bochs.bz2
 poke_file "$TEST_IMG" "$catalog_size_offset" "\xff\xff\xff\xff"
 { $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
 
+echo
+echo "== Overflow for catalog size * sizeof(uint32_t) =="
+_use_sample_img empty.bochs.bz2
+poke_file "$TEST_IMG" "$catalog_size_offset" "\x00\x00\x00\x40"
+{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== Too small catalog bitmap for image size =="
+_use_sample_img empty.bochs.bz2
+poke_file "$TEST_IMG" "$disk_size_offset" "\x00\xc0\x0f\x00\x00\x00\x00\x7f"
+{ $QEMU_IO -c "read 2T 4k" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/078.out b/tests/qemu-iotests/078.out
index ef8c42d..7254693 100644
--- a/tests/qemu-iotests/078.out
+++ b/tests/qemu-iotests/078.out
@@ -5,6 +5,14 @@ read 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 
 == Negative catalog size ==
-qemu-io: can't open device TEST_DIR/empty.bochs: Could not open 'TEST_DIR/empty.bochs': Interrupted system call
+qemu-io: can't open device TEST_DIR/empty.bochs: Catalog size is too large
+no file open, try 'help open'
+
+== Overflow for catalog size * sizeof(uint32_t) ==
+qemu-io: can't open device TEST_DIR/empty.bochs: Catalog size is too large
+no file open, try 'help open'
+
+== Too small catalog bitmap for image size ==
+qemu-io: can't open device TEST_DIR/empty.bochs: Catalog size is too small for this disk size
 no file open, try 'help open'
 *** done
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 15/51] bochs: Check extent_size header field (CVE-2014-0142)

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

This fixes two possible division by zero crashes: In bochs_open() and in
seek_to_sector().

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/bochs.c              |  8 ++++++++
 tests/qemu-iotests/078     | 13 +++++++++++++
 tests/qemu-iotests/078.out |  8 ++++++++
 3 files changed, 29 insertions(+)

diff --git a/block/bochs.c b/block/bochs.c
index 0ffa9c1..a922782 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -148,6 +148,14 @@ static int bochs_open(BlockDriverState *bs, QDict *options, int flags,
     s->extent_blocks = 1 + (le32_to_cpu(bochs.extent) - 1) / 512;
 
     s->extent_size = le32_to_cpu(bochs.extent);
+    if (s->extent_size == 0) {
+        error_setg(errp, "Extent size may not be zero");
+        return -EINVAL;
+    } else if (s->extent_size > 0x800000) {
+        error_setg(errp, "Extent size %" PRIu32 " is too large",
+                   s->extent_size);
+        return -EINVAL;
+    }
 
     if (s->catalog_size < bs->total_sectors / s->extent_size) {
         error_setg(errp, "Catalog size is too small for this disk size");
diff --git a/tests/qemu-iotests/078 b/tests/qemu-iotests/078
index 902ef0f..872e734 100755
--- a/tests/qemu-iotests/078
+++ b/tests/qemu-iotests/078
@@ -43,6 +43,7 @@ _supported_proto generic
 _supported_os Linux
 
 catalog_size_offset=$((0x48))
+extent_size_offset=$((0x50))
 disk_size_offset=$((0x58))
 
 echo
@@ -68,6 +69,18 @@ _use_sample_img empty.bochs.bz2
 poke_file "$TEST_IMG" "$disk_size_offset" "\x00\xc0\x0f\x00\x00\x00\x00\x7f"
 { $QEMU_IO -c "read 2T 4k" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
 
+echo
+echo "== Negative extent size =="
+_use_sample_img empty.bochs.bz2
+poke_file "$TEST_IMG" "$extent_size_offset" "\xff\xff\xff\xff"
+{ $QEMU_IO -c "read 768k 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== Zero extent size =="
+_use_sample_img empty.bochs.bz2
+poke_file "$TEST_IMG" "$extent_size_offset" "\x00\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/078.out b/tests/qemu-iotests/078.out
index 7254693..ea95ffd 100644
--- a/tests/qemu-iotests/078.out
+++ b/tests/qemu-iotests/078.out
@@ -15,4 +15,12 @@ no file open, try 'help open'
 == Too small catalog bitmap for image size ==
 qemu-io: can't open device TEST_DIR/empty.bochs: Catalog size is too small for this disk size
 no file open, try 'help open'
+
+== Negative extent size ==
+qemu-io: can't open device TEST_DIR/empty.bochs: Extent size 4294967295 is too large
+no file open, try 'help open'
+
+== Zero extent size ==
+qemu-io: can't open device TEST_DIR/empty.bochs: Extent size may not be zero
+no file open, try 'help open'
 *** done
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 16/51] bochs: Fix bitmap offset calculation

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

32 bit truncation could let us access the wrong offset in the image.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/bochs.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/block/bochs.c b/block/bochs.c
index a922782..826ec12 100644
--- a/block/bochs.c
+++ b/block/bochs.c
@@ -186,8 +186,9 @@ static int64_t seek_to_sector(BlockDriverState *bs, int64_t sector_num)
 	return -1; /* not allocated */
     }
 
-    bitmap_offset = s->data_offset + (512 * s->catalog_bitmap[extent_index] *
-	(s->extent_blocks + s->bitmap_blocks));
+    bitmap_offset = s->data_offset +
+        (512 * (uint64_t) s->catalog_bitmap[extent_index] *
+        (s->extent_blocks + s->bitmap_blocks));
 
     /* read in bitmap for current extent */
     if (bdrv_pread(bs->file, bitmap_offset + (extent_offset / 8),
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 17/51] vpc/vhd: add bounds check for max_table_entries and block_size (CVE-2014-0144)

[Stefan Hajnoczi]

From: Jeff Cody <jcody@redhat.com>

This adds checks to make sure that max_table_entries and block_size
are in sane ranges.  Memory is allocated based on max_table_entries,
and block_size is used to calculate indices into that allocated
memory, so if these values are incorrect that can lead to potential
unbounded memory allocation, or invalid memory accesses.

Also, the allocation of the pagetable is changed from g_malloc0()
to qemu_blockalign().

Signed-off-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/vpc.c | 27 +++++++++++++++++++++++----
 1 file changed, 23 insertions(+), 4 deletions(-)

diff --git a/block/vpc.c b/block/vpc.c
index 82bf248..ba82d48 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -45,6 +45,8 @@ enum vhd_type {
 // Seconds since Jan 1, 2000 0:00:00 (UTC)
 #define VHD_TIMESTAMP_BASE 946684800
 
+#define VHD_MAX_SECTORS       (65535LL * 255 * 255)
+
 // always big-endian
 typedef struct vhd_footer {
     char        creator[8]; // "conectix"
@@ -164,6 +166,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
     VHDDynDiskHeader *dyndisk_header;
     uint8_t buf[HEADER_SIZE];
     uint32_t checksum;
+    uint64_t computed_size;
     int disk_type = VHD_DYNAMIC;
     int ret;
 
@@ -222,7 +225,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     /* Allow a maximum disk size of approximately 2 TB */
-    if (bs->total_sectors >= 65535LL * 255 * 255) {
+    if (bs->total_sectors >= VHD_MAX_SECTORS) {
         ret = -EFBIG;
         goto fail;
     }
@@ -245,7 +248,23 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
         s->bitmap_size = ((s->block_size / (8 * 512)) + 511) & ~511;
 
         s->max_table_entries = be32_to_cpu(dyndisk_header->max_table_entries);
-        s->pagetable = g_malloc(s->max_table_entries * 4);
+
+        if ((bs->total_sectors * 512) / s->block_size > 0xffffffffU) {
+            ret = -EINVAL;
+            goto fail;
+        }
+        if (s->max_table_entries > (VHD_MAX_SECTORS * 512) / s->block_size) {
+            ret = -EINVAL;
+            goto fail;
+        }
+
+        computed_size = (uint64_t) s->max_table_entries * s->block_size;
+        if (computed_size < bs->total_sectors * 512) {
+            ret = -EINVAL;
+            goto fail;
+        }
+
+        s->pagetable = qemu_blockalign(bs, s->max_table_entries * 4);
 
         s->bat_offset = be64_to_cpu(dyndisk_header->table_offset);
 
@@ -298,7 +317,7 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
     return 0;
 
 fail:
-    g_free(s->pagetable);
+    qemu_vfree(s->pagetable);
 #ifdef CACHE
     g_free(s->pageentry_u8);
 #endif
@@ -833,7 +852,7 @@ static int vpc_has_zero_init(BlockDriverState *bs)
 static void vpc_close(BlockDriverState *bs)
 {
     BDRVVPCState *s = bs->opaque;
-    g_free(s->pagetable);
+    qemu_vfree(s->pagetable);
 #ifdef CACHE
     g_free(s->pageentry_u8);
 #endif
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 18/51] vpc: Validate block size (CVE-2014-0142)

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

This fixes some cases of division by zero crashes.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/vpc.c                |  5 ++++
 tests/qemu-iotests/088     | 64 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/088.out | 17 ++++++++++++
 tests/qemu-iotests/group   |  1 +
 4 files changed, 87 insertions(+)
 create mode 100755 tests/qemu-iotests/088
 create mode 100644 tests/qemu-iotests/088.out

diff --git a/block/vpc.c b/block/vpc.c
index ba82d48..2e25f57 100644
--- a/block/vpc.c
+++ b/block/vpc.c
@@ -245,6 +245,11 @@ static int vpc_open(BlockDriverState *bs, QDict *options, int flags,
         }
 
         s->block_size = be32_to_cpu(dyndisk_header->block_size);
+        if (!is_power_of_2(s->block_size) || s->block_size < BDRV_SECTOR_SIZE) {
+            error_setg(errp, "Invalid block size %" PRIu32, s->block_size);
+            ret = -EINVAL;
+            goto fail;
+        }
         s->bitmap_size = ((s->block_size / (8 * 512)) + 511) & ~511;
 
         s->max_table_entries = be32_to_cpu(dyndisk_header->max_table_entries);
diff --git a/tests/qemu-iotests/088 b/tests/qemu-iotests/088
new file mode 100755
index 0000000..c09adf8
--- /dev/null
+++ b/tests/qemu-iotests/088
@@ -0,0 +1,64 @@
+#!/bin/bash
+#
+# vpc (VHD) format input validation tests
+#
+# Copyright (C) 2014 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=kwolf@redhat.com
+
+seq=`basename $0`
+echo "QA output created by $seq"
+
+here=`pwd`
+tmp=/tmp/$$
+status=1	# failure is the default!
+
+_cleanup()
+{
+    rm -f $TEST_IMG.snap
+    _cleanup_test_img
+}
+trap "_cleanup; exit \$status" 0 1 2 3 15
+
+# get standard environment, filters and checks
+. ./common.rc
+. ./common.filter
+
+_supported_fmt vpc
+_supported_proto generic
+_supported_os Linux
+
+offset_block_size=$((512 + 32))
+
+echo
+echo "== Invalid block size =="
+_make_test_img 64M
+poke_file "$TEST_IMG" "$offset_block_size" "\x00\x00\x00\x00"
+{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+{ $QEMU_IO -c "write 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+poke_file "$TEST_IMG" "$offset_block_size" "\x00\x00\x00\x80"
+{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+{ $QEMU_IO -c "write 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+poke_file "$TEST_IMG" "$offset_block_size" "\x12\x34\x56\x78"
+{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+{ $QEMU_IO -c "write 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/088.out b/tests/qemu-iotests/088.out
new file mode 100644
index 0000000..d961609
--- /dev/null
+++ b/tests/qemu-iotests/088.out
@@ -0,0 +1,17 @@
+QA output created by 088
+
+== Invalid block size ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+qemu-io: can't open device TEST_DIR/t.vpc: Invalid block size 0
+no file open, try 'help open'
+qemu-io: can't open device TEST_DIR/t.vpc: Invalid block size 0
+no file open, try 'help open'
+qemu-io: can't open device TEST_DIR/t.vpc: Invalid block size 128
+no file open, try 'help open'
+qemu-io: can't open device TEST_DIR/t.vpc: Invalid block size 128
+no file open, try 'help open'
+qemu-io: can't open device TEST_DIR/t.vpc: Invalid block size 305419896
+no file open, try 'help open'
+qemu-io: can't open device TEST_DIR/t.vpc: Invalid block size 305419896
+no file open, try 'help open'
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index ecba432..9c99edc 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -91,3 +91,4 @@
 085 rw auto
 086 rw auto quick
 087 rw auto
+088 rw auto
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 19/51] vdi: add bounds checks for blocks_in_image and disk_size header fields (CVE-2014-0144)

[Stefan Hajnoczi]

From: Jeff Cody <jcody@redhat.com>

The maximum blocks_in_image is 0xffffffff / 4, which also limits the
maximum disk_size for a VDI image to 1024TB.  Note that this is the maximum
size that QEMU will currently support with this driver, not necessarily the
maximum size allowed by the image format.

This also fixes an incorrect error message, a bug introduced by commit
5b7aa9b56d1bfc79916262f380c3fc7961becb50 (Reported by Stefan Weil)

Signed-off-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/vdi.c | 37 +++++++++++++++++++++++++++++++++----
 1 file changed, 33 insertions(+), 4 deletions(-)

diff --git a/block/vdi.c b/block/vdi.c
index ac9a025..820cd37 100644
--- a/block/vdi.c
+++ b/block/vdi.c
@@ -120,6 +120,11 @@ typedef unsigned char uuid_t[16];
 
 #define VDI_IS_ALLOCATED(X) ((X) < VDI_DISCARDED)
 
+/* max blocks in image is (0xffffffff / 4) */
+#define VDI_BLOCKS_IN_IMAGE_MAX  0x3fffffff
+#define VDI_DISK_SIZE_MAX        ((uint64_t)VDI_BLOCKS_IN_IMAGE_MAX * \
+                                  (uint64_t)DEFAULT_CLUSTER_SIZE)
+
 #if !defined(CONFIG_UUID)
 static inline void uuid_generate(uuid_t out)
 {
@@ -385,6 +390,14 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
     vdi_header_print(&header);
 #endif
 
+    if (header.disk_size > VDI_DISK_SIZE_MAX) {
+        error_setg(errp, "Unsupported VDI image size (size is 0x%" PRIx64
+                          ", max supported is 0x%" PRIx64 ")",
+                          header.disk_size, VDI_DISK_SIZE_MAX);
+        ret = -ENOTSUP;
+        goto fail;
+    }
+
     if (header.disk_size % SECTOR_SIZE != 0) {
         /* 'VBoxManage convertfromraw' can create images with odd disk sizes.
            We accept them but round the disk size to the next multiple of
@@ -420,9 +433,9 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
                    header.sector_size, SECTOR_SIZE);
         ret = -ENOTSUP;
         goto fail;
-    } else if (header.block_size != 1 * MiB) {
-        error_setg(errp, "unsupported VDI image (sector size %u is not %u)",
-                   header.block_size, 1 * MiB);
+    } else if (header.block_size != DEFAULT_CLUSTER_SIZE) {
+        error_setg(errp, "unsupported VDI image (block size %u is not %u)",
+                   header.block_size, DEFAULT_CLUSTER_SIZE);
         ret = -ENOTSUP;
         goto fail;
     } else if (header.disk_size >
@@ -441,6 +454,12 @@ static int vdi_open(BlockDriverState *bs, QDict *options, int flags,
         error_setg(errp, "unsupported VDI image (non-NULL parent UUID)");
         ret = -ENOTSUP;
         goto fail;
+    } else if (header.blocks_in_image > VDI_BLOCKS_IN_IMAGE_MAX) {
+        error_setg(errp, "unsupported VDI image "
+                         "(too many blocks %u, max is %u)",
+                          header.blocks_in_image, VDI_BLOCKS_IN_IMAGE_MAX);
+        ret = -ENOTSUP;
+        goto fail;
     }
 
     bs->total_sectors = header.disk_size / SECTOR_SIZE;
@@ -689,11 +708,20 @@ static int vdi_create(const char *filename, QEMUOptionParameter *options,
         options++;
     }
 
+    if (bytes > VDI_DISK_SIZE_MAX) {
+        result = -ENOTSUP;
+        error_setg(errp, "Unsupported VDI image size (size is 0x%" PRIx64
+                          ", max supported is 0x%" PRIx64 ")",
+                          bytes, VDI_DISK_SIZE_MAX);
+        goto exit;
+    }
+
     fd = qemu_open(filename,
                    O_WRONLY | O_CREAT | O_TRUNC | O_BINARY | O_LARGEFILE,
                    0644);
     if (fd < 0) {
-        return -errno;
+        result = -errno;
+        goto exit;
     }
 
     /* We need enough blocks to store the given disk size,
@@ -754,6 +782,7 @@ static int vdi_create(const char *filename, QEMUOptionParameter *options,
         result = -errno;
     }
 
+exit:
     return result;
 }
 
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 20/51] vhdx: Bounds checking for block_size and logical_sector_size (CVE-2014-0148)

[Stefan Hajnoczi]

From: Jeff Cody <jcody@redhat.com>

Other variables (e.g. sectors_per_block) are calculated using these
variables, and if not range-checked illegal values could be obtained
causing infinite loops and other potential issues when calculating
BAT entries.

The 1.00 VHDX spec requires BlockSize to be min 1MB, max 256MB.
LogicalSectorSize is required to be either 512 or 4096 bytes.

Reported-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Jeff Cody <jcody@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/vhdx.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/block/vhdx.c b/block/vhdx.c
index 5390ba6..509baaf 100644
--- a/block/vhdx.c
+++ b/block/vhdx.c
@@ -780,12 +780,20 @@ static int vhdx_parse_metadata(BlockDriverState *bs, BDRVVHDXState *s)
     le32_to_cpus(&s->logical_sector_size);
     le32_to_cpus(&s->physical_sector_size);
 
-    if (s->logical_sector_size == 0 || s->params.block_size == 0) {
+    if (s->params.block_size < VHDX_BLOCK_SIZE_MIN ||
+        s->params.block_size > VHDX_BLOCK_SIZE_MAX) {
         ret = -EINVAL;
         goto exit;
     }
 
-    /* both block_size and sector_size are guaranteed powers of 2 */
+    /* only 2 supported sector sizes */
+    if (s->logical_sector_size != 512 && s->logical_sector_size != 4096) {
+        ret = -EINVAL;
+        goto exit;
+    }
+
+    /* Both block_size and sector_size are guaranteed powers of 2, below.
+       Due to range checks above, s->sectors_per_block can never be < 256 */
     s->sectors_per_block = s->params.block_size / s->logical_sector_size;
     s->chunk_ratio = (VHDX_MAX_SECTORS_PER_BLOCK) *
                      (uint64_t)s->logical_sector_size /
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 21/51] curl: check data size before memcpy to local buffer. (CVE-2014-0144)

[Stefan Hajnoczi]

From: Fam Zheng <famz@redhat.com>

curl_read_cb is callback function for libcurl when data arrives. The
data size passed in here is not guaranteed to be within the range of
request we submitted, so we may overflow the guest IO buffer. Check the
real size we have before memcpy to buffer to avoid overflow.

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/curl.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/block/curl.c b/block/curl.c
index 3494c6d..1b9b1f6 100644
--- a/block/curl.c
+++ b/block/curl.c
@@ -157,6 +157,11 @@ static size_t curl_read_cb(void *ptr, size_t size, size_t nmemb, void *opaque)
     if (!s || !s->orig_buf)
         goto read_end;
 
+    if (s->buf_off >= s->buf_len) {
+        /* buffer full, read nothing */
+        return 0;
+    }
+    realsize = MIN(realsize, s->buf_len - s->buf_off);
     memcpy(s->orig_buf + s->buf_off, ptr, realsize);
     s->buf_off += realsize;
 
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 22/51] qcow2: Check header_length (CVE-2014-0144)

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

This fixes an unbounded allocation for s->unknown_header_fields.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2.c              | 34 +++++++++++++++++++-------
 tests/qemu-iotests/080     | 61 ++++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/080.out |  9 +++++++
 tests/qemu-iotests/group   |  1 +
 4 files changed, 96 insertions(+), 9 deletions(-)
 create mode 100755 tests/qemu-iotests/080
 create mode 100644 tests/qemu-iotests/080.out

diff --git a/block/qcow2.c b/block/qcow2.c
index 10eccf9..7809f5c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -460,6 +460,18 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
 
     s->qcow_version = header.version;
 
+    /* Initialise cluster size */
+    if (header.cluster_bits < MIN_CLUSTER_BITS ||
+        header.cluster_bits > MAX_CLUSTER_BITS) {
+        error_setg(errp, "Unsupported cluster size: 2^%i", header.cluster_bits);
+        ret = -EINVAL;
+        goto fail;
+    }
+
+    s->cluster_bits = header.cluster_bits;
+    s->cluster_size = 1 << s->cluster_bits;
+    s->cluster_sectors = 1 << (s->cluster_bits - 9);
+
     /* Initialise version 3 header fields */
     if (header.version == 2) {
         header.incompatible_features    = 0;
@@ -473,6 +485,18 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
         be64_to_cpus(&header.autoclear_features);
         be32_to_cpus(&header.refcount_order);
         be32_to_cpus(&header.header_length);
+
+        if (header.header_length < 104) {
+            error_setg(errp, "qcow2 header too short");
+            ret = -EINVAL;
+            goto fail;
+        }
+    }
+
+    if (header.header_length > s->cluster_size) {
+        error_setg(errp, "qcow2 header exceeds cluster size");
+        ret = -EINVAL;
+        goto fail;
     }
 
     if (header.header_length > sizeof(header)) {
@@ -530,12 +554,6 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     }
     s->refcount_order = header.refcount_order;
 
-    if (header.cluster_bits < MIN_CLUSTER_BITS ||
-        header.cluster_bits > MAX_CLUSTER_BITS) {
-        error_setg(errp, "Unsupported cluster size: 2^%i", header.cluster_bits);
-        ret = -EINVAL;
-        goto fail;
-    }
     if (header.crypt_method > QCOW_CRYPT_AES) {
         error_setg(errp, "Unsupported encryption method: %i",
                    header.crypt_method);
@@ -546,9 +564,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     if (s->crypt_method_header) {
         bs->encrypted = 1;
     }
-    s->cluster_bits = header.cluster_bits;
-    s->cluster_size = 1 << s->cluster_bits;
-    s->cluster_sectors = 1 << (s->cluster_bits - 9);
+
     s->l2_bits = s->cluster_bits - 3; /* L2 is always one cluster */
     s->l2_size = 1 << s->l2_bits;
     bs->total_sectors = header.size / 512;
diff --git a/tests/qemu-iotests/080 b/tests/qemu-iotests/080
new file mode 100755
index 0000000..6512701
--- /dev/null
+++ b/tests/qemu-iotests/080
@@ -0,0 +1,61 @@
+#!/bin/bash
+#
+# qcow2 format input validation tests
+#
+# Copyright (C) 2013 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=kwolf@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
+
+_supported_fmt qcow2
+_supported_proto generic
+_supported_os Linux
+
+header_size=104
+offset_header_size=100
+offset_ext_magic=$header_size
+offset_ext_size=$((header_size + 4))
+
+echo
+echo "== Huge header size =="
+_make_test_img 64M
+poke_file "$TEST_IMG" "$offset_header_size" "\xff\xff\xff\xff"
+{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+poke_file "$TEST_IMG" "$offset_header_size" "\x7f\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
+status=0
diff --git a/tests/qemu-iotests/080.out b/tests/qemu-iotests/080.out
new file mode 100644
index 0000000..41a166a
--- /dev/null
+++ b/tests/qemu-iotests/080.out
@@ -0,0 +1,9 @@
+QA output created by 080
+
+== Huge header size ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+qemu-io: can't open device TEST_DIR/t.qcow2: qcow2 header exceeds cluster size
+no file open, try 'help open'
+qemu-io: can't open device TEST_DIR/t.qcow2: qcow2 header exceeds cluster size
+no file open, try 'help open'
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index 9c99edc..ed44f35 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -85,6 +85,7 @@
 077 rw auto quick
 078 rw auto
 079 rw auto
+080 rw auto
 081 rw auto
 082 rw auto quick
 083 rw auto
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 23/51] qcow2: Check backing_file_offset (CVE-2014-0144)

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

Header, header extension and the backing file name must all be stored in
the first cluster. Setting the backing file to a much higher value
allowed header extensions to become much bigger than we want them to be
(unbounded allocation).

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2.c              |  6 ++++++
 tests/qemu-iotests/080     | 12 ++++++++++++
 tests/qemu-iotests/080.out |  7 +++++++
 3 files changed, 25 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 7809f5c..f0411a9 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -511,6 +511,12 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
         }
     }
 
+    if (header.backing_file_offset > s->cluster_size) {
+        error_setg(errp, "Invalid backing file offset");
+        ret = -EINVAL;
+        goto fail;
+    }
+
     if (header.backing_file_offset) {
         ext_end = header.backing_file_offset;
     } else {
diff --git a/tests/qemu-iotests/080 b/tests/qemu-iotests/080
index 6512701..6d588dd 100755
--- a/tests/qemu-iotests/080
+++ b/tests/qemu-iotests/080
@@ -43,6 +43,8 @@ _supported_proto generic
 _supported_os Linux
 
 header_size=104
+
+offset_backing_file_offset=8
 offset_header_size=100
 offset_ext_magic=$header_size
 offset_ext_size=$((header_size + 4))
@@ -55,6 +57,16 @@ poke_file "$TEST_IMG" "$offset_header_size" "\xff\xff\xff\xff"
 poke_file "$TEST_IMG" "$offset_header_size" "\x7f\xff\xff\xff"
 { $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
 
+echo
+echo "== Huge unknown header extension =="
+_make_test_img 64M
+poke_file "$TEST_IMG" "$offset_backing_file_offset" "\xff\xff\xff\xff\xff\xff\xff\xff"
+poke_file "$TEST_IMG" "$offset_ext_magic" "\x12\x34\x56\x78"
+poke_file "$TEST_IMG" "$offset_ext_size" "\x7f\xff\xff\xff"
+{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+poke_file "$TEST_IMG" "$offset_backing_file_offset" "\x00\x00\x00\x00\x00\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/080.out b/tests/qemu-iotests/080.out
index 41a166a..48c40aa 100644
--- a/tests/qemu-iotests/080.out
+++ b/tests/qemu-iotests/080.out
@@ -6,4 +6,11 @@ qemu-io: can't open device TEST_DIR/t.qcow2: qcow2 header exceeds cluster size
 no file open, try 'help open'
 qemu-io: can't open device TEST_DIR/t.qcow2: qcow2 header exceeds cluster size
 no file open, try 'help open'
+
+== Huge unknown header extension ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+qemu-io: can't open device TEST_DIR/t.qcow2: Invalid backing file offset
+no file open, try 'help open'
+qemu-io: can't open device TEST_DIR/t.qcow2: Header extension too large
+no file open, try 'help open'
 *** done
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 24/51] qcow2: Check refcount table size (CVE-2014-0144)

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

Limit the in-memory reference count table size to 8 MB, it's enough in
practice. This fixes an unbounded allocation as well as a buffer
overflow in qcow2_refcount_init().

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2-refcount.c     |  4 +++-
 block/qcow2.c              |  9 +++++++++
 tests/qemu-iotests/080     | 10 ++++++++++
 tests/qemu-iotests/080.out |  7 +++++++
 4 files changed, 29 insertions(+), 1 deletion(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 4a2df5f..e3c7ecd 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -40,8 +40,10 @@ static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
 int qcow2_refcount_init(BlockDriverState *bs)
 {
     BDRVQcowState *s = bs->opaque;
-    int ret, refcount_table_size2, i;
+    unsigned int refcount_table_size2, i;
+    int ret;
 
+    assert(s->refcount_table_size <= INT_MAX / sizeof(uint64_t));
     refcount_table_size2 = s->refcount_table_size * sizeof(uint64_t);
     s->refcount_table = g_malloc(refcount_table_size2);
     if (s->refcount_table_size > 0) {
diff --git a/block/qcow2.c b/block/qcow2.c
index f0411a9..b9b6e70 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -577,10 +577,19 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     s->csize_shift = (62 - (s->cluster_bits - 8));
     s->csize_mask = (1 << (s->cluster_bits - 8)) - 1;
     s->cluster_offset_mask = (1LL << s->csize_shift) - 1;
+
     s->refcount_table_offset = header.refcount_table_offset;
     s->refcount_table_size =
         header.refcount_table_clusters << (s->cluster_bits - 3);
 
+    if (header.refcount_table_clusters > (0x800000 >> s->cluster_bits)) {
+        /* 8 MB refcount table is enough for 2 PB images at 64k cluster size
+         * (128 GB for 512 byte clusters, 2 EB for 2 MB clusters) */
+        error_setg(errp, "Reference count table too large");
+        ret = -EINVAL;
+        goto fail;
+    }
+
     s->snapshots_offset = header.snapshots_offset;
     s->nb_snapshots = header.nb_snapshots;
 
diff --git a/tests/qemu-iotests/080 b/tests/qemu-iotests/080
index 6d588dd..6179e05 100755
--- a/tests/qemu-iotests/080
+++ b/tests/qemu-iotests/080
@@ -45,6 +45,7 @@ _supported_os Linux
 header_size=104
 
 offset_backing_file_offset=8
+offset_refcount_table_clusters=56
 offset_header_size=100
 offset_ext_magic=$header_size
 offset_ext_size=$((header_size + 4))
@@ -67,6 +68,15 @@ poke_file "$TEST_IMG" "$offset_ext_size" "\x7f\xff\xff\xff"
 poke_file "$TEST_IMG" "$offset_backing_file_offset" "\x00\x00\x00\x00\x00\x00\x00\x00"
 { $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
 
+echo
+echo "== Huge refcount table size =="
+_make_test_img 64M
+poke_file "$TEST_IMG" "$offset_refcount_table_clusters" "\xff\xff\xff\xff"
+{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+poke_file "$TEST_IMG" "$offset_refcount_table_clusters" "\x00\x02\x00\x01"
+{ $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/080.out b/tests/qemu-iotests/080.out
index 48c40aa..6fef6d9 100644
--- a/tests/qemu-iotests/080.out
+++ b/tests/qemu-iotests/080.out
@@ -13,4 +13,11 @@ qemu-io: can't open device TEST_DIR/t.qcow2: Invalid backing file offset
 no file open, try 'help open'
 qemu-io: can't open device TEST_DIR/t.qcow2: Header extension too large
 no file open, try 'help open'
+
+== Huge refcount table size ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+qemu-io: can't open device TEST_DIR/t.qcow2: Reference count table too large
+no file open, try 'help open'
+qemu-io: can't open device TEST_DIR/t.qcow2: Reference count table too large
+no file open, try 'help open'
 *** done
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 25/51] qcow2: Validate refcount table offset

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

The end of the refcount table must not exceed INT64_MAX so that integer
overflows are avoided.

Also check for misaligned refcount table. Such images are invalid and
probably the result of data corruption. Error out to avoid further
corruption.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2.c              | 33 +++++++++++++++++++++++++++++++++
 tests/qemu-iotests/080     | 13 +++++++++++++
 tests/qemu-iotests/080.out | 10 ++++++++++
 3 files changed, 56 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index b9b6e70..37a332f 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -329,6 +329,32 @@ static int qcow2_check(BlockDriverState *bs, BdrvCheckResult *result,
     return ret;
 }
 
+static int validate_table_offset(BlockDriverState *bs, uint64_t offset,
+                                 uint64_t entries, size_t entry_len)
+{
+    BDRVQcowState *s = bs->opaque;
+    uint64_t size;
+
+    /* Use signed INT64_MAX as the maximum even for uint64_t header fields,
+     * because values will be passed to qemu functions taking int64_t. */
+    if (entries > INT64_MAX / entry_len) {
+        return -EINVAL;
+    }
+
+    size = entries * entry_len;
+
+    if (INT64_MAX - size < offset) {
+        return -EINVAL;
+    }
+
+    /* Tables must be cluster aligned */
+    if (offset & (s->cluster_size - 1)) {
+        return -EINVAL;
+    }
+
+    return 0;
+}
+
 static QemuOptsList qcow2_runtime_opts = {
     .name = "qcow2",
     .head = QTAILQ_HEAD_INITIALIZER(qcow2_runtime_opts.head),
@@ -590,6 +616,13 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
+    ret = validate_table_offset(bs, s->refcount_table_offset,
+                                s->refcount_table_size, sizeof(uint64_t));
+    if (ret < 0) {
+        error_setg(errp, "Invalid reference count table offset");
+        goto fail;
+    }
+
     s->snapshots_offset = header.snapshots_offset;
     s->nb_snapshots = header.nb_snapshots;
 
diff --git a/tests/qemu-iotests/080 b/tests/qemu-iotests/080
index 6179e05..f58ac73 100755
--- a/tests/qemu-iotests/080
+++ b/tests/qemu-iotests/080
@@ -45,6 +45,7 @@ _supported_os Linux
 header_size=104
 
 offset_backing_file_offset=8
+offset_refcount_table_offset=48
 offset_refcount_table_clusters=56
 offset_header_size=100
 offset_ext_magic=$header_size
@@ -76,6 +77,18 @@ poke_file "$TEST_IMG" "$offset_refcount_table_clusters" "\xff\xff\xff\xff"
 poke_file "$TEST_IMG" "$offset_refcount_table_clusters" "\x00\x02\x00\x01"
 { $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
 
+echo
+echo "== Misaligned refcount table =="
+_make_test_img 64M
+poke_file "$TEST_IMG" "$offset_refcount_table_offset" "\x12\x34\x56\x78\x90\xab\xcd\xef"
+{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== Huge refcount offset =="
+_make_test_img 64M
+poke_file "$TEST_IMG" "$offset_refcount_table_offset" "\xff\xff\xff\xff\xff\xff\x00\x00"
+poke_file "$TEST_IMG" "$offset_refcount_table_clusters" "\x00\x00\x00\x7f"
+{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
 
 # success, all done
 echo "*** done"
diff --git a/tests/qemu-iotests/080.out b/tests/qemu-iotests/080.out
index 6fef6d9..f919b58 100644
--- a/tests/qemu-iotests/080.out
+++ b/tests/qemu-iotests/080.out
@@ -20,4 +20,14 @@ qemu-io: can't open device TEST_DIR/t.qcow2: Reference count table too large
 no file open, try 'help open'
 qemu-io: can't open device TEST_DIR/t.qcow2: Reference count table too large
 no file open, try 'help open'
+
+== Misaligned refcount table ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+qemu-io: can't open device TEST_DIR/t.qcow2: Invalid reference count table offset
+no file open, try 'help open'
+
+== Huge refcount offset ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+qemu-io: can't open device TEST_DIR/t.qcow2: Invalid reference count table offset
+no file open, try 'help open'
 *** done
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 26/51] qcow2: Validate snapshot table offset/size (CVE-2014-0144)

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

This avoid unbounded memory allocation and fixes a potential buffer
overflow on 32 bit hosts.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2-snapshot.c     | 29 ++++-------------------------
 block/qcow2.c              | 15 +++++++++++++++
 block/qcow2.h              | 29 ++++++++++++++++++++++++++++-
 tests/qemu-iotests/080     | 27 +++++++++++++++++++++++++++
 tests/qemu-iotests/080.out | 17 +++++++++++++++++
 5 files changed, 91 insertions(+), 26 deletions(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 2fc6320..87fbfe1 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -26,31 +26,6 @@
 #include "block/block_int.h"
 #include "block/qcow2.h"
 
-typedef struct QEMU_PACKED QCowSnapshotHeader {
-    /* header is 8 byte aligned */
-    uint64_t l1_table_offset;
-
-    uint32_t l1_size;
-    uint16_t id_str_size;
-    uint16_t name_size;
-
-    uint32_t date_sec;
-    uint32_t date_nsec;
-
-    uint64_t vm_clock_nsec;
-
-    uint32_t vm_state_size;
-    uint32_t extra_data_size; /* for extension */
-    /* extra data follows */
-    /* id_str follows */
-    /* name follows  */
-} QCowSnapshotHeader;
-
-typedef struct QEMU_PACKED QCowSnapshotExtraData {
-    uint64_t vm_state_size_large;
-    uint64_t disk_size;
-} QCowSnapshotExtraData;
-
 void qcow2_free_snapshots(BlockDriverState *bs)
 {
     BDRVQcowState *s = bs->opaque;
@@ -357,6 +332,10 @@ int qcow2_snapshot_create(BlockDriverState *bs, QEMUSnapshotInfo *sn_info)
     uint64_t *l1_table = NULL;
     int64_t l1_table_offset;
 
+    if (s->nb_snapshots >= QCOW_MAX_SNAPSHOTS) {
+        return -EFBIG;
+    }
+
     memset(sn, 0, sizeof(*sn));
 
     /* Generate an ID if it wasn't passed */
diff --git a/block/qcow2.c b/block/qcow2.c
index 37a332f..8d0a09e 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -623,6 +623,21 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
+    /* Snapshot table offset/length */
+    if (header.nb_snapshots > QCOW_MAX_SNAPSHOTS) {
+        error_setg(errp, "Too many snapshots");
+        ret = -EINVAL;
+        goto fail;
+    }
+
+    ret = validate_table_offset(bs, header.snapshots_offset,
+                                header.nb_snapshots,
+                                sizeof(QCowSnapshotHeader));
+    if (ret < 0) {
+        error_setg(errp, "Invalid snapshot table offset");
+        goto fail;
+    }
+
     s->snapshots_offset = header.snapshots_offset;
     s->nb_snapshots = header.nb_snapshots;
 
diff --git a/block/qcow2.h b/block/qcow2.h
index 0b0eac8..b153505 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -38,6 +38,7 @@
 #define QCOW_CRYPT_AES  1
 
 #define QCOW_MAX_CRYPT_CLUSTERS 32
+#define QCOW_MAX_SNAPSHOTS 65536
 
 /* indicate that the refcount of the referenced cluster is exactly one. */
 #define QCOW_OFLAG_COPIED     (1ULL << 63)
@@ -97,6 +98,32 @@ typedef struct QCowHeader {
     uint32_t header_length;
 } QEMU_PACKED QCowHeader;
 
+typedef struct QEMU_PACKED QCowSnapshotHeader {
+    /* header is 8 byte aligned */
+    uint64_t l1_table_offset;
+
+    uint32_t l1_size;
+    uint16_t id_str_size;
+    uint16_t name_size;
+
+    uint32_t date_sec;
+    uint32_t date_nsec;
+
+    uint64_t vm_clock_nsec;
+
+    uint32_t vm_state_size;
+    uint32_t extra_data_size; /* for extension */
+    /* extra data follows */
+    /* id_str follows */
+    /* name follows  */
+} QCowSnapshotHeader;
+
+typedef struct QEMU_PACKED QCowSnapshotExtraData {
+    uint64_t vm_state_size_large;
+    uint64_t disk_size;
+} QCowSnapshotExtraData;
+
+
 typedef struct QCowSnapshot {
     uint64_t l1_table_offset;
     uint32_t l1_size;
@@ -202,7 +229,7 @@ typedef struct BDRVQcowState {
     AES_KEY aes_decrypt_key;
     uint64_t snapshots_offset;
     int snapshots_size;
-    int nb_snapshots;
+    unsigned int nb_snapshots;
     QCowSnapshot *snapshots;
 
     int flags;
diff --git a/tests/qemu-iotests/080 b/tests/qemu-iotests/080
index f58ac73..8a8b460 100755
--- a/tests/qemu-iotests/080
+++ b/tests/qemu-iotests/080
@@ -47,6 +47,8 @@ header_size=104
 offset_backing_file_offset=8
 offset_refcount_table_offset=48
 offset_refcount_table_clusters=56
+offset_nb_snapshots=60
+offset_snapshots_offset=64
 offset_header_size=100
 offset_ext_magic=$header_size
 offset_ext_size=$((header_size + 4))
@@ -90,6 +92,31 @@ poke_file "$TEST_IMG" "$offset_refcount_table_offset" "\xff\xff\xff\xff\xff\xff\
 poke_file "$TEST_IMG" "$offset_refcount_table_clusters" "\x00\x00\x00\x7f"
 { $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
 
+echo
+echo "== Invalid snapshot table =="
+_make_test_img 64M
+poke_file "$TEST_IMG" "$offset_nb_snapshots" "\xff\xff\xff\xff"
+{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+poke_file "$TEST_IMG" "$offset_nb_snapshots" "\x7f\xff\xff\xff"
+{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+
+poke_file "$TEST_IMG" "$offset_snapshots_offset" "\xff\xff\xff\xff\xff\xff\x00\x00"
+poke_file "$TEST_IMG" "$offset_nb_snapshots" "\x00\x00\xff\xff"
+{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+
+poke_file "$TEST_IMG" "$offset_snapshots_offset" "\x12\x34\x56\x78\x90\xab\xcd\xef"
+poke_file "$TEST_IMG" "$offset_nb_snapshots" "\x00\x00\x00\x00"
+{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== Hitting snapshot table size limit =="
+_make_test_img 64M
+# Put the refcount table in a more or less safe place (16 MB)
+poke_file "$TEST_IMG" "$offset_snapshots_offset" "\x00\x00\x00\x00\x01\x00\x00\x00"
+poke_file "$TEST_IMG" "$offset_nb_snapshots" "\x00\x01\x00\x00"
+{ $QEMU_IMG snapshot -c test $TEST_IMG; } 2>&1 | _filter_testdir
+{ $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/080.out b/tests/qemu-iotests/080.out
index f919b58..b06f47f 100644
--- a/tests/qemu-iotests/080.out
+++ b/tests/qemu-iotests/080.out
@@ -30,4 +30,21 @@ no file open, try 'help open'
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
 qemu-io: can't open device TEST_DIR/t.qcow2: Invalid reference count table offset
 no file open, try 'help open'
+
+== Invalid snapshot table ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+qemu-io: can't open device TEST_DIR/t.qcow2: Too many snapshots
+no file open, try 'help open'
+qemu-io: can't open device TEST_DIR/t.qcow2: Too many snapshots
+no file open, try 'help open'
+qemu-io: can't open device TEST_DIR/t.qcow2: Invalid snapshot table offset
+no file open, try 'help open'
+qemu-io: can't open device TEST_DIR/t.qcow2: Invalid snapshot table offset
+no file open, try 'help open'
+
+== Hitting snapshot table size limit ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+qemu-img: Could not create snapshot 'test': -27 (File too large)
+read 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 *** done
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 27/51] qcow2: Validate active L1 table offset and size (CVE-2014-0144)

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

This avoids an unbounded allocation.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2.c              | 16 ++++++++++++++++
 tests/qemu-iotests/080     | 18 ++++++++++++++++++
 tests/qemu-iotests/080.out | 11 +++++++++++
 3 files changed, 45 insertions(+)

diff --git a/block/qcow2.c b/block/qcow2.c
index 8d0a09e..3639528 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -642,6 +642,13 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     s->nb_snapshots = header.nb_snapshots;
 
     /* read the level 1 table */
+    if (header.l1_size > 0x2000000) {
+        /* 32 MB L1 table is enough for 2 PB images at 64k cluster size
+         * (128 GB for 512 byte clusters, 2 EB for 2 MB clusters) */
+        error_setg(errp, "Active L1 table too large");
+        ret = -EFBIG;
+        goto fail;
+    }
     s->l1_size = header.l1_size;
 
     l1_vm_state_index = size_to_l1(s, header.size);
@@ -659,7 +666,16 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
         ret = -EINVAL;
         goto fail;
     }
+
+    ret = validate_table_offset(bs, header.l1_table_offset,
+                                header.l1_size, sizeof(uint64_t));
+    if (ret < 0) {
+        error_setg(errp, "Invalid L1 table offset");
+        goto fail;
+    }
     s->l1_table_offset = header.l1_table_offset;
+
+
     if (s->l1_size > 0) {
         s->l1_table = g_malloc0(
             align_offset(s->l1_size * sizeof(uint64_t), 512));
diff --git a/tests/qemu-iotests/080 b/tests/qemu-iotests/080
index 8a8b460..7255b6c 100755
--- a/tests/qemu-iotests/080
+++ b/tests/qemu-iotests/080
@@ -45,6 +45,8 @@ _supported_os Linux
 header_size=104
 
 offset_backing_file_offset=8
+offset_l1_size=36
+offset_l1_table_offset=40
 offset_refcount_table_offset=48
 offset_refcount_table_clusters=56
 offset_nb_snapshots=60
@@ -117,6 +119,22 @@ poke_file "$TEST_IMG" "$offset_nb_snapshots" "\x00\x01\x00\x00"
 { $QEMU_IMG snapshot -c test $TEST_IMG; } 2>&1 | _filter_testdir
 { $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
 
+echo
+echo "== Invalid L1 table =="
+_make_test_img 64M
+poke_file "$TEST_IMG" "$offset_l1_size" "\xff\xff\xff\xff"
+{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+poke_file "$TEST_IMG" "$offset_l1_size" "\x7f\xff\xff\xff"
+{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+
+poke_file "$TEST_IMG" "$offset_l1_table_offset" "\x7f\xff\xff\xff\xff\xff\x00\x00"
+poke_file "$TEST_IMG" "$offset_l1_size" "\x00\x00\xff\xff"
+{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+
+poke_file "$TEST_IMG" "$offset_l1_table_offset" "\x12\x34\x56\x78\x90\xab\xcd\xef"
+poke_file "$TEST_IMG" "$offset_l1_size" "\x00\x00\x00\x01"
+{ $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/080.out b/tests/qemu-iotests/080.out
index b06f47f..4ec2545 100644
--- a/tests/qemu-iotests/080.out
+++ b/tests/qemu-iotests/080.out
@@ -47,4 +47,15 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864
 qemu-img: Could not create snapshot 'test': -27 (File too large)
 read 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== Invalid L1 table ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+qemu-io: can't open device TEST_DIR/t.qcow2: Active L1 table too large
+no file open, try 'help open'
+qemu-io: can't open device TEST_DIR/t.qcow2: Active L1 table too large
+no file open, try 'help open'
+qemu-io: can't open device TEST_DIR/t.qcow2: Invalid L1 table offset
+no file open, try 'help open'
+qemu-io: can't open device TEST_DIR/t.qcow2: Invalid L1 table offset
+no file open, try 'help open'
 *** done
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 28/51] qcow2: Fix backing file name length check

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

len could become negative and would pass the check then. Nothing bad
happened because bdrv_pread() happens to return an error for negative
length values, but make variables for sizes unsigned anyway.

This patch also changes the behaviour to error out on invalid lengths
instead of silently truncating it to 1023.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2.c              | 9 ++++++---
 tests/qemu-iotests/080     | 8 ++++++++
 tests/qemu-iotests/080.out | 5 +++++
 3 files changed, 19 insertions(+), 3 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 3639528..cc1bfeb 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -445,7 +445,8 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
                       Error **errp)
 {
     BDRVQcowState *s = bs->opaque;
-    int len, i, ret = 0;
+    unsigned int len, i;
+    int ret = 0;
     QCowHeader header;
     QemuOpts *opts;
     Error *local_err = NULL;
@@ -721,8 +722,10 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     /* read the backing file name */
     if (header.backing_file_offset != 0) {
         len = header.backing_file_size;
-        if (len > 1023) {
-            len = 1023;
+        if (len > MIN(1023, s->cluster_size - header.backing_file_offset)) {
+            error_setg(errp, "Backing file name too long");
+            ret = -EINVAL;
+            goto fail;
         }
         ret = bdrv_pread(bs->file, header.backing_file_offset,
                          bs->backing_file, len);
diff --git a/tests/qemu-iotests/080 b/tests/qemu-iotests/080
index 7255b6c..f3091a9 100755
--- a/tests/qemu-iotests/080
+++ b/tests/qemu-iotests/080
@@ -45,6 +45,7 @@ _supported_os Linux
 header_size=104
 
 offset_backing_file_offset=8
+offset_backing_file_size=16
 offset_l1_size=36
 offset_l1_table_offset=40
 offset_refcount_table_offset=48
@@ -135,6 +136,13 @@ poke_file "$TEST_IMG" "$offset_l1_table_offset" "\x12\x34\x56\x78\x90\xab\xcd\xe
 poke_file "$TEST_IMG" "$offset_l1_size" "\x00\x00\x00\x01"
 { $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
 
+echo
+echo "== Invalid backing file size =="
+_make_test_img 64M
+poke_file "$TEST_IMG" "$offset_backing_file_offset" "\x00\x00\x00\x00\x00\x00\x10\x00"
+poke_file "$TEST_IMG" "$offset_backing_file_size" "\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/080.out b/tests/qemu-iotests/080.out
index 4ec2545..8103211 100644
--- a/tests/qemu-iotests/080.out
+++ b/tests/qemu-iotests/080.out
@@ -58,4 +58,9 @@ qemu-io: can't open device TEST_DIR/t.qcow2: Invalid L1 table offset
 no file open, try 'help open'
 qemu-io: can't open device TEST_DIR/t.qcow2: Invalid L1 table offset
 no file open, try 'help open'
+
+== Invalid backing file size ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+qemu-io: can't open device TEST_DIR/t.qcow2: Backing file name too long
+no file open, try 'help open'
 *** done
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 29/51] qcow2: Don't rely on free_cluster_index in alloc_refcount_block() (CVE-2014-0147)

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

free_cluster_index is only correct if update_refcount() was called from
an allocation function, and even there it's brittle because it's used to
protect unfinished allocations which still have a refcount of 0 - if it
moves in the wrong place, the unfinished allocation can be corrupted.

So not using it any more seems to be a good idea. Instead, use the
first requested cluster to do the calculations. Return -EAGAIN if
unfinished allocations could become invalid and let the caller restart
its search for some free clusters.

The context of creating a snapsnot is one situation where
update_refcount() is called outside of a cluster allocation. For this
case, the change fixes a buffer overflow if a cluster is referenced in
an L2 table that cannot be represented by an existing refcount block.
(new_table[refcount_table_index] was out of bounds)

[Bump the qemu-iotests 026 refblock_alloc.write leak count from 10 to
11.
--Stefan]

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2-refcount.c     | 72 ++++++++++++++++++++++++----------------------
 block/qcow2.c              | 11 +++----
 tests/qemu-iotests/026.out |  6 ++--
 tests/qemu-iotests/044.out |  2 +-
 tests/qemu-iotests/080     | 11 +++++++
 tests/qemu-iotests/080.out |  7 +++++
 6 files changed, 65 insertions(+), 44 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index e3c7ecd..220b322 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -194,10 +194,11 @@ static int alloc_refcount_block(BlockDriverState *bs,
      *   they can describe them themselves.
      *
      * - We need to consider that at this point we are inside update_refcounts
-     *   and doing the initial refcount increase. This means that some clusters
-     *   have already been allocated by the caller, but their refcount isn't
-     *   accurate yet. free_cluster_index tells us where this allocation ends
-     *   as long as we don't overwrite it by freeing clusters.
+     *   and potentially doing an initial refcount increase. This means that
+     *   some clusters have already been allocated by the caller, but their
+     *   refcount isn't accurate yet. If we allocate clusters for metadata, we
+     *   need to return -EAGAIN to signal the caller that it needs to restart
+     *   the search for free clusters.
      *
      * - alloc_clusters_noref and qcow2_free_clusters may load a different
      *   refcount block into the cache
@@ -282,7 +283,10 @@ static int alloc_refcount_block(BlockDriverState *bs,
         }
 
         s->refcount_table[refcount_table_index] = new_block;
-        return 0;
+
+        /* The new refcount block may be where the caller intended to put its
+         * data, so let it restart the search. */
+        return -EAGAIN;
     }
 
     ret = qcow2_cache_put(bs, s->refcount_block_cache, (void**) refcount_block);
@@ -305,8 +309,7 @@ static int alloc_refcount_block(BlockDriverState *bs,
 
     /* Calculate the number of refcount blocks needed so far */
     uint64_t refcount_block_clusters = 1 << (s->cluster_bits - REFCOUNT_SHIFT);
-    uint64_t blocks_used = (s->free_cluster_index +
-        refcount_block_clusters - 1) / refcount_block_clusters;
+    uint64_t blocks_used = DIV_ROUND_UP(cluster_index, refcount_block_clusters);
 
     /* And now we need at least one block more for the new metadata */
     uint64_t table_size = next_refcount_table_size(s, blocks_used + 1);
@@ -339,8 +342,6 @@ static int alloc_refcount_block(BlockDriverState *bs,
     uint16_t *new_blocks = g_malloc0(blocks_clusters * s->cluster_size);
     uint64_t *new_table = g_malloc0(table_size * sizeof(uint64_t));
 
-    assert(meta_offset >= (s->free_cluster_index * s->cluster_size));
-
     /* Fill the new refcount table */
     memcpy(new_table, s->refcount_table,
         s->refcount_table_size * sizeof(uint64_t));
@@ -403,18 +404,19 @@ static int alloc_refcount_block(BlockDriverState *bs,
     s->refcount_table_size = table_size;
     s->refcount_table_offset = table_offset;
 
-    /* Free old table. Remember, we must not change free_cluster_index */
-    uint64_t old_free_cluster_index = s->free_cluster_index;
+    /* Free old table. */
     qcow2_free_clusters(bs, old_table_offset, old_table_size * sizeof(uint64_t),
                         QCOW2_DISCARD_OTHER);
-    s->free_cluster_index = old_free_cluster_index;
 
     ret = load_refcount_block(bs, new_block, (void**) refcount_block);
     if (ret < 0) {
         return ret;
     }
 
-    return 0;
+    /* If we were trying to do the initial refcount update for some cluster
+     * allocation, we might have used the same clusters to store newly
+     * allocated metadata. Make the caller search some new space. */
+    return -EAGAIN;
 
 fail_table:
     g_free(new_table);
@@ -660,12 +662,15 @@ int64_t qcow2_alloc_clusters(BlockDriverState *bs, int64_t size)
     int ret;
 
     BLKDBG_EVENT(bs->file, BLKDBG_CLUSTER_ALLOC);
-    offset = alloc_clusters_noref(bs, size);
-    if (offset < 0) {
-        return offset;
-    }
+    do {
+        offset = alloc_clusters_noref(bs, size);
+        if (offset < 0) {
+            return offset;
+        }
+
+        ret = update_refcount(bs, offset, size, 1, QCOW2_DISCARD_NEVER);
+    } while (ret == -EAGAIN);
 
-    ret = update_refcount(bs, offset, size, 1, QCOW2_DISCARD_NEVER);
     if (ret < 0) {
         return ret;
     }
@@ -678,7 +683,6 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
 {
     BDRVQcowState *s = bs->opaque;
     uint64_t cluster_index;
-    uint64_t old_free_cluster_index;
     uint64_t i;
     int refcount, ret;
 
@@ -687,30 +691,28 @@ int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
         return 0;
     }
 
-    /* Check how many clusters there are free */
-    cluster_index = offset >> s->cluster_bits;
-    for(i = 0; i < nb_clusters; i++) {
-        refcount = get_refcount(bs, cluster_index++);
+    do {
+        /* Check how many clusters there are free */
+        cluster_index = offset >> s->cluster_bits;
+        for(i = 0; i < nb_clusters; i++) {
+            refcount = get_refcount(bs, cluster_index++);
 
-        if (refcount < 0) {
-            return refcount;
-        } else if (refcount != 0) {
-            break;
+            if (refcount < 0) {
+                return refcount;
+            } else if (refcount != 0) {
+                break;
+            }
         }
-    }
 
-    /* And then allocate them */
-    old_free_cluster_index = s->free_cluster_index;
-    s->free_cluster_index = cluster_index + i;
+        /* And then allocate them */
+        ret = update_refcount(bs, offset, i << s->cluster_bits, 1,
+                              QCOW2_DISCARD_NEVER);
+    } while (ret == -EAGAIN);
 
-    ret = update_refcount(bs, offset, i << s->cluster_bits, 1,
-                          QCOW2_DISCARD_NEVER);
     if (ret < 0) {
         return ret;
     }
 
-    s->free_cluster_index = old_free_cluster_index;
-
     return i;
 }
 
diff --git a/block/qcow2.c b/block/qcow2.c
index cc1bfeb..d4d991c 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1602,7 +1602,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
      */
     BlockDriverState* bs;
     QCowHeader *header;
-    uint8_t* refcount_table;
+    uint64_t* refcount_table;
     Error *local_err = NULL;
     int ret;
 
@@ -1654,9 +1654,10 @@ static int qcow2_create2(const char *filename, int64_t total_size,
         goto out;
     }
 
-    /* Write an empty refcount table */
-    refcount_table = g_malloc0(cluster_size);
-    ret = bdrv_pwrite(bs, cluster_size, refcount_table, cluster_size);
+    /* Write a refcount table with one refcount block */
+    refcount_table = g_malloc0(2 * cluster_size);
+    refcount_table[0] = cpu_to_be64(2 * cluster_size);
+    ret = bdrv_pwrite(bs, cluster_size, refcount_table, 2 * cluster_size);
     g_free(refcount_table);
 
     if (ret < 0) {
@@ -1681,7 +1682,7 @@ static int qcow2_create2(const char *filename, int64_t total_size,
         goto out;
     }
 
-    ret = qcow2_alloc_clusters(bs, 2 * cluster_size);
+    ret = qcow2_alloc_clusters(bs, 3 * cluster_size);
     if (ret < 0) {
         error_setg_errno(errp, -ret, "Could not allocate clusters for qcow2 "
                          "header and refcount table");
diff --git a/tests/qemu-iotests/026.out b/tests/qemu-iotests/026.out
index 1504579..f7c78e7 100644
--- a/tests/qemu-iotests/026.out
+++ b/tests/qemu-iotests/026.out
@@ -475,7 +475,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 Event: refblock_alloc.write_blocks; errno: 28; imm: off; once: off; write 
 write failed: No space left on device
 
-10 leaked clusters were found on the image.
+11 leaked clusters were found on the image.
 This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
@@ -499,7 +499,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 Event: refblock_alloc.write_table; errno: 28; imm: off; once: off; write 
 write failed: No space left on device
 
-10 leaked clusters were found on the image.
+11 leaked clusters were found on the image.
 This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
@@ -523,7 +523,7 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824
 Event: refblock_alloc.switch_table; errno: 28; imm: off; once: off; write 
 write failed: No space left on device
 
-10 leaked clusters were found on the image.
+11 leaked clusters were found on the image.
 This means waste of disk space, but no harm to data.
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1073741824 
 
diff --git a/tests/qemu-iotests/044.out b/tests/qemu-iotests/044.out
index 5c5aa92..4789a53 100644
--- a/tests/qemu-iotests/044.out
+++ b/tests/qemu-iotests/044.out
@@ -1,6 +1,6 @@
 No errors were found on the image.
 7292415/33554432 = 21.73% allocated, 0.00% fragmented, 0.00% compressed clusters
-Image end offset: 4296448000
+Image end offset: 4296152064
 .
 ----------------------------------------------------------------------
 Ran 1 tests
diff --git a/tests/qemu-iotests/080 b/tests/qemu-iotests/080
index f3091a9..56f8903 100755
--- a/tests/qemu-iotests/080
+++ b/tests/qemu-iotests/080
@@ -56,6 +56,8 @@ offset_header_size=100
 offset_ext_magic=$header_size
 offset_ext_size=$((header_size + 4))
 
+offset_l2_table_0=$((0x40000))
+
 echo
 echo "== Huge header size =="
 _make_test_img 64M
@@ -143,6 +145,15 @@ poke_file "$TEST_IMG" "$offset_backing_file_offset" "\x00\x00\x00\x00\x00\x00\x1
 poke_file "$TEST_IMG" "$offset_backing_file_size" "\xff\xff\xff\xff"
 { $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
 
+echo
+echo "== Invalid L2 entry (huge physical offset) =="
+_make_test_img 64M
+{ $QEMU_IO -c "write 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+poke_file "$TEST_IMG" "$offset_l2_table_0" "\xbf\xff\xff\xff\xff\xff\x00\x00"
+{ $QEMU_IMG snapshot -c test $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+poke_file "$TEST_IMG" "$offset_l2_table_0" "\x80\x00\x00\xff\xff\xff\x00\x00"
+{ $QEMU_IMG snapshot -c test $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/080.out b/tests/qemu-iotests/080.out
index 8103211..303d6c3 100644
--- a/tests/qemu-iotests/080.out
+++ b/tests/qemu-iotests/080.out
@@ -63,4 +63,11 @@ no file open, try 'help open'
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
 qemu-io: can't open device TEST_DIR/t.qcow2: Backing file name too long
 no file open, try 'help open'
+
+== Invalid L2 entry (huge physical offset) ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+wrote 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-img: Could not create snapshot 'test': -27 (File too large)
+qemu-img: Could not create snapshot 'test': -11 (Resource temporarily unavailable)
 *** done
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 30/51] qcow2: Avoid integer overflow in get_refcount (CVE-2014-0143)

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

This ensures that the checks catch all invalid cluster indexes
instead of returning the refcount of a wrong cluster.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2-refcount.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 220b322..561d659 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -89,7 +89,7 @@ static int load_refcount_block(BlockDriverState *bs,
 static int get_refcount(BlockDriverState *bs, int64_t cluster_index)
 {
     BDRVQcowState *s = bs->opaque;
-    int refcount_table_index, block_index;
+    uint64_t refcount_table_index, block_index;
     int64_t refcount_block_offset;
     int ret;
     uint16_t *refcount_block;
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 31/51] qcow2: Check new refcount table size on growth

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

If the size becomes larger than what qcow2_open() would accept, fail the
growing operation.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2-refcount.c | 4 ++++
 block/qcow2.c          | 4 +---
 block/qcow2.h          | 9 +++++++++
 3 files changed, 14 insertions(+), 3 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 561d659..1c78ff8 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -311,6 +311,10 @@ static int alloc_refcount_block(BlockDriverState *bs,
     uint64_t refcount_block_clusters = 1 << (s->cluster_bits - REFCOUNT_SHIFT);
     uint64_t blocks_used = DIV_ROUND_UP(cluster_index, refcount_block_clusters);
 
+    if (blocks_used > QCOW_MAX_REFTABLE_SIZE / sizeof(uint64_t)) {
+        return -EFBIG;
+    }
+
     /* And now we need at least one block more for the new metadata */
     uint64_t table_size = next_refcount_table_size(s, blocks_used + 1);
     uint64_t last_table_size;
diff --git a/block/qcow2.c b/block/qcow2.c
index d4d991c..b64564d 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -609,9 +609,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     s->refcount_table_size =
         header.refcount_table_clusters << (s->cluster_bits - 3);
 
-    if (header.refcount_table_clusters > (0x800000 >> s->cluster_bits)) {
-        /* 8 MB refcount table is enough for 2 PB images at 64k cluster size
-         * (128 GB for 512 byte clusters, 2 EB for 2 MB clusters) */
+    if (header.refcount_table_clusters > qcow2_max_refcount_clusters(s)) {
         error_setg(errp, "Reference count table too large");
         ret = -EINVAL;
         goto fail;
diff --git a/block/qcow2.h b/block/qcow2.h
index b153505..4015373 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -40,6 +40,10 @@
 #define QCOW_MAX_CRYPT_CLUSTERS 32
 #define QCOW_MAX_SNAPSHOTS 65536
 
+/* 8 MB refcount table is enough for 2 PB images at 64k cluster size
+ * (128 GB for 512 byte clusters, 2 EB for 2 MB clusters) */
+#define QCOW_MAX_REFTABLE_SIZE 0x800000
+
 /* indicate that the refcount of the referenced cluster is exactly one. */
 #define QCOW_OFLAG_COPIED     (1ULL << 63)
 /* indicate that the cluster is compressed (they never have the copied flag) */
@@ -410,6 +414,11 @@ static inline int64_t qcow2_vm_state_offset(BDRVQcowState *s)
     return (int64_t)s->l1_vm_state_index << (s->cluster_bits + s->l2_bits);
 }
 
+static inline uint64_t qcow2_max_refcount_clusters(BDRVQcowState *s)
+{
+    return QCOW_MAX_REFTABLE_SIZE >> s->cluster_bits;
+}
+
 static inline int qcow2_get_cluster_type(uint64_t l2_entry)
 {
     if (l2_entry & QCOW_OFLAG_COMPRESSED) {
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 32/51] qcow2: Fix types in qcow2_alloc_clusters and alloc_clusters_noref

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

In order to avoid integer overflows.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2-refcount.c | 11 ++++++-----
 block/qcow2.h          |  6 +++---
 2 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 1c78ff8..9130042 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -28,7 +28,7 @@
 #include "qemu/range.h"
 #include "qapi/qmp/types.h"
 
-static int64_t alloc_clusters_noref(BlockDriverState *bs, int64_t size);
+static int64_t alloc_clusters_noref(BlockDriverState *bs, uint64_t size);
 static int QEMU_WARN_UNUSED_RESULT update_refcount(BlockDriverState *bs,
                             int64_t offset, int64_t length,
                             int addend, enum qcow2_discard_type type);
@@ -635,15 +635,16 @@ int qcow2_update_cluster_refcount(BlockDriverState *bs,
 
 
 /* return < 0 if error */
-static int64_t alloc_clusters_noref(BlockDriverState *bs, int64_t size)
+static int64_t alloc_clusters_noref(BlockDriverState *bs, uint64_t size)
 {
     BDRVQcowState *s = bs->opaque;
-    int i, nb_clusters, refcount;
+    uint64_t i, nb_clusters;
+    int refcount;
 
     nb_clusters = size_to_clusters(s, size);
 retry:
     for(i = 0; i < nb_clusters; i++) {
-        int64_t next_cluster_index = s->free_cluster_index++;
+        uint64_t next_cluster_index = s->free_cluster_index++;
         refcount = get_refcount(bs, next_cluster_index);
 
         if (refcount < 0) {
@@ -660,7 +661,7 @@ retry:
     return (s->free_cluster_index - nb_clusters) << s->cluster_bits;
 }
 
-int64_t qcow2_alloc_clusters(BlockDriverState *bs, int64_t size)
+int64_t qcow2_alloc_clusters(BlockDriverState *bs, uint64_t size)
 {
     int64_t offset;
     int ret;
diff --git a/block/qcow2.h b/block/qcow2.h
index 4015373..3649465 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -222,8 +222,8 @@ typedef struct BDRVQcowState {
     uint64_t *refcount_table;
     uint64_t refcount_table_offset;
     uint32_t refcount_table_size;
-    int64_t free_cluster_index;
-    int64_t free_byte_offset;
+    uint64_t free_cluster_index;
+    uint64_t free_byte_offset;
 
     CoMutex lock;
 
@@ -467,7 +467,7 @@ void qcow2_refcount_close(BlockDriverState *bs);
 int qcow2_update_cluster_refcount(BlockDriverState *bs, int64_t cluster_index,
                                   int addend, enum qcow2_discard_type type);
 
-int64_t qcow2_alloc_clusters(BlockDriverState *bs, int64_t size);
+int64_t qcow2_alloc_clusters(BlockDriverState *bs, uint64_t size);
 int qcow2_alloc_clusters_at(BlockDriverState *bs, uint64_t offset,
     int nb_clusters);
 int64_t qcow2_alloc_bytes(BlockDriverState *bs, int size);
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 33/51] qcow2: Protect against some integer overflows in bdrv_check

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2-refcount.c | 18 +++++++++---------
 1 file changed, 9 insertions(+), 9 deletions(-)

diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
index 9130042..a37ee45 100644
--- a/block/qcow2-refcount.c
+++ b/block/qcow2-refcount.c
@@ -1020,8 +1020,7 @@ static void inc_refcounts(BlockDriverState *bs,
                           int64_t offset, int64_t size)
 {
     BDRVQcowState *s = bs->opaque;
-    int64_t start, last, cluster_offset;
-    int k;
+    uint64_t start, last, cluster_offset, k;
 
     if (size <= 0)
         return;
@@ -1031,11 +1030,7 @@ static void inc_refcounts(BlockDriverState *bs,
     for(cluster_offset = start; cluster_offset <= last;
         cluster_offset += s->cluster_size) {
         k = cluster_offset >> s->cluster_bits;
-        if (k < 0) {
-            fprintf(stderr, "ERROR: invalid cluster offset=0x%" PRIx64 "\n",
-                cluster_offset);
-            res->corruptions++;
-        } else if (k >= refcount_table_size) {
+        if (k >= refcount_table_size) {
             fprintf(stderr, "Warning: cluster offset=0x%" PRIx64 " is after "
                 "the end of the image file, can't properly check refcounts.\n",
                 cluster_offset);
@@ -1478,14 +1473,19 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
                           BdrvCheckMode fix)
 {
     BDRVQcowState *s = bs->opaque;
-    int64_t size, i, highest_cluster;
-    int nb_clusters, refcount1, refcount2;
+    int64_t size, i, highest_cluster, nb_clusters;
+    int refcount1, refcount2;
     QCowSnapshot *sn;
     uint16_t *refcount_table;
     int ret;
 
     size = bdrv_getlength(bs->file);
     nb_clusters = size_to_clusters(s, size);
+    if (nb_clusters > INT_MAX) {
+        res->check_errors++;
+        return -EFBIG;
+    }
+
     refcount_table = g_malloc0(nb_clusters * sizeof(uint16_t));
 
     res->bfi.total_clusters =
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 34/51] qcow2: Fix new L1 table size check (CVE-2014-0143)

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

The size in bytes is assigned to an int later, so check that instead of
the number of entries.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2-cluster.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 9499df9..242e1f8 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -55,7 +55,7 @@ int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
         }
     }
 
-    if (new_l1_size > INT_MAX) {
+    if (new_l1_size > INT_MAX / sizeof(uint64_t)) {
         return -EFBIG;
     }
 
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 35/51] dmg: coding style and indentation cleanup

[Stefan Hajnoczi]

Clean up the mix of tabs and spaces, as well as the coding style
violations in block/dmg.c.  There are no semantic changes since this
patch simply reformats the code.

This patch is necessary before we can make meaningful changes to this
file, due to the inconsistent formatting and confusing indentation.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/dmg.c | 224 ++++++++++++++++++++++++++++++++----------------------------
 1 file changed, 120 insertions(+), 104 deletions(-)

diff --git a/block/dmg.c b/block/dmg.c
index d5e9b1f..be2f26e 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -96,9 +96,9 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
                     Error **errp)
 {
     BDRVDMGState *s = bs->opaque;
-    uint64_t info_begin,info_end,last_in_offset,last_out_offset;
+    uint64_t info_begin, info_end, last_in_offset, last_out_offset;
     uint32_t count, tmp;
-    uint32_t max_compressed_size=1,max_sectors_per_chunk=1,i;
+    uint32_t max_compressed_size = 1, max_sectors_per_chunk = 1, i;
     int64_t offset;
     int ret;
 
@@ -160,37 +160,39 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
             goto fail;
         }
 
-	if (type == 0x6d697368 && count >= 244) {
-	    int new_size, chunk_count;
+        if (type == 0x6d697368 && count >= 244) {
+            int new_size, chunk_count;
 
             offset += 4;
             offset += 200;
 
-	    chunk_count = (count-204)/40;
-	    new_size = sizeof(uint64_t) * (s->n_chunks + chunk_count);
-	    s->types = g_realloc(s->types, new_size/2);
-	    s->offsets = g_realloc(s->offsets, new_size);
-	    s->lengths = g_realloc(s->lengths, new_size);
-	    s->sectors = g_realloc(s->sectors, new_size);
-	    s->sectorcounts = g_realloc(s->sectorcounts, new_size);
+            chunk_count = (count - 204) / 40;
+            new_size = sizeof(uint64_t) * (s->n_chunks + chunk_count);
+            s->types = g_realloc(s->types, new_size / 2);
+            s->offsets = g_realloc(s->offsets, new_size);
+            s->lengths = g_realloc(s->lengths, new_size);
+            s->sectors = g_realloc(s->sectors, new_size);
+            s->sectorcounts = g_realloc(s->sectorcounts, new_size);
 
             for (i = s->n_chunks; i < s->n_chunks + chunk_count; i++) {
                 ret = read_uint32(bs, offset, &s->types[i]);
                 if (ret < 0) {
                     goto fail;
                 }
-		offset += 4;
-		if(s->types[i]!=0x80000005 && s->types[i]!=1 && s->types[i]!=2) {
-		    if(s->types[i]==0xffffffff) {
-			last_in_offset = s->offsets[i-1]+s->lengths[i-1];
-			last_out_offset = s->sectors[i-1]+s->sectorcounts[i-1];
-		    }
-		    chunk_count--;
-		    i--;
-		    offset += 36;
-		    continue;
-		}
-		offset += 4;
+                offset += 4;
+                if (s->types[i] != 0x80000005 && s->types[i] != 1 &&
+                    s->types[i] != 2) {
+                    if (s->types[i] == 0xffffffff) {
+                        last_in_offset = s->offsets[i - 1] + s->lengths[i - 1];
+                        last_out_offset = s->sectors[i - 1] +
+                                          s->sectorcounts[i - 1];
+                    }
+                    chunk_count--;
+                    i--;
+                    offset += 36;
+                    continue;
+                }
+                offset += 4;
 
                 ret = read_uint64(bs, offset, &s->sectors[i]);
                 if (ret < 0) {
@@ -218,19 +220,21 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
                 }
                 offset += 8;
 
-		if(s->lengths[i]>max_compressed_size)
-		    max_compressed_size = s->lengths[i];
-		if(s->sectorcounts[i]>max_sectors_per_chunk)
-		    max_sectors_per_chunk = s->sectorcounts[i];
-	    }
-	    s->n_chunks+=chunk_count;
-	}
+                if (s->lengths[i] > max_compressed_size) {
+                    max_compressed_size = s->lengths[i];
+                }
+                if (s->sectorcounts[i] > max_sectors_per_chunk) {
+                    max_sectors_per_chunk = s->sectorcounts[i];
+                }
+            }
+            s->n_chunks += chunk_count;
+        }
     }
 
     /* initialize zlib engine */
-    s->compressed_chunk = g_malloc(max_compressed_size+1);
-    s->uncompressed_chunk = g_malloc(512*max_sectors_per_chunk);
-    if(inflateInit(&s->zstream) != Z_OK) {
+    s->compressed_chunk = g_malloc(max_compressed_size + 1);
+    s->uncompressed_chunk = g_malloc(512 * max_sectors_per_chunk);
+    if (inflateInit(&s->zstream) != Z_OK) {
         ret = -EINVAL;
         goto fail;
     }
@@ -252,27 +256,29 @@ fail:
 }
 
 static inline int is_sector_in_chunk(BDRVDMGState* s,
-		uint32_t chunk_num,int sector_num)
+                uint32_t chunk_num, int sector_num)
 {
-    if(chunk_num>=s->n_chunks || s->sectors[chunk_num]>sector_num ||
-	    s->sectors[chunk_num]+s->sectorcounts[chunk_num]<=sector_num)
-	return 0;
-    else
-	return -1;
+    if (chunk_num >= s->n_chunks || s->sectors[chunk_num] > sector_num ||
+            s->sectors[chunk_num] + s->sectorcounts[chunk_num] <= sector_num) {
+        return 0;
+    } else {
+        return -1;
+    }
 }
 
-static inline uint32_t search_chunk(BDRVDMGState* s,int sector_num)
+static inline uint32_t search_chunk(BDRVDMGState *s, int sector_num)
 {
     /* binary search */
-    uint32_t chunk1=0,chunk2=s->n_chunks,chunk3;
-    while(chunk1!=chunk2) {
-	chunk3 = (chunk1+chunk2)/2;
-	if(s->sectors[chunk3]>sector_num)
-	    chunk2 = chunk3;
-	else if(s->sectors[chunk3]+s->sectorcounts[chunk3]>sector_num)
-	    return chunk3;
-	else
-	    chunk1 = chunk3;
+    uint32_t chunk1 = 0, chunk2 = s->n_chunks, chunk3;
+    while (chunk1 != chunk2) {
+        chunk3 = (chunk1 + chunk2) / 2;
+        if (s->sectors[chunk3] > sector_num) {
+            chunk2 = chunk3;
+        } else if (s->sectors[chunk3] + s->sectorcounts[chunk3] > sector_num) {
+            return chunk3;
+        } else {
+            chunk1 = chunk3;
+        }
     }
     return s->n_chunks; /* error */
 }
@@ -281,54 +287,62 @@ static inline int dmg_read_chunk(BlockDriverState *bs, int sector_num)
 {
     BDRVDMGState *s = bs->opaque;
 
-    if(!is_sector_in_chunk(s,s->current_chunk,sector_num)) {
-	int ret;
-	uint32_t chunk = search_chunk(s,sector_num);
+    if (!is_sector_in_chunk(s, s->current_chunk, sector_num)) {
+        int ret;
+        uint32_t chunk = search_chunk(s, sector_num);
 
-	if(chunk>=s->n_chunks)
-	    return -1;
+        if (chunk >= s->n_chunks) {
+            return -1;
+        }
 
-	s->current_chunk = s->n_chunks;
-	switch(s->types[chunk]) {
-	case 0x80000005: { /* zlib compressed */
-	    int i;
+        s->current_chunk = s->n_chunks;
+        switch (s->types[chunk]) {
+        case 0x80000005: { /* zlib compressed */
+            int i;
 
-	    /* we need to buffer, because only the chunk as whole can be
-	     * inflated. */
-	    i=0;
-	    do {
+            /* we need to buffer, because only the chunk as whole can be
+             * inflated. */
+            i = 0;
+            do {
                 ret = bdrv_pread(bs->file, s->offsets[chunk] + i,
-                                 s->compressed_chunk+i, s->lengths[chunk]-i);
-		if(ret<0 && errno==EINTR)
-		    ret=0;
-		i+=ret;
-	    } while(ret>=0 && ret+i<s->lengths[chunk]);
-
-	    if (ret != s->lengths[chunk])
-		return -1;
-
-	    s->zstream.next_in = s->compressed_chunk;
-	    s->zstream.avail_in = s->lengths[chunk];
-	    s->zstream.next_out = s->uncompressed_chunk;
-	    s->zstream.avail_out = 512*s->sectorcounts[chunk];
-	    ret = inflateReset(&s->zstream);
-	    if(ret != Z_OK)
-		return -1;
-	    ret = inflate(&s->zstream, Z_FINISH);
-	    if(ret != Z_STREAM_END || s->zstream.total_out != 512*s->sectorcounts[chunk])
-		return -1;
-	    break; }
-	case 1: /* copy */
-	    ret = bdrv_pread(bs->file, s->offsets[chunk],
+                                 s->compressed_chunk + i,
+                                 s->lengths[chunk] - i);
+                if (ret < 0 && errno == EINTR) {
+                    ret = 0;
+                }
+                i += ret;
+            } while (ret >= 0 && ret + i < s->lengths[chunk]);
+
+            if (ret != s->lengths[chunk]) {
+                return -1;
+            }
+
+            s->zstream.next_in = s->compressed_chunk;
+            s->zstream.avail_in = s->lengths[chunk];
+            s->zstream.next_out = s->uncompressed_chunk;
+            s->zstream.avail_out = 512 * s->sectorcounts[chunk];
+            ret = inflateReset(&s->zstream);
+            if (ret != Z_OK) {
+                return -1;
+            }
+            ret = inflate(&s->zstream, Z_FINISH);
+            if (ret != Z_STREAM_END ||
+                s->zstream.total_out != 512 * s->sectorcounts[chunk]) {
+                return -1;
+            }
+            break; }
+        case 1: /* copy */
+            ret = bdrv_pread(bs->file, s->offsets[chunk],
                              s->uncompressed_chunk, s->lengths[chunk]);
-	    if (ret != s->lengths[chunk])
-		return -1;
-	    break;
-	case 2: /* zero */
-	    memset(s->uncompressed_chunk, 0, 512*s->sectorcounts[chunk]);
-	    break;
-	}
-	s->current_chunk = chunk;
+            if (ret != s->lengths[chunk]) {
+                return -1;
+            }
+            break;
+        case 2: /* zero */
+            memset(s->uncompressed_chunk, 0, 512 * s->sectorcounts[chunk]);
+            break;
+        }
+        s->current_chunk = chunk;
     }
     return 0;
 }
@@ -339,12 +353,14 @@ static int dmg_read(BlockDriverState *bs, int64_t sector_num,
     BDRVDMGState *s = bs->opaque;
     int i;
 
-    for(i=0;i<nb_sectors;i++) {
-	uint32_t sector_offset_in_chunk;
-	if(dmg_read_chunk(bs, sector_num+i) != 0)
-	    return -1;
-	sector_offset_in_chunk = sector_num+i-s->sectors[s->current_chunk];
-	memcpy(buf+i*512,s->uncompressed_chunk+sector_offset_in_chunk*512,512);
+    for (i = 0; i < nb_sectors; i++) {
+        uint32_t sector_offset_in_chunk;
+        if (dmg_read_chunk(bs, sector_num + i) != 0) {
+            return -1;
+        }
+        sector_offset_in_chunk = sector_num + i - s->sectors[s->current_chunk];
+        memcpy(buf + i * 512,
+               s->uncompressed_chunk + sector_offset_in_chunk * 512, 512);
     }
     return 0;
 }
@@ -376,12 +392,12 @@ static void dmg_close(BlockDriverState *bs)
 }
 
 static BlockDriver bdrv_dmg = {
-    .format_name	= "dmg",
-    .instance_size	= sizeof(BDRVDMGState),
-    .bdrv_probe		= dmg_probe,
-    .bdrv_open		= dmg_open,
-    .bdrv_read          = dmg_co_read,
-    .bdrv_close		= dmg_close,
+    .format_name    = "dmg",
+    .instance_size  = sizeof(BDRVDMGState),
+    .bdrv_probe     = dmg_probe,
+    .bdrv_open      = dmg_open,
+    .bdrv_read      = dmg_co_read,
+    .bdrv_close     = dmg_close,
 };
 
 static void bdrv_dmg_init(void)
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 36/51] dmg: prevent out-of-bounds array access on terminator

[Stefan Hajnoczi]

When a terminator is reached the base for offsets and sectors is stored.
The following records that are processed will use this base value.

If the first record we encounter is a terminator, then calculating the
base values would result in out-of-bounds array accesses.  Don't do
that.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/dmg.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/dmg.c b/block/dmg.c
index be2f26e..f4f3e8e 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -182,7 +182,7 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
                 offset += 4;
                 if (s->types[i] != 0x80000005 && s->types[i] != 1 &&
                     s->types[i] != 2) {
-                    if (s->types[i] == 0xffffffff) {
+                    if (s->types[i] == 0xffffffff && i > 0) {
                         last_in_offset = s->offsets[i - 1] + s->lengths[i - 1];
                         last_out_offset = s->sectors[i - 1] +
                                           s->sectorcounts[i - 1];
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 37/51] dmg: drop broken bdrv_pread() loop

[Stefan Hajnoczi]

It is not necessary to check errno for EINTR and the block layer does
not produce short reads.  Therefore we can drop the loop that attempts
to read a compressed chunk.

The loop is buggy because it incorrectly adds the transferred bytes
twice:

  do {
      ret = bdrv_pread(...);
      i += ret;
  } while (ret >= 0 && ret + i < s->lengths[chunk]);

Luckily we can drop the loop completely and perform a single
bdrv_pread().

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/dmg.c | 15 ++-------------
 1 file changed, 2 insertions(+), 13 deletions(-)

diff --git a/block/dmg.c b/block/dmg.c
index f4f3e8e..1cc5426 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -298,21 +298,10 @@ static inline int dmg_read_chunk(BlockDriverState *bs, int sector_num)
         s->current_chunk = s->n_chunks;
         switch (s->types[chunk]) {
         case 0x80000005: { /* zlib compressed */
-            int i;
-
             /* we need to buffer, because only the chunk as whole can be
              * inflated. */
-            i = 0;
-            do {
-                ret = bdrv_pread(bs->file, s->offsets[chunk] + i,
-                                 s->compressed_chunk + i,
-                                 s->lengths[chunk] - i);
-                if (ret < 0 && errno == EINTR) {
-                    ret = 0;
-                }
-                i += ret;
-            } while (ret >= 0 && ret + i < s->lengths[chunk]);
-
+            ret = bdrv_pread(bs->file, s->offsets[chunk],
+                             s->compressed_chunk, s->lengths[chunk]);
             if (ret != s->lengths[chunk]) {
                 return -1;
             }
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 38/51] dmg: use appropriate types when reading chunks

[Stefan Hajnoczi]

Use the right types instead of signed int:

  size_t new_size;

  This is a byte count for g_realloc() that is calculated from uint32_t
  and size_t values.

  uint32_t chunk_count;

  Use the same type as s->n_chunks, which is used together with
  chunk_count.

This patch is a cleanup and does not fix bugs.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/dmg.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/block/dmg.c b/block/dmg.c
index 1cc5426..f98c94d 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -161,7 +161,8 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
         }
 
         if (type == 0x6d697368 && count >= 244) {
-            int new_size, chunk_count;
+            size_t new_size;
+            uint32_t chunk_count;
 
             offset += 4;
             offset += 200;
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 39/51] dmg: sanitize chunk length and sectorcount (CVE-2014-0145)

[Stefan Hajnoczi]

Chunk length and sectorcount are used for decompression buffers as well
as the bdrv_pread() count argument.  Ensure that they have reasonable
values so neither memory allocation nor conversion from uint64_t to int
will cause problems.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/dmg.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/block/dmg.c b/block/dmg.c
index f98c94d..ad253fe 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -27,6 +27,14 @@
 #include "qemu/module.h"
 #include <zlib.h>
 
+enum {
+    /* Limit chunk sizes to prevent unreasonable amounts of memory being used
+     * or truncating when converting to 32-bit types
+     */
+    DMG_LENGTHS_MAX = 64 * 1024 * 1024, /* 64 MB */
+    DMG_SECTORCOUNTS_MAX = DMG_LENGTHS_MAX / 512,
+};
+
 typedef struct BDRVDMGState {
     CoMutex lock;
     /* each chunk contains a certain number of sectors,
@@ -208,6 +216,14 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
                 }
                 offset += 8;
 
+                if (s->sectorcounts[i] > DMG_SECTORCOUNTS_MAX) {
+                    error_report("sector count %" PRIu64 " for chunk %u is "
+                                 "larger than max (%u)",
+                                 s->sectorcounts[i], i, DMG_SECTORCOUNTS_MAX);
+                    ret = -EINVAL;
+                    goto fail;
+                }
+
                 ret = read_uint64(bs, offset, &s->offsets[i]);
                 if (ret < 0) {
                     goto fail;
@@ -221,6 +237,14 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
                 }
                 offset += 8;
 
+                if (s->lengths[i] > DMG_LENGTHS_MAX) {
+                    error_report("length %" PRIu64 " for chunk %u is larger "
+                                 "than max (%u)",
+                                 s->lengths[i], i, DMG_LENGTHS_MAX);
+                    ret = -EINVAL;
+                    goto fail;
+                }
+
                 if (s->lengths[i] > max_compressed_size) {
                     max_compressed_size = s->lengths[i];
                 }
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 40/51] dmg: use uint64_t consistently for sectors and lengths

[Stefan Hajnoczi]

The DMG metadata is stored as uint64_t, so use the same type for
sector_num.  int was a particularly poor choice since it is only 32-bit
and would truncate large values.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/dmg.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/block/dmg.c b/block/dmg.c
index ad253fe..be0ee33 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -281,7 +281,7 @@ fail:
 }
 
 static inline int is_sector_in_chunk(BDRVDMGState* s,
-                uint32_t chunk_num, int sector_num)
+                uint32_t chunk_num, uint64_t sector_num)
 {
     if (chunk_num >= s->n_chunks || s->sectors[chunk_num] > sector_num ||
             s->sectors[chunk_num] + s->sectorcounts[chunk_num] <= sector_num) {
@@ -291,7 +291,7 @@ static inline int is_sector_in_chunk(BDRVDMGState* s,
     }
 }
 
-static inline uint32_t search_chunk(BDRVDMGState *s, int sector_num)
+static inline uint32_t search_chunk(BDRVDMGState *s, uint64_t sector_num)
 {
     /* binary search */
     uint32_t chunk1 = 0, chunk2 = s->n_chunks, chunk3;
@@ -308,7 +308,7 @@ static inline uint32_t search_chunk(BDRVDMGState *s, int sector_num)
     return s->n_chunks; /* error */
 }
 
-static inline int dmg_read_chunk(BlockDriverState *bs, int sector_num)
+static inline int dmg_read_chunk(BlockDriverState *bs, uint64_t sector_num)
 {
     BDRVDMGState *s = bs->opaque;
 
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 41/51] dmg: prevent chunk buffer overflow (CVE-2014-0145)

[Stefan Hajnoczi]

Both compressed and uncompressed I/O is buffered.  dmg_open() calculates
the maximum buffer size needed from the metadata in the image file.

There is currently a buffer overflow since ->lengths[] is accounted
against the maximum compressed buffer size but actually uses the
uncompressed buffer:

  switch (s->types[chunk]) {
  case 1: /* copy */
      ret = bdrv_pread(bs->file, s->offsets[chunk],
                       s->uncompressed_chunk, s->lengths[chunk]);

We must account against the maximum uncompressed buffer size for type=1
chunks.

This patch fixes the maximum buffer size calculation to take into
account the chunk type.  It is critical that we update the correct
maximum since there are two buffers ->compressed_chunk and
->uncompressed_chunk.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/dmg.c | 39 +++++++++++++++++++++++++++++++++------
 1 file changed, 33 insertions(+), 6 deletions(-)

diff --git a/block/dmg.c b/block/dmg.c
index be0ee33..856402e 100644
--- a/block/dmg.c
+++ b/block/dmg.c
@@ -100,6 +100,37 @@ static int read_uint32(BlockDriverState *bs, int64_t offset, uint32_t *result)
     return 0;
 }
 
+/* Increase max chunk sizes, if necessary.  This function is used to calculate
+ * the buffer sizes needed for compressed/uncompressed chunk I/O.
+ */
+static void update_max_chunk_size(BDRVDMGState *s, uint32_t chunk,
+                                  uint32_t *max_compressed_size,
+                                  uint32_t *max_sectors_per_chunk)
+{
+    uint32_t compressed_size = 0;
+    uint32_t uncompressed_sectors = 0;
+
+    switch (s->types[chunk]) {
+    case 0x80000005: /* zlib compressed */
+        compressed_size = s->lengths[chunk];
+        uncompressed_sectors = s->sectorcounts[chunk];
+        break;
+    case 1: /* copy */
+        uncompressed_sectors = (s->lengths[chunk] + 511) / 512;
+        break;
+    case 2: /* zero */
+        uncompressed_sectors = s->sectorcounts[chunk];
+        break;
+    }
+
+    if (compressed_size > *max_compressed_size) {
+        *max_compressed_size = compressed_size;
+    }
+    if (uncompressed_sectors > *max_sectors_per_chunk) {
+        *max_sectors_per_chunk = uncompressed_sectors;
+    }
+}
+
 static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
                     Error **errp)
 {
@@ -245,12 +276,8 @@ static int dmg_open(BlockDriverState *bs, QDict *options, int flags,
                     goto fail;
                 }
 
-                if (s->lengths[i] > max_compressed_size) {
-                    max_compressed_size = s->lengths[i];
-                }
-                if (s->sectorcounts[i] > max_sectors_per_chunk) {
-                    max_sectors_per_chunk = s->sectorcounts[i];
-                }
+                update_max_chunk_size(s, i, &max_compressed_size,
+                                      &max_sectors_per_chunk);
             }
             s->n_chunks += chunk_count;
         }
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 42/51] block: vdi bounds check qemu-io tests

[Stefan Hajnoczi]

From: Jeff Cody <jcody@redhat.com>

This test checks for proper bounds checking of some VDI input
headers.  The following is checked:

1. Max image size (1024TB) with the appropriate Blocks In Image
   value (0x3fffffff) is detected as valid.

2. Image size exceeding max (1024TB) is seen as invalid

3. Valid image size but with Blocks In Image value that is too
   small fails

4. Blocks In Image size exceeding max (0x3fffffff) is seen as invalid

5. 64MB image, with 64 Blocks In Image, and 1MB Block Size is seen
   as valid

6. Block Size < 1MB not supported

7. Block Size > 1MB not supported

[Max Reitz <mreitz@redhat.com> pointed out that "1MB + 1" in the test
case is wrong.  Change to "1MB + 64KB" to match the 0x110000 value.
--Stefan]

Signed-off-by: Jeff Cody <jcody@redhat.com>
Reviewed-by: Stefan Hajnoczi <stefanha@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 tests/qemu-iotests/084     | 104 +++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/084.out |  33 ++++++++++++++
 tests/qemu-iotests/group   |   1 +
 3 files changed, 138 insertions(+)
 create mode 100755 tests/qemu-iotests/084
 create mode 100644 tests/qemu-iotests/084.out

diff --git a/tests/qemu-iotests/084 b/tests/qemu-iotests/084
new file mode 100755
index 0000000..cb4d7b7
--- /dev/null
+++ b/tests/qemu-iotests/084
@@ -0,0 +1,104 @@
+#!/bin/bash
+#
+# Test case for VDI header corruption; image too large, and too many blocks
+#
+# Copyright (C) 2013 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=jcody@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 vdi-specific header fields
+_supported_fmt vdi
+_supported_proto generic
+_supported_os Linux
+
+ds_offset=368  # disk image size field offset
+bs_offset=376  # block size field offset
+bii_offset=384 # block in image field offset
+
+echo
+echo "=== Testing image size bounds ==="
+echo
+_make_test_img 64M
+
+# check for image size too large
+# poke max image size, and appropriate blocks_in_image value
+echo "Test 1: Maximum size (1024 TB):"
+poke_file "$TEST_IMG" "$ds_offset" "\x00\x00\xf0\xff\xff\xff\x03\x00"
+poke_file "$TEST_IMG" "$bii_offset" "\xff\xff\xff\x3f"
+_img_info
+
+echo
+echo "Test 2: Size too large (1024TB + 1)"
+# This should be too large (-EINVAL):
+poke_file "$TEST_IMG" "$ds_offset" "\x00\x00\xf1\xff\xff\xff\x03\x00"
+_img_info
+
+echo
+echo "Test 3: Size valid (64M), but Blocks In Image too small (63)"
+# This sets the size to 64M, but with a blocks_in_image size that is
+# too small
+poke_file "$TEST_IMG" "$ds_offset" "\x00\x00\x00\x04\x00\x00\x00\x00"
+# For a 64M image, we would need a blocks_in_image value of at least 64,
+# so 63 should be too small and give us -ENOTSUP
+poke_file "$TEST_IMG" "$bii_offset" "\x3f\x00\x00\x00"
+_img_info
+
+echo
+echo "Test 4: Size valid (64M), but Blocks In Image exceeds max allowed"
+# Now check the bounds of blocks_in_image - 0x3fffffff should be the max
+# value here, and we should get -ENOTSUP
+poke_file "$TEST_IMG" "$bii_offset" "\x00\x00\x00\x40"
+_img_info
+
+# Finally, 1MB is the only block size supported.  Verify that
+# a value != 1MB results in error, both smaller and larger
+echo
+echo "Test 5: Valid Image: 64MB, Blocks In Image 64, Block Size 1MB"
+poke_file "$TEST_IMG" "$bii_offset" "\x40\x00\x00\x00" # reset bii to valid
+poke_file "$TEST_IMG" "$bs_offset" "\x00\x00\x10\x00"  # valid
+_img_info
+echo
+echo "Test 6: Block Size != 1MB; too small test (1MB - 1)"
+poke_file "$TEST_IMG" "$bs_offset" "\xff\xff\x0f\x00"  # invalid (too small)
+_img_info
+echo
+echo "Test 7: Block Size != 1MB; too large test (1MB + 64KB)"
+poke_file "$TEST_IMG" "$bs_offset" "\x00\x00\x11\x00"  # invalid (too large)
+_img_info
+# success, all done
+echo
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/084.out b/tests/qemu-iotests/084.out
new file mode 100644
index 0000000..e681924
--- /dev/null
+++ b/tests/qemu-iotests/084.out
@@ -0,0 +1,33 @@
+QA output created by 084
+
+=== Testing image size bounds ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+Test 1: Maximum size (1024 TB):
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 1024T (1125899905794048 bytes)
+cluster_size: 1048576
+
+Test 2: Size too large (1024TB + 1)
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': Unsupported VDI image size (size is 0x3fffffff10000, max supported is 0x3fffffff00000)
+
+Test 3: Size valid (64M), but Blocks In Image too small (63)
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': unsupported VDI image (disk size 67108864, image bitmap has room for 66060288)
+
+Test 4: Size valid (64M), but Blocks In Image exceeds max allowed
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': unsupported VDI image (too many blocks 1073741824, max is 1073741823)
+
+Test 5: Valid Image: 64MB, Blocks In Image 64, Block Size 1MB
+image: TEST_DIR/t.IMGFMT
+file format: IMGFMT
+virtual size: 64M (67108864 bytes)
+cluster_size: 1048576
+
+Test 6: Block Size != 1MB; too small test (1MB - 1)
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': unsupported VDI image (block size 1048575 is not 1048576)
+
+Test 7: Block Size != 1MB; too large test (1MB + 64KB)
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': unsupported VDI image (block size 1114112 is not 1048576)
+
+*** done
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index ed44f35..c51640c 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -89,6 +89,7 @@
 081 rw auto
 082 rw auto quick
 083 rw auto
+084 img auto
 085 rw auto
 086 rw auto quick
 087 rw auto
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 43/51] block: Limit request size (CVE-2014-0143)

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

Limiting the size of a single request to INT_MAX not only fixes a
direct integer overflow in bdrv_check_request() (which would only
trigger bad behaviour with ridiculously huge images, as in close to
2^64 bytes), but can also prevent overflows in all block drivers.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/block.c b/block.c
index acb70fd..7a90a1b 100644
--- a/block.c
+++ b/block.c
@@ -2588,6 +2588,10 @@ static int bdrv_check_byte_request(BlockDriverState *bs, int64_t offset,
 static int bdrv_check_request(BlockDriverState *bs, int64_t sector_num,
                               int nb_sectors)
 {
+    if (nb_sectors > INT_MAX / BDRV_SECTOR_SIZE) {
+        return -EIO;
+    }
+
     return bdrv_check_byte_request(bs, sector_num * BDRV_SECTOR_SIZE,
                                    nb_sectors * BDRV_SECTOR_SIZE);
 }
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 44/51] qcow2: Fix copy_sectors() with VM state

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

bs->total_sectors is not the highest possible sector number that could
be involved in a copy on write operation: VM state is after the end of
the virtual disk. This resulted in wrong values for the number of
sectors to be copied (n).

The code that checks for the end of the image isn't required any more
because the code hasn't been calling the block layer's bdrv_read() for a
long time; instead, it directly calls qcow2_readv(), which doesn't error
out on VM state sector numbers.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2-cluster.c      |  9 ---------
 tests/qemu-iotests/029     | 22 ++++++++++++++++++++--
 tests/qemu-iotests/029.out | 13 +++++++++++++
 3 files changed, 33 insertions(+), 11 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index 242e1f8..60a6910 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -359,15 +359,6 @@ static int coroutine_fn copy_sectors(BlockDriverState *bs,
     struct iovec iov;
     int n, ret;
 
-    /*
-     * If this is the last cluster and it is only partially used, we must only
-     * copy until the end of the image, or bdrv_check_request will fail for the
-     * bdrv_read/write calls below.
-     */
-    if (start_sect + n_end > bs->total_sectors) {
-        n_end = bs->total_sectors - start_sect;
-    }
-
     n = n_end - n_start;
     if (n <= 0) {
         return 0;
diff --git a/tests/qemu-iotests/029 b/tests/qemu-iotests/029
index b424726..567e071 100755
--- a/tests/qemu-iotests/029
+++ b/tests/qemu-iotests/029
@@ -1,7 +1,6 @@
 #!/bin/bash
 #
-# Test loading internal snapshots where the L1 table of the snapshot
-# is smaller than the current L1 table.
+# qcow2 internal snapshots/VM state tests
 #
 # Copyright (C) 2011 Red Hat, Inc.
 #
@@ -45,6 +44,11 @@ _supported_fmt qcow2
 _supported_proto generic
 _supported_os Linux
 
+echo
+echo Test loading internal snapshots where the L1 table of the snapshot
+echo is smaller than the current L1 table.
+echo
+
 CLUSTER_SIZE=65536
 _make_test_img 64M
 $QEMU_IMG snapshot -c foo "$TEST_IMG"
@@ -59,6 +63,20 @@ $QEMU_IO -c 'write -b 0 4M' "$TEST_IMG" | _filter_qemu_io
 $QEMU_IMG snapshot -a foo "$TEST_IMG"
 _check_test_img
 
+
+echo
+echo Try using a huge VM state
+echo
+
+CLUSTER_SIZE=65536
+_make_test_img 64M
+{ $QEMU_IO -c "write -b -P 0x11 1T 4k" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+{ $QEMU_IMG snapshot -c foo $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+{ $QEMU_IMG snapshot -a foo $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+{ $QEMU_IO -c "read -b -P 0x11 1T 4k" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+_check_test_img
+
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/029.out b/tests/qemu-iotests/029.out
index 0eedb3a..9029698 100644
--- a/tests/qemu-iotests/029.out
+++ b/tests/qemu-iotests/029.out
@@ -1,4 +1,8 @@
 QA output created by 029
+
+Test loading internal snapshots where the L1 table of the snapshot
+is smaller than the current L1 table.
+
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
 wrote 4096/4096 bytes at offset 0
 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
@@ -7,4 +11,13 @@ Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=16777216
 wrote 4194304/4194304 bytes at offset 0
 4 MiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 No errors were found on the image.
+
+Try using a huge VM state
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+wrote 4096/4096 bytes at offset 1099511627776
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+read 4096/4096 bytes at offset 1099511627776
+4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+No errors were found on the image.
 *** done
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 45/51] qcow2: Fix NULL dereference in qcow2_open() error path (CVE-2014-0146)

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

The qcow2 code assumes that s->snapshots is non-NULL if s->nb_snapshots
!= 0. By having the initialisation of both fields separated in
qcow2_open(), any error occuring in between would cause the error path
to dereference NULL in qcow2_free_snapshots() if the image had any
snapshots.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2.c              | 7 ++++---
 tests/qemu-iotests/080     | 7 +++++++
 tests/qemu-iotests/080.out | 4 ++++
 3 files changed, 15 insertions(+), 3 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index b64564d..be48a27 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -637,9 +637,6 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
         goto fail;
     }
 
-    s->snapshots_offset = header.snapshots_offset;
-    s->nb_snapshots = header.nb_snapshots;
-
     /* read the level 1 table */
     if (header.l1_size > 0x2000000) {
         /* 32 MB L1 table is enough for 2 PB images at 64k cluster size
@@ -734,6 +731,10 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
         bs->backing_file[len] = '\0';
     }
 
+    /* Internal snapshots */
+    s->snapshots_offset = header.snapshots_offset;
+    s->nb_snapshots = header.nb_snapshots;
+
     ret = qcow2_read_snapshots(bs);
     if (ret < 0) {
         error_setg_errno(errp, -ret, "Could not read snapshots");
diff --git a/tests/qemu-iotests/080 b/tests/qemu-iotests/080
index 56f8903..59e7a44 100755
--- a/tests/qemu-iotests/080
+++ b/tests/qemu-iotests/080
@@ -139,6 +139,13 @@ poke_file "$TEST_IMG" "$offset_l1_size" "\x00\x00\x00\x01"
 { $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
 
 echo
+echo "== Invalid L1 table (with internal snapshot in the image) =="
+_make_test_img 64M
+{ $QEMU_IMG snapshot -c foo $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+poke_file "$TEST_IMG" "$offset_l1_size" "\x00\x00\x00\x00"
+_img_info
+
+echo
 echo "== Invalid backing file size =="
 _make_test_img 64M
 poke_file "$TEST_IMG" "$offset_backing_file_offset" "\x00\x00\x00\x00\x00\x00\x10\x00"
diff --git a/tests/qemu-iotests/080.out b/tests/qemu-iotests/080.out
index 303d6c3..4d84fbf 100644
--- a/tests/qemu-iotests/080.out
+++ b/tests/qemu-iotests/080.out
@@ -59,6 +59,10 @@ no file open, try 'help open'
 qemu-io: can't open device TEST_DIR/t.qcow2: Invalid L1 table offset
 no file open, try 'help open'
 
+== Invalid L1 table (with internal snapshot in the image) ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': L1 table is too small
+
 == Invalid backing file size ==
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
 qemu-io: can't open device TEST_DIR/t.qcow2: Backing file name too long
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 46/51] qcow2: Fix L1 allocation size in qcow2_snapshot_load_tmp() (CVE-2014-0145)

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

For the L1 table to loaded for an internal snapshot, the code allocated
only enough memory to hold the currently active L1 table. If the
snapshot's L1 table is actually larger than the current one, this leads
to a buffer overflow.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2-snapshot.c     |  2 +-
 tests/qemu-iotests/029     | 18 +++++++++++++++++-
 tests/qemu-iotests/029.out |  4 ++++
 3 files changed, 22 insertions(+), 2 deletions(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 87fbfe1..715168e 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -680,7 +680,7 @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs,
     sn = &s->snapshots[snapshot_index];
 
     /* Allocate and read in the snapshot's L1 table */
-    new_l1_bytes = s->l1_size * sizeof(uint64_t);
+    new_l1_bytes = sn->l1_size * sizeof(uint64_t);
     new_l1_table = g_malloc0(align_offset(new_l1_bytes, 512));
 
     ret = bdrv_pread(bs->file, sn->l1_table_offset, new_l1_table, new_l1_bytes);
diff --git a/tests/qemu-iotests/029 b/tests/qemu-iotests/029
index 567e071..fa46ace 100755
--- a/tests/qemu-iotests/029
+++ b/tests/qemu-iotests/029
@@ -30,7 +30,8 @@ status=1	# failure is the default!
 
 _cleanup()
 {
-	_cleanup_test_img
+    rm -f $TEST_IMG.snap
+    _cleanup_test_img
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
@@ -44,6 +45,9 @@ _supported_fmt qcow2
 _supported_proto generic
 _supported_os Linux
 
+offset_size=24
+offset_l1_size=36
+
 echo
 echo Test loading internal snapshots where the L1 table of the snapshot
 echo is smaller than the current L1 table.
@@ -77,6 +81,18 @@ _make_test_img 64M
 _check_test_img
 
 
+echo
+echo "qcow2_snapshot_load_tmp() should take the L1 size from the snapshot"
+echo
+
+CLUSTER_SIZE=512
+_make_test_img 64M
+{ $QEMU_IMG snapshot -c foo $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+poke_file "$TEST_IMG" "$offset_size" "\x00\x00\x00\x00\x00\x00\x02\x00"
+poke_file "$TEST_IMG" "$offset_l1_size" "\x00\x00\x00\x01"
+{ $QEMU_IMG convert -s foo $TEST_IMG $TEST_IMG.snap; } 2>&1 | _filter_qemu_io | _filter_testdir
+
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/029.out b/tests/qemu-iotests/029.out
index 9029698..ce0e64d 100644
--- a/tests/qemu-iotests/029.out
+++ b/tests/qemu-iotests/029.out
@@ -20,4 +20,8 @@ wrote 4096/4096 bytes at offset 1099511627776
 read 4096/4096 bytes at offset 1099511627776
 4 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 No errors were found on the image.
+
+qcow2_snapshot_load_tmp() should take the L1 size from the snapshot
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
 *** done
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 47/51] qcow2: Check maximum L1 size in qcow2_snapshot_load_tmp() (CVE-2014-0143)

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

This avoids an unbounded allocation.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2-snapshot.c     |  4 ++++
 block/qcow2.c              |  4 +---
 block/qcow2.h              |  4 ++++
 tests/qemu-iotests/080     | 15 ++++++++++++++-
 tests/qemu-iotests/080.out |  6 ++++++
 5 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 715168e..5db4f30 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -680,6 +680,10 @@ int qcow2_snapshot_load_tmp(BlockDriverState *bs,
     sn = &s->snapshots[snapshot_index];
 
     /* Allocate and read in the snapshot's L1 table */
+    if (sn->l1_size > QCOW_MAX_L1_SIZE) {
+        error_setg(errp, "Snapshot L1 table too large");
+        return -EFBIG;
+    }
     new_l1_bytes = sn->l1_size * sizeof(uint64_t);
     new_l1_table = g_malloc0(align_offset(new_l1_bytes, 512));
 
diff --git a/block/qcow2.c b/block/qcow2.c
index be48a27..bb6000f 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -638,9 +638,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags,
     }
 
     /* read the level 1 table */
-    if (header.l1_size > 0x2000000) {
-        /* 32 MB L1 table is enough for 2 PB images at 64k cluster size
-         * (128 GB for 512 byte clusters, 2 EB for 2 MB clusters) */
+    if (header.l1_size > QCOW_MAX_L1_SIZE) {
         error_setg(errp, "Active L1 table too large");
         ret = -EFBIG;
         goto fail;
diff --git a/block/qcow2.h b/block/qcow2.h
index 3649465..f28e7d9 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -44,6 +44,10 @@
  * (128 GB for 512 byte clusters, 2 EB for 2 MB clusters) */
 #define QCOW_MAX_REFTABLE_SIZE 0x800000
 
+/* 32 MB L1 table is enough for 2 PB images at 64k cluster size
+ * (128 GB for 512 byte clusters, 2 EB for 2 MB clusters) */
+#define QCOW_MAX_L1_SIZE 0x2000000
+
 /* indicate that the refcount of the referenced cluster is exactly one. */
 #define QCOW_OFLAG_COPIED     (1ULL << 63)
 /* indicate that the cluster is compressed (they never have the copied flag) */
diff --git a/tests/qemu-iotests/080 b/tests/qemu-iotests/080
index 59e7a44..6b3a3e7 100755
--- a/tests/qemu-iotests/080
+++ b/tests/qemu-iotests/080
@@ -30,7 +30,8 @@ status=1	# failure is the default!
 
 _cleanup()
 {
-	_cleanup_test_img
+    rm -f $TEST_IMG.snap
+    _cleanup_test_img
 }
 trap "_cleanup; exit \$status" 0 1 2 3 15
 
@@ -58,6 +59,10 @@ offset_ext_size=$((header_size + 4))
 
 offset_l2_table_0=$((0x40000))
 
+offset_snap1=$((0x70000))
+offset_snap1_l1_offset=$((offset_snap1 + 0))
+offset_snap1_l1_size=$((offset_snap1 + 8))
+
 echo
 echo "== Huge header size =="
 _make_test_img 64M
@@ -161,6 +166,14 @@ poke_file "$TEST_IMG" "$offset_l2_table_0" "\xbf\xff\xff\xff\xff\xff\x00\x00"
 poke_file "$TEST_IMG" "$offset_l2_table_0" "\x80\x00\x00\xff\xff\xff\x00\x00"
 { $QEMU_IMG snapshot -c test $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
 
+echo
+echo "== Invalid snapshot L1 table =="
+_make_test_img 64M
+{ $QEMU_IO -c "write 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+{ $QEMU_IMG snapshot -c test $TEST_IMG; } 2>&1 | _filter_testdir
+poke_file "$TEST_IMG" "$offset_snap1_l1_size" "\x10\x00\x00\x00"
+{ $QEMU_IMG convert -s test $TEST_IMG $TEST_IMG.snap; } 2>&1 | _filter_testdir
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/080.out b/tests/qemu-iotests/080.out
index 4d84fbf..f7a943c 100644
--- a/tests/qemu-iotests/080.out
+++ b/tests/qemu-iotests/080.out
@@ -74,4 +74,10 @@ wrote 512/512 bytes at offset 0
 512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
 qemu-img: Could not create snapshot 'test': -27 (File too large)
 qemu-img: Could not create snapshot 'test': -11 (Resource temporarily unavailable)
+
+== Invalid snapshot L1 table ==
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+wrote 512/512 bytes at offset 0
+512 bytes, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+qemu-img: Failed to load snapshot: Snapshot L1 table too large
 *** done
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 48/51] qcow2: Limit snapshot table size

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

Even with a limit of 64k snapshots, each snapshot could have a filename
and an ID with up to 64k, which would still lead to pretty large
allocations, which could potentially lead to qemu aborting. Limit the
total size of the snapshot table to an average of 1k per entry when
the limit of 64k snapshots is fully used. This should be plenty for any
reasonable user.

This also fixes potential integer overflows of s->snapshot_size.

Suggested-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2-snapshot.c | 15 ++++++++++++++-
 block/qcow2.h          |  4 ++++
 2 files changed, 18 insertions(+), 1 deletion(-)

diff --git a/block/qcow2-snapshot.c b/block/qcow2-snapshot.c
index 5db4f30..0aa9def 100644
--- a/block/qcow2-snapshot.c
+++ b/block/qcow2-snapshot.c
@@ -116,8 +116,14 @@ int qcow2_read_snapshots(BlockDriverState *bs)
         }
         offset += name_size;
         sn->name[name_size] = '\0';
+
+        if (offset - s->snapshots_offset > QCOW_MAX_SNAPSHOTS_SIZE) {
+            ret = -EFBIG;
+            goto fail;
+        }
     }
 
+    assert(offset - s->snapshots_offset <= INT_MAX);
     s->snapshots_size = offset - s->snapshots_offset;
     return 0;
 
@@ -138,7 +144,7 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
         uint32_t nb_snapshots;
         uint64_t snapshots_offset;
     } QEMU_PACKED header_data;
-    int64_t offset, snapshots_offset;
+    int64_t offset, snapshots_offset = 0;
     int ret;
 
     /* compute the size of the snapshots */
@@ -150,7 +156,14 @@ static int qcow2_write_snapshots(BlockDriverState *bs)
         offset += sizeof(extra);
         offset += strlen(sn->id_str);
         offset += strlen(sn->name);
+
+        if (offset > QCOW_MAX_SNAPSHOTS_SIZE) {
+            ret = -EFBIG;
+            goto fail;
+        }
     }
+
+    assert(offset <= INT_MAX);
     snapshots_size = offset;
 
     /* Allocate space for the new snapshot list */
diff --git a/block/qcow2.h b/block/qcow2.h
index f28e7d9..b49424b 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -48,6 +48,10 @@
  * (128 GB for 512 byte clusters, 2 EB for 2 MB clusters) */
 #define QCOW_MAX_L1_SIZE 0x2000000
 
+/* Allow for an average of 1k per snapshot table entry, should be plenty of
+ * space for snapshot names and IDs */
+#define QCOW_MAX_SNAPSHOTS_SIZE (1024 * QCOW_MAX_SNAPSHOTS)
+
 /* indicate that the refcount of the referenced cluster is exactly one. */
 #define QCOW_OFLAG_COPIED     (1ULL << 63)
 /* indicate that the cluster is compressed (they never have the copied flag) */
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 49/51] parallels: Fix catalog size integer overflow (CVE-2014-0143)

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

The first test case would cause a huge memory allocation, leading to a
qemu abort; the second one to a too small malloc() for the catalog
(smaller than s->catalog_size), which causes a read-only out-of-bounds
array access and on big endian hosts an endianess conversion for an
undefined memory area.

The sample image used here is not an original Parallels image. It was
created using an hexeditor on the basis of the struct that qemu uses.
Good enough for trying to crash the driver, but not for ensuring
compatibility.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/parallels.c                                  |   7 ++-
 tests/qemu-iotests/076                             |  69 +++++++++++++++++++++
 tests/qemu-iotests/076.out                         |  14 +++++
 tests/qemu-iotests/common                          |   7 +++
 tests/qemu-iotests/group                           |   1 +
 .../qemu-iotests/sample_images/fake.parallels.bz2  | Bin 0 -> 141 bytes
 6 files changed, 97 insertions(+), 1 deletion(-)
 create mode 100755 tests/qemu-iotests/076
 create mode 100644 tests/qemu-iotests/076.out
 create mode 100644 tests/qemu-iotests/sample_images/fake.parallels.bz2

diff --git a/block/parallels.c b/block/parallels.c
index 3f588f5..fe47ecb 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -49,7 +49,7 @@ typedef struct BDRVParallelsState {
     CoMutex lock;
 
     uint32_t *catalog_bitmap;
-    int catalog_size;
+    unsigned int catalog_size;
 
     int tracks;
 } BDRVParallelsState;
@@ -95,6 +95,11 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
     s->tracks = le32_to_cpu(ph.tracks);
 
     s->catalog_size = le32_to_cpu(ph.catalog_entries);
+    if (s->catalog_size > INT_MAX / 4) {
+        error_setg(errp, "Catalog too large");
+        ret = -EFBIG;
+        goto fail;
+    }
     s->catalog_bitmap = g_malloc(s->catalog_size * 4);
 
     ret = bdrv_pread(bs->file, 64, s->catalog_bitmap, s->catalog_size * 4);
diff --git a/tests/qemu-iotests/076 b/tests/qemu-iotests/076
new file mode 100755
index 0000000..6028ac5
--- /dev/null
+++ b/tests/qemu-iotests/076
@@ -0,0 +1,69 @@
+#!/bin/bash
+#
+# parallels format input validation tests
+#
+# Copyright (C) 2013 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=kwolf@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
+
+_supported_fmt parallels
+_supported_proto generic
+_supported_os Linux
+
+catalog_entries_offset=$((0x20))
+nb_sectors_offset=$((0x24))
+
+echo
+echo "== Read from a valid (enough) image =="
+_use_sample_img fake.parallels.bz2
+{ $QEMU_IO -c "read -P 0x11 0 64k" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== Negative catalog size =="
+_use_sample_img fake.parallels.bz2
+poke_file "$TEST_IMG" "$catalog_entries_offset" "\xff\xff\xff\xff"
+{ $QEMU_IO -c "read 0 512" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+
+echo
+echo "== Overflow in catalog allocation =="
+_use_sample_img fake.parallels.bz2
+poke_file "$TEST_IMG" "$nb_sectors_offset" "\xff\xff\xff\xff"
+poke_file "$TEST_IMG" "$catalog_entries_offset" "\x01\x00\x00\x40"
+{ $QEMU_IO -c "read 64M 64M" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/076.out b/tests/qemu-iotests/076.out
new file mode 100644
index 0000000..12af42a
--- /dev/null
+++ b/tests/qemu-iotests/076.out
@@ -0,0 +1,14 @@
+QA output created by 076
+
+== Read from a valid (enough) image ==
+read 65536/65536 bytes at offset 0
+64 KiB, X ops; XX:XX:XX.X (XXX YYY/sec and XXX ops/sec)
+
+== Negative catalog size ==
+qemu-io: can't open device TEST_DIR/fake.parallels: Catalog too large
+no file open, try 'help open'
+
+== Overflow in catalog allocation ==
+qemu-io: can't open device TEST_DIR/fake.parallels: Catalog too large
+no file open, try 'help open'
+*** done
diff --git a/tests/qemu-iotests/common b/tests/qemu-iotests/common
index a09d9c8..0aaf84d 100644
--- a/tests/qemu-iotests/common
+++ b/tests/qemu-iotests/common
@@ -139,6 +139,7 @@ check options
     -bochs              test bochs
     -cow                test cow
     -cloop              test cloop
+    -parallels          test parallels
     -qcow               test qcow
     -qcow2              test qcow2
     -qed                test qed
@@ -192,6 +193,12 @@ testlist options
             xpand=false
             ;;
 
+        -parallels)
+            IMGFMT=parallels
+            IMGFMT_GENERIC=false
+            xpand=false
+            ;;
+
         -qcow)
             IMGFMT=qcow
             xpand=false
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index c51640c..864643d 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -82,6 +82,7 @@
 073 rw auto quick
 074 rw auto quick
 075 rw auto
+076 auto
 077 rw auto quick
 078 rw auto
 079 rw auto
diff --git a/tests/qemu-iotests/sample_images/fake.parallels.bz2 b/tests/qemu-iotests/sample_images/fake.parallels.bz2
new file mode 100644
index 0000000000000000000000000000000000000000..ffb5f13bac31bc9ab6e1ea5c0cfa26786f2c4cc6
GIT binary patch
literal 141
zcmV;80CN9AT4*^jL0KkKS*i&LJ^%_Hf6(xNVE_;S2ml2D2!JYJ)&M{N00969FaWp;
z000b`1pojBOn|7QnnOSv)YEF7cgIVO0ByGSdk7e?fW`f$x`2Bi3t$bd06owJs09G{
vKo+1B1LXi)0CVe)J@eC^zBuEJbFFJA24D=p8Gt*$AL8yvrwS4kK_LggA5<|C

literal 0
HcmV?d00001

-- 
1.9.0

[Qemu-devel] [PULL for-2.0 50/51] parallels: Sanity check for s->tracks (CVE-2014-0142)

[Stefan Hajnoczi]

From: Kevin Wolf <kwolf@redhat.com>

This avoids a possible division by zero.

Convert s->tracks to unsigned as well because it feels better than
surviving just because the results of calculations with s->tracks are
converted to unsigned anyway.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Reviewed-by: Max Reitz <mreitz@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/parallels.c          | 7 ++++++-
 tests/qemu-iotests/076     | 7 +++++++
 tests/qemu-iotests/076.out | 4 ++++
 3 files changed, 17 insertions(+), 1 deletion(-)

diff --git a/block/parallels.c b/block/parallels.c
index fe47ecb..1a5bd35 100644
--- a/block/parallels.c
+++ b/block/parallels.c
@@ -51,7 +51,7 @@ typedef struct BDRVParallelsState {
     uint32_t *catalog_bitmap;
     unsigned int catalog_size;
 
-    int tracks;
+    unsigned int tracks;
 } BDRVParallelsState;
 
 static int parallels_probe(const uint8_t *buf, int buf_size, const char *filename)
@@ -93,6 +93,11 @@ static int parallels_open(BlockDriverState *bs, QDict *options, int flags,
     bs->total_sectors = le32_to_cpu(ph.nb_sectors);
 
     s->tracks = le32_to_cpu(ph.tracks);
+    if (s->tracks == 0) {
+        error_setg(errp, "Invalid image: Zero sectors per track");
+        ret = -EINVAL;
+        goto fail;
+    }
 
     s->catalog_size = le32_to_cpu(ph.catalog_entries);
     if (s->catalog_size > INT_MAX / 4) {
diff --git a/tests/qemu-iotests/076 b/tests/qemu-iotests/076
index 6028ac5..b614a7d 100755
--- a/tests/qemu-iotests/076
+++ b/tests/qemu-iotests/076
@@ -42,6 +42,7 @@ _supported_fmt parallels
 _supported_proto generic
 _supported_os Linux
 
+tracks_offset=$((0x1c))
 catalog_entries_offset=$((0x20))
 nb_sectors_offset=$((0x24))
 
@@ -63,6 +64,12 @@ poke_file "$TEST_IMG" "$nb_sectors_offset" "\xff\xff\xff\xff"
 poke_file "$TEST_IMG" "$catalog_entries_offset" "\x01\x00\x00\x40"
 { $QEMU_IO -c "read 64M 64M" $TEST_IMG; } 2>&1 | _filter_qemu_io | _filter_testdir
 
+echo
+echo "== Zero sectors per track =="
+_use_sample_img fake.parallels.bz2
+poke_file "$TEST_IMG" "$tracks_offset" "\x00\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/076.out b/tests/qemu-iotests/076.out
index 12af42a..f7745d8 100644
--- a/tests/qemu-iotests/076.out
+++ b/tests/qemu-iotests/076.out
@@ -11,4 +11,8 @@ no file open, try 'help open'
 == Overflow in catalog allocation ==
 qemu-io: can't open device TEST_DIR/fake.parallels: Catalog too large
 no file open, try 'help open'
+
+== Zero sectors per track ==
+qemu-io: can't open device TEST_DIR/fake.parallels: Invalid image: Zero sectors per track
+no file open, try 'help open'
 *** done
-- 
1.9.0

[Qemu-devel] [PULL for-2.0 51/51] qcow2: link all L2 meta updates in preallocate()

[Stefan Hajnoczi]

preallocate() only links the first QCowL2Meta's data clusters into the
L2 table and ignores any chained QCowL2Metas in the linked list.

Chains of QCowL2Meta structs are built up when contiguous clusters span
L2 tables.  Each QCowL2Meta describes one L2 table update.  This is a
rare case in preallocate() but can happen.

This patch fixes preallocate() by iterating over the whole list of
QCowL2Metas.  Compare with the qcow2_co_writev() function's
implementation, which is similar but also also handles request
dependencies.  preallocate() only performs one allocation at a time so
there can be no dependencies.

Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
---
 block/qcow2.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index bb6000f..333e26d 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -1531,7 +1531,9 @@ static int preallocate(BlockDriverState *bs)
             return ret;
         }
 
-        if (meta != NULL) {
+        while (meta) {
+            QCowL2Meta *next = meta->next;
+
             ret = qcow2_alloc_cluster_link_l2(bs, meta);
             if (ret < 0) {
                 qcow2_free_any_clusters(bs, meta->alloc_offset,
@@ -1542,6 +1544,9 @@ static int preallocate(BlockDriverState *bs)
             /* There are no dependent requests, but we need to remove our
              * request from the list of in-flight requests */
             QLIST_REMOVE(meta, next_in_flight);
+
+            g_free(meta);
+            meta = next;
         }
 
         /* TODO Preallocate data if requested */
-- 
1.9.0

Re: [Qemu-devel] [PULL for-2.0 00/51] Block patches

[Peter Maydell]

On 1 April 2014 18:18, Stefan Hajnoczi <stefanha@redhat.com> wrote:
> This pull request for QEMU 2.0 contains the block layer CVE security fixes and
> additional bug fixes by Markus and Prasad.
>
> The following changes since commit 63678e17cf399ff81b93417fe7bee8d6ef6b6b1b:
>
>   configure: add option to disable -fstack-protector flags (2014-03-31 20:16:02 +0100)
>
> are available in the git repository at:
>
>   git://github.com/stefanha/qemu.git tags/block-pull-request
>
> for you to fetch changes up to c792707f54aa445cfb63a42411c66594b52b8f79:
>
>   qcow2: link all L2 meta updates in preallocate() (2014-04-01 15:22:35 +0200)

Applied, thanks.
-- PMM