[Qemu-devel] [PATCH for-2.1] qcow2: Fix error path for unknown incompatible features

[Qemu-devel] [PATCH for-2.1] qcow2: Fix error path for unknown incompatible features

[Kevin Wolf]

qcow2's report_unsupported_feature() had two bugs: A 32 bit truncation
would prevent feature table entries for bits 32-63 from being used, and
it could assign errp multiple times if there was more than one unknown
feature, resulting in an error_set() assertion failure.

Fix the truncation, make sure to set the error exactly once and add a
qemu-iotests case for it.

This fixes https://bugs.launchpad.net/qemu/+bug/1342704/

Reported-by: Maria Kustova <maria.k@catit.be>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
 block/qcow2.c               | 21 +++++++++++++-----
 tests/qemu-iotests/036      | 52 ++++++++++++++++++++++++++++++++++++++++++++-
 tests/qemu-iotests/036.out  | 35 ++++++++++++++++++++++++++++++
 tests/qemu-iotests/qcow2.py | 15 ++++++++-----
 4 files changed, 112 insertions(+), 11 deletions(-)

diff --git a/block/qcow2.c b/block/qcow2.c
index 445ead4..1e3ab6b 100644
--- a/block/qcow2.c
+++ b/block/qcow2.c
@@ -210,20 +210,31 @@ static void GCC_FMT_ATTR(3, 4) report_unsupported(BlockDriverState *bs,
 static void report_unsupported_feature(BlockDriverState *bs,
     Error **errp, Qcow2Feature *table, uint64_t mask)
 {
+    char *features = g_strdup("");
+    char *old;
+
     while (table && table->name[0] != '\0') {
         if (table->type == QCOW2_FEAT_TYPE_INCOMPATIBLE) {
-            if (mask & (1 << table->bit)) {
-                report_unsupported(bs, errp, "%.46s", table->name);
-                mask &= ~(1 << table->bit);
+            if (mask & (1ULL << table->bit)) {
+                old = features;
+                features = g_strdup_printf("%s%s%.46s", old, *old ? ", " : "",
+                                           table->name);
+                g_free(old);
+                mask &= ~(1ULL << table->bit);
             }
         }
         table++;
     }
 
     if (mask) {
-        report_unsupported(bs, errp, "Unknown incompatible feature: %" PRIx64,
-                           mask);
+        old = features;
+        features = g_strdup_printf("%s%sUnknown incompatible feature: %" PRIx64,
+                                   old, *old ? ", " : "", mask);
+        g_free(old);
     }
+
+    report_unsupported(bs, errp, "%s", features);
+    g_free(features);
 }
 
 /*
diff --git a/tests/qemu-iotests/036 b/tests/qemu-iotests/036
index a773653..392f1ef 100755
--- a/tests/qemu-iotests/036
+++ b/tests/qemu-iotests/036
@@ -1,6 +1,6 @@
 #!/bin/bash
 #
-# Test that qcow2 unknown autoclear feature bits are cleared
+# Test qcow2 feature bits
 #
 # Copyright (C) 2011 Red Hat, Inc.
 # Copyright IBM, Corp. 2010
@@ -50,6 +50,56 @@ _supported_os Linux
 # Only qcow2v3 and later supports feature bits
 IMGOPTS="compat=1.1"
 
+echo
+echo === Image with unknown incompatible feature bit ===
+echo
+_make_test_img 64M
+$PYTHON qcow2.py "$TEST_IMG" set-feature-bit incompatible 63
+
+# Without feature table
+$PYTHON qcow2.py "$TEST_IMG" dump-header
+_img_info
+
+# With feature table containing bit 63
+printf "\x00\x3f%s" "Test feature" | $PYTHON qcow2.py "$TEST_IMG" add-header-ext-stdio 0x6803f857
+_img_info
+
+echo
+echo === Image with multiple incompatible feature bits ===
+echo
+_make_test_img 64M
+$PYTHON qcow2.py "$TEST_IMG" set-feature-bit incompatible 61
+$PYTHON qcow2.py "$TEST_IMG" set-feature-bit incompatible 62
+$PYTHON qcow2.py "$TEST_IMG" set-feature-bit incompatible 63
+
+# Without feature table
+_img_info
+
+# With feature table containing bit 63
+printf "\x00\x3f%s" "Test feature" | $PYTHON qcow2.py "$TEST_IMG" add-header-ext-stdio 0x6803f857
+_img_info
+
+# With feature table containing bit 61
+$PYTHON qcow2.py "$TEST_IMG" del-header-ext 0x6803f857
+printf "\x00\x3d%s" "Test feature" | $PYTHON qcow2.py "$TEST_IMG" add-header-ext-stdio 0x6803f857
+_img_info
+
+# With feature table containing bits 61 and 62
+$PYTHON qcow2.py "$TEST_IMG" del-header-ext 0x6803f857
+printf "\x00\x3d%s\x00%40s\x00\x3e%s\x00%40s" "test1" "" "test2" "" | $PYTHON qcow2.py "$TEST_IMG" add-header-ext-stdio 0x6803f857
+_img_info
+
+# With feature table containing all bits
+$PYTHON qcow2.py "$TEST_IMG" del-header-ext 0x6803f857
+printf "\x00\x3d%s\x00%40s\x00\x3e%s\x00%40s\x00\x3f%s\x00%40s" "test1" "" "test2" "" "test3" "" | $PYTHON qcow2.py "$TEST_IMG" add-header-ext-stdio 0x6803f857
+_img_info
+
+# With feature table containing unrelated bits, including compatible/autoclear
+$PYTHON qcow2.py "$TEST_IMG" del-header-ext 0x6803f857
+printf "\x01\x3d%s\x00%40s\x00\x3e%s\x00%40s\x02\x3f%s\x00%40s\x00\x3c%s\x00%40s" "test1" "" "test2" "" "test3" "" "test4" "" | $PYTHON qcow2.py "$TEST_IMG" add-header-ext-stdio 0x6803f857
+_img_info
+
+
 echo === Create image with unknown autoclear feature bit ===
 echo
 _make_test_img 64M
diff --git a/tests/qemu-iotests/036.out b/tests/qemu-iotests/036.out
index 55a3e6e..720bd89 100644
--- a/tests/qemu-iotests/036.out
+++ b/tests/qemu-iotests/036.out
@@ -1,4 +1,39 @@
 QA output created by 036
+
+=== Image with unknown incompatible feature bit ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+magic                     0x514649fb
+version                   3
+backing_file_offset       0x0
+backing_file_size         0x0
+cluster_bits              16
+size                      67108864
+crypt_method              0
+l1_size                   1
+l1_table_offset           0x30000
+refcount_table_offset     0x10000
+refcount_table_clusters   1
+nb_snapshots              0
+snapshot_offset           0x0
+incompatible_features     0x8000000000000000
+compatible_features       0x0
+autoclear_features        0x0
+refcount_order            4
+header_length             104
+
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': 'image' uses a IMGFMT feature which is not supported by this qemu version: Unknown incompatible feature: 8000000000000000
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': 'image' uses a IMGFMT feature which is not supported by this qemu version: Test feature
+
+=== Image with multiple incompatible feature bits ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': 'image' uses a IMGFMT feature which is not supported by this qemu version: Unknown incompatible feature: e000000000000000
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': 'image' uses a IMGFMT feature which is not supported by this qemu version: Test feature, Unknown incompatible feature: 6000000000000000
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': 'image' uses a IMGFMT feature which is not supported by this qemu version: Test feature, Unknown incompatible feature: c000000000000000
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': 'image' uses a IMGFMT feature which is not supported by this qemu version: test1, test2, Unknown incompatible feature: 8000000000000000
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': 'image' uses a IMGFMT feature which is not supported by this qemu version: test1, test2, test3
+qemu-img: Could not open 'TEST_DIR/t.IMGFMT': 'image' uses a IMGFMT feature which is not supported by this qemu version: test2, Unknown incompatible feature: a000000000000000
 === Create image with unknown autoclear feature bit ===
 
 Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=67108864 
diff --git a/tests/qemu-iotests/qcow2.py b/tests/qemu-iotests/qcow2.py
index 44a2b45..2058596 100755
--- a/tests/qemu-iotests/qcow2.py
+++ b/tests/qemu-iotests/qcow2.py
@@ -176,6 +176,10 @@ def cmd_add_header_ext(fd, magic, data):
     h.extensions.append(QcowHeaderExtension.create(magic, data))
     h.update(fd)
 
+def cmd_add_header_ext_stdio(fd, magic):
+    data = sys.stdin.read()
+    cmd_add_header_ext(fd, magic, data)
+
 def cmd_del_header_ext(fd, magic):
     try:
         magic = int(magic, 0)
@@ -220,11 +224,12 @@ def cmd_set_feature_bit(fd, group, bit):
     h.update(fd)
 
 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'],
+    [ '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' ],
+    [ 'add-header-ext-stdio', cmd_add_header_ext_stdio, 1, 'Add a header extension, data from stdin' ],
+    [ 'del-header-ext',       cmd_del_header_ext,       1, 'Delete a header extension' ],
+    [ 'set-feature-bit',      cmd_set_feature_bit,      2, 'Set a feature bit'],
 ]
 
 def main(filename, cmd, args):
-- 
1.8.3.1

Re: [Qemu-devel] [PATCH for-2.1] qcow2: Fix error path for unknown incompatible features

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1075 bytes --]

On 07/17/2014 03:41 AM, Kevin Wolf wrote:
> qcow2's report_unsupported_feature() had two bugs: A 32 bit truncation
> would prevent feature table entries for bits 32-63 from being used, and
> it could assign errp multiple times if there was more than one unknown
> feature, resulting in an error_set() assertion failure.
> 
> Fix the truncation, make sure to set the error exactly once and add a
> qemu-iotests case for it.
> 
> This fixes https://bugs.launchpad.net/qemu/+bug/1342704/
> 
> Reported-by: Maria Kustova <maria.k@catit.be>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/qcow2.c               | 21 +++++++++++++-----
>  tests/qemu-iotests/036      | 52 ++++++++++++++++++++++++++++++++++++++++++++-
>  tests/qemu-iotests/036.out  | 35 ++++++++++++++++++++++++++++++
>  tests/qemu-iotests/qcow2.py | 15 ++++++++-----
>  4 files changed, 112 insertions(+), 11 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 604 bytes --]

Re: [Qemu-devel] [PATCH for-2.1] qcow2: Fix error path for unknown incompatible features

[Stefan Hajnoczi]

[-- Attachment #1: Type: text/plain, Size: 1030 bytes --]

On Thu, Jul 17, 2014 at 11:41:53AM +0200, Kevin Wolf wrote:
> qcow2's report_unsupported_feature() had two bugs: A 32 bit truncation
> would prevent feature table entries for bits 32-63 from being used, and
> it could assign errp multiple times if there was more than one unknown
> feature, resulting in an error_set() assertion failure.
> 
> Fix the truncation, make sure to set the error exactly once and add a
> qemu-iotests case for it.
> 
> This fixes https://bugs.launchpad.net/qemu/+bug/1342704/
> 
> Reported-by: Maria Kustova <maria.k@catit.be>
> Signed-off-by: Kevin Wolf <kwolf@redhat.com>
> ---
>  block/qcow2.c               | 21 +++++++++++++-----
>  tests/qemu-iotests/036      | 52 ++++++++++++++++++++++++++++++++++++++++++++-
>  tests/qemu-iotests/036.out  | 35 ++++++++++++++++++++++++++++++
>  tests/qemu-iotests/qcow2.py | 15 ++++++++-----
>  4 files changed, 112 insertions(+), 11 deletions(-)

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

Stefan

[-- Attachment #2: Type: application/pgp-signature, Size: 473 bytes --]