[Qemu-devel] [PATCH v2 for-1.5 0/4] qcow2: Catch some L1 table index overflows

[Qemu-devel] [PATCH v2 for-1.5 0/4] qcow2: Catch some L1 table index overflows

[Kevin Wolf]

Kevin Wolf (4):
  qcow2: Catch some L1 table index overflows
  block: Add hint to -EFBIG error message
  qcow2.py: Subcommand for changing header fields
  qemu-iotests: Try creating huge qcow2 image

 block.c                      |  8 ++++--
 block/qcow2-cluster.c        | 23 ++++++++++++------
 block/qcow2.c                | 13 ++++++++--
 block/qcow2.h                |  5 ++--
 tests/qemu-iotests/054       | 58 ++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/054.out   | 10 ++++++++
 tests/qemu-iotests/common.rc |  2 +-
 tests/qemu-iotests/group     |  1 +
 tests/qemu-iotests/qcow2.py  | 17 +++++++++++++
 9 files changed, 122 insertions(+), 15 deletions(-)
 create mode 100755 tests/qemu-iotests/054
 create mode 100644 tests/qemu-iotests/054.out

-- 
1.8.1.4

[Qemu-devel] [PATCH v2 for-1.5 1/4] qcow2: Catch some L1 table index overflows

[Kevin Wolf]

This catches the situation that is described in the bug report at
https://bugs.launchpad.net/qemu/+bug/865518 and goes like this:

    $ qemu-img create -f qcow2 huge.qcow2 $((1024*1024))T
    Formatting 'huge.qcow2', fmt=qcow2 size=1152921504606846976 encryption=off cluster_size=65536 lazy_refcounts=off
    $ qemu-io /tmp/huge.qcow2 -c "write $((1024*1024*1024*1024*1024*1024 - 1024)) 512"
    Segmentation fault

With this patch applied the segfault will be avoided, however the case
will still fail, though gracefully:

    $ qemu-img create -f qcow2 /tmp/huge.qcow2 $((1024*1024))T
    Formatting 'huge.qcow2', fmt=qcow2 size=1152921504606846976 encryption=off cluster_size=65536 lazy_refcounts=off
    qemu-img: The image size is too large for file format 'qcow2'

Note that even long before these overflow checks kick in, you get
insanely high memory usage (up to INT_MAX * sizeof(uint64_t) = 16 GB for
the L1 table), so with somewhat smaller image sizes you'll probably see
qemu aborting for a failed g_malloc().

If you need huge image sizes, you should increase the cluster size to
the maximum of 2 MB in order to get higher limits.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2-cluster.c | 23 +++++++++++++++--------
 block/qcow2.c         | 13 +++++++++++--
 block/qcow2.h         |  5 +++--
 3 files changed, 29 insertions(+), 12 deletions(-)

diff --git a/block/qcow2-cluster.c b/block/qcow2-cluster.c
index c71470a..76f30e5 100644
--- a/block/qcow2-cluster.c
+++ b/block/qcow2-cluster.c
@@ -29,12 +29,13 @@
 #include "block/qcow2.h"
 #include "trace.h"
 
-int qcow2_grow_l1_table(BlockDriverState *bs, int min_size, bool exact_size)
+int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
+                        bool exact_size)
 {
     BDRVQcowState *s = bs->opaque;
-    int new_l1_size, new_l1_size2, ret, i;
+    int new_l1_size2, ret, i;
     uint64_t *new_l1_table;
-    int64_t new_l1_table_offset;
+    int64_t new_l1_table_offset, new_l1_size;
     uint8_t data[12];
 
     if (min_size <= s->l1_size)
@@ -53,8 +54,13 @@ int qcow2_grow_l1_table(BlockDriverState *bs, int min_size, bool exact_size)
         }
     }
 
+    if (new_l1_size > INT_MAX) {
+        return -EFBIG;
+    }
+
 #ifdef DEBUG_ALLOC2
-    fprintf(stderr, "grow l1_table from %d to %d\n", s->l1_size, new_l1_size);
+    fprintf(stderr, "grow l1_table from %d to %" PRId64 "\n",
+            s->l1_size, new_l1_size);
 #endif
 
     new_l1_size2 = sizeof(uint64_t) * new_l1_size;
@@ -391,8 +397,8 @@ int qcow2_get_cluster_offset(BlockDriverState *bs, uint64_t offset,
     int *num, uint64_t *cluster_offset)
 {
     BDRVQcowState *s = bs->opaque;
-    unsigned int l1_index, l2_index;
-    uint64_t l2_offset, *l2_table;
+    unsigned int l2_index;
+    uint64_t l1_index, l2_offset, *l2_table;
     int l1_bits, c;
     unsigned int index_in_cluster, nb_clusters;
     uint64_t nb_available, nb_needed;
@@ -507,8 +513,8 @@ static int get_cluster_table(BlockDriverState *bs, uint64_t offset,
                              int *new_l2_index)
 {
     BDRVQcowState *s = bs->opaque;
-    unsigned int l1_index, l2_index;
-    uint64_t l2_offset;
+    unsigned int l2_index;
+    uint64_t l1_index, l2_offset;
     uint64_t *l2_table = NULL;
     int ret;
 
@@ -522,6 +528,7 @@ static int get_cluster_table(BlockDriverState *bs, uint64_t offset,
         }
     }
 
+    assert(l1_index < s->l1_size);
     l2_offset = s->l1_table[l1_index] & L1E_OFFSET_MASK;
 
     /* seek the l2 table of the given l2 offset */
diff --git a/block/qcow2.c b/block/qcow2.c
index 2e346d8..0fa5cb2 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -307,6 +307,7 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags)
     QemuOpts *opts;
     Error *local_err = NULL;
     uint64_t ext_end;
+    uint64_t l1_vm_state_index;
 
     ret = bdrv_pread(bs->file, 0, &header, sizeof(header));
     if (ret < 0) {
@@ -424,7 +425,14 @@ static int qcow2_open(BlockDriverState *bs, QDict *options, int flags)
 
     /* read the level 1 table */
     s->l1_size = header.l1_size;
-    s->l1_vm_state_index = size_to_l1(s, header.size);
+
+    l1_vm_state_index = size_to_l1(s, header.size);
+    if (l1_vm_state_index > INT_MAX) {
+        ret = -EFBIG;
+        goto fail;
+    }
+    s->l1_vm_state_index = l1_vm_state_index;
+
     /* the L1 table must contain at least enough entries to put
        header.size bytes */
     if (s->l1_size < s->l1_vm_state_index) {
@@ -1480,7 +1488,8 @@ static coroutine_fn int qcow2_co_discard(BlockDriverState *bs,
 static int qcow2_truncate(BlockDriverState *bs, int64_t offset)
 {
     BDRVQcowState *s = bs->opaque;
-    int ret, new_l1_size;
+    int64_t new_l1_size;
+    int ret;
 
     if (offset & 511) {
         error_report("The new size must be a multiple of 512");
diff --git a/block/qcow2.h b/block/qcow2.h
index 9421843..6959c6a 100644
--- a/block/qcow2.h
+++ b/block/qcow2.h
@@ -284,7 +284,7 @@ static inline int size_to_clusters(BDRVQcowState *s, int64_t size)
     return (size + (s->cluster_size - 1)) >> s->cluster_bits;
 }
 
-static inline int size_to_l1(BDRVQcowState *s, int64_t size)
+static inline int64_t size_to_l1(BDRVQcowState *s, int64_t size)
 {
     int shift = s->cluster_bits + s->l2_bits;
     return (size + (1ULL << shift) - 1) >> shift;
@@ -360,7 +360,8 @@ int qcow2_check_refcounts(BlockDriverState *bs, BdrvCheckResult *res,
                           BdrvCheckMode fix);
 
 /* qcow2-cluster.c functions */
-int qcow2_grow_l1_table(BlockDriverState *bs, int min_size, bool exact_size);
+int qcow2_grow_l1_table(BlockDriverState *bs, uint64_t min_size,
+                        bool exact_size);
 void qcow2_l2_cache_reset(BlockDriverState *bs);
 int qcow2_decompress_cluster(BlockDriverState *bs, uint64_t cluster_offset);
 void qcow2_encrypt_sectors(BDRVQcowState *s, int64_t sector_num,
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v2 for-1.5 1/4] qcow2: Catch some L1 table index overflows

[Stefan Hajnoczi]

On Tue, May 14, 2013 at 04:14:33PM +0200, Kevin Wolf wrote:
> This catches the situation that is described in the bug report at
> https://bugs.launchpad.net/qemu/+bug/865518 and goes like this:
> 
>     $ qemu-img create -f qcow2 huge.qcow2 $((1024*1024))T
>     Formatting 'huge.qcow2', fmt=qcow2 size=1152921504606846976 encryption=off cluster_size=65536 lazy_refcounts=off
>     $ qemu-io /tmp/huge.qcow2 -c "write $((1024*1024*1024*1024*1024*1024 - 1024)) 512"
>     Segmentation fault
> 
> With this patch applied the segfault will be avoided, however the case
> will still fail, though gracefully:
> 
>     $ qemu-img create -f qcow2 /tmp/huge.qcow2 $((1024*1024))T
>     Formatting 'huge.qcow2', fmt=qcow2 size=1152921504606846976 encryption=off cluster_size=65536 lazy_refcounts=off
>     qemu-img: The image size is too large for file format 'qcow2'
> 
> Note that even long before these overflow checks kick in, you get
> insanely high memory usage (up to INT_MAX * sizeof(uint64_t) = 16 GB for
> the L1 table), so with somewhat smaller image sizes you'll probably see
> qemu aborting for a failed g_malloc().
> 
> If you need huge image sizes, you should increase the cluster size to
> the maximum of 2 MB in order to get higher limits.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/qcow2-cluster.c | 23 +++++++++++++++--------
>  block/qcow2.c         | 13 +++++++++++--
>  block/qcow2.h         |  5 +++--
>  3 files changed, 29 insertions(+), 12 deletions(-)

Thanks, applied to my block tree for 1.5:
https://github.com/stefanha/qemu/commits/block

Stefan

[Qemu-devel] [PATCH v2 for-1.5 2/4] block: Add hint to -EFBIG error message

[Kevin Wolf]

The limit of qcow2 files at least depends on the cluster size. If the
image format has a cluster_size option, suggest to increase it.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/block.c b/block.c
index aa9a533..3f87489 100644
--- a/block.c
+++ b/block.c
@@ -4857,8 +4857,12 @@ void bdrv_img_create(const char *filename, const char *fmt,
             error_setg(errp,"Formatting or formatting option not supported for "
                             "file format '%s'", fmt);
         } else if (ret == -EFBIG) {
-            error_setg(errp, "The image size is too large for file format '%s'",
-                       fmt);
+            const char *cluster_size_hint = "";
+            if (get_option_parameter(create_options, BLOCK_OPT_CLUSTER_SIZE)) {
+                cluster_size_hint = " (try using a larger cluster size)";
+            }
+            error_setg(errp, "The image size is too large for file format '%s'%s",
+                       fmt, cluster_size_hint);
         } else {
             error_setg(errp, "%s: error while creating %s: %s", filename, fmt,
                        strerror(-ret));
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v2 for-1.5 2/4] block: Add hint to -EFBIG error message

[Stefan Hajnoczi]

On Tue, May 14, 2013 at 04:14:34PM +0200, Kevin Wolf wrote:
> The limit of qcow2 files at least depends on the cluster size. If the
> image format has a cluster_size option, suggest to increase it.
> 
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block.c | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

Thanks, applied to my block tree for 1.5:
https://github.com/stefanha/qemu/commits/block

Stefan

[Qemu-devel] [PATCH v2 3/4] qcow2.py: Subcommand for changing header fields

[Kevin Wolf]

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/qcow2.py | 17 +++++++++++++++++
 1 file changed, 17 insertions(+)

diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
index fecf5b9..44a2b45 100755
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -149,6 +149,22 @@ def cmd_dump_header(fd):
     h.dump()
     h.dump_extensions()
 
+def cmd_set_header(fd, name, value):
+    try:
+        value = int(value, 0)
+    except:
+        print "'%s' is not a valid number" % value
+        sys.exit(1)
+
+    fields = (field[2] for field in QcowHeader.fields)
+    if not name in fields:
+        print "'%s' is not a known header field" % name
+        sys.exit(1)
+
+    h = QcowHeader(fd)
+    h.__dict__[name] = value
+    h.update(fd)
+
 def cmd_add_header_ext(fd, magic, data):
     try:
         magic = int(magic, 0)
@@ -205,6 +221,7 @@ def cmd_set_feature_bit(fd, group, bit):
 
 cmds = [
     [ 'dump-header',    cmd_dump_header,    0, 'Dump image header and header extensions' ],
+    [ 'set-header',     cmd_set_header,     2, 'Set a field in the header'],
     [ 'add-header-ext', cmd_add_header_ext, 2, 'Add a header extension' ],
     [ 'del-header-ext', cmd_del_header_ext, 1, 'Delete a header extension' ],
     [ 'set-feature-bit', cmd_set_feature_bit, 2, 'Set a feature bit'],
-- 
1.8.1.4

[Qemu-devel] [PATCH v2 4/4] qemu-iotests: Try creating huge qcow2 image

[Kevin Wolf]

It's supposed to fail gracefully instead of segfaulting.

Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 tests/qemu-iotests/054       | 58 ++++++++++++++++++++++++++++++++++++++++++++
 tests/qemu-iotests/054.out   | 10 ++++++++
 tests/qemu-iotests/common.rc |  2 +-
 tests/qemu-iotests/group     |  1 +
 4 files changed, 70 insertions(+), 1 deletion(-)
 create mode 100755 tests/qemu-iotests/054
 create mode 100644 tests/qemu-iotests/054.out

diff --git a/tests/qemu-iotests/054 b/tests/qemu-iotests/054
new file mode 100755
index 0000000..b360429
--- /dev/null
+++ b/tests/qemu-iotests/054
@@ -0,0 +1,58 @@
+#!/bin/bash
+#
+# Test huge qcow2 images
+#
+# 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
+
+echo
+echo "creating too large image (1 EB)"
+_make_test_img $((1024*1024))T
+
+echo
+echo "creating too large image (1 EB) using qcow2.py"
+_make_test_img 4G
+./qcow2.py $TEST_IMG set-header size $((1024 ** 6))
+_check_test_img
+
+# success, all done
+echo "*** done"
+rm -f $seq.full
+status=0
diff --git a/tests/qemu-iotests/054.out b/tests/qemu-iotests/054.out
new file mode 100644
index 0000000..0b2fe30
--- /dev/null
+++ b/tests/qemu-iotests/054.out
@@ -0,0 +1,10 @@
+QA output created by 054
+
+creating too large image (1 EB)
+qemu-img: The image size is too large for file format 'qcow2'
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=1152921504606846976 
+
+creating too large image (1 EB) using qcow2.py
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=4294967296 
+qemu-img: Could not open 'TEST_DIR/t.qcow2': File too large
+*** done
diff --git a/tests/qemu-iotests/common.rc b/tests/qemu-iotests/common.rc
index 31eb62b..e9ba358 100644
--- a/tests/qemu-iotests/common.rc
+++ b/tests/qemu-iotests/common.rc
@@ -167,7 +167,7 @@ _cleanup_test_img()
 
 _check_test_img()
 {
-    $QEMU_IMG check "$@" -f $IMGFMT $TEST_IMG 2>&1 | \
+    $QEMU_IMG check "$@" -f $IMGFMT $TEST_IMG 2>&1 | _filter_testdir | \
         sed -e '/allocated.*fragmented.*compressed clusters/d' \
             -e 's/qemu-img: This image format does not support checks/No errors were found on the image./' \
             -e '/Image end offset: [0-9]\+/d'
diff --git a/tests/qemu-iotests/group b/tests/qemu-iotests/group
index bf944d9..387b050 100644
--- a/tests/qemu-iotests/group
+++ b/tests/qemu-iotests/group
@@ -60,3 +60,4 @@
 #051 rw auto
 052 rw auto backing
 053 rw auto
+054 rw auto
-- 
1.8.1.4

Re: [Qemu-devel] [PATCH v2 for-1.5 0/4] qcow2: Catch some L1 table index overflows

[Richard W.M. Jones]

On Tue, May 14, 2013 at 04:14:32PM +0200, Kevin Wolf wrote:
> Kevin Wolf (4):
>   qcow2: Catch some L1 table index overflows
>   block: Add hint to -EFBIG error message
>   qcow2.py: Subcommand for changing header fields
>   qemu-iotests: Try creating huge qcow2 image
> 
>  block.c                      |  8 ++++--
>  block/qcow2-cluster.c        | 23 ++++++++++++------
>  block/qcow2.c                | 13 ++++++++--
>  block/qcow2.h                |  5 ++--
>  tests/qemu-iotests/054       | 58 ++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/054.out   | 10 ++++++++
>  tests/qemu-iotests/common.rc |  2 +-
>  tests/qemu-iotests/group     |  1 +
>  tests/qemu-iotests/qcow2.py  | 17 +++++++++++++
>  9 files changed, 122 insertions(+), 15 deletions(-)
>  create mode 100755 tests/qemu-iotests/054
>  create mode 100644 tests/qemu-iotests/054.out

ACK, at least for the first two.  I didn't not test patches 3 & 4.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
libguestfs lets you edit virtual machines.  Supports shell scripting,
bindings from many languages.  http://libguestfs.org

Re: [Qemu-devel] [PATCH v2 for-1.5 0/4] qcow2: Catch some L1 table index overflows

[Stefan Hajnoczi]

On Tue, May 14, 2013 at 04:14:32PM +0200, Kevin Wolf wrote:
> Kevin Wolf (4):
>   qcow2: Catch some L1 table index overflows
>   block: Add hint to -EFBIG error message
>   qcow2.py: Subcommand for changing header fields
>   qemu-iotests: Try creating huge qcow2 image
> 
>  block.c                      |  8 ++++--
>  block/qcow2-cluster.c        | 23 ++++++++++++------
>  block/qcow2.c                | 13 ++++++++--
>  block/qcow2.h                |  5 ++--
>  tests/qemu-iotests/054       | 58 ++++++++++++++++++++++++++++++++++++++++++++
>  tests/qemu-iotests/054.out   | 10 ++++++++
>  tests/qemu-iotests/common.rc |  2 +-
>  tests/qemu-iotests/group     |  1 +
>  tests/qemu-iotests/qcow2.py  | 17 +++++++++++++
>  9 files changed, 122 insertions(+), 15 deletions(-)
>  create mode 100755 tests/qemu-iotests/054
>  create mode 100644 tests/qemu-iotests/054.out

Thanks, applied Patches 2 & 3 to my block-next tree:
https://github.com/stefanha/qemu/commits/block-next

Stefan

Re: [Qemu-devel] [PATCH v2 for-1.5 0/4] qcow2: Catch some L1 table index overflows

[Stefan Hajnoczi]

On Wed, May 15, 2013 at 10:28:04AM +0200, Stefan Hajnoczi wrote:
> On Tue, May 14, 2013 at 04:14:32PM +0200, Kevin Wolf wrote:
> > Kevin Wolf (4):
> >   qcow2: Catch some L1 table index overflows
> >   block: Add hint to -EFBIG error message
> >   qcow2.py: Subcommand for changing header fields
> >   qemu-iotests: Try creating huge qcow2 image
> > 
> >  block.c                      |  8 ++++--
> >  block/qcow2-cluster.c        | 23 ++++++++++++------
> >  block/qcow2.c                | 13 ++++++++--
> >  block/qcow2.h                |  5 ++--
> >  tests/qemu-iotests/054       | 58 ++++++++++++++++++++++++++++++++++++++++++++
> >  tests/qemu-iotests/054.out   | 10 ++++++++
> >  tests/qemu-iotests/common.rc |  2 +-
> >  tests/qemu-iotests/group     |  1 +
> >  tests/qemu-iotests/qcow2.py  | 17 +++++++++++++
> >  9 files changed, 122 insertions(+), 15 deletions(-)
> >  create mode 100755 tests/qemu-iotests/054
> >  create mode 100644 tests/qemu-iotests/054.out
> 
> Thanks, applied Patches 2 & 3 to my block-next tree:
> https://github.com/stefanha/qemu/commits/block-next

This should have said "Patches 3 & 4" :).

Stefan