[Qemu-devel] [PATCH v2 0/4] qapi: fix crash in dealloc visitor for union types

[Qemu-devel] [PATCH v2 0/4] qapi: fix crash in dealloc visitor for union types

[Michael Roth]

This series introduces visit_start_union and visit_end_union as a way
of allowing visitors to trigger generated code to bail out on visiting
union fields if the visitor implementation deems doing so to be unsafe.

See patch 1 for the circumstances that cause the segfault in the
dealloc visitor.

This is a spin-off of a patch submitted by Fam Zheng earlier. See the
thread for additional background on why we're taking this approach:

  http://thread.gmane.org/gmane.comp.emulators.qemu/296090

v2:
  * added comments to clarify true/false visit_start_union vs. errp
    in dealloc visitor implementation (Eric)
  * fixed a line that was >80 characters (Eric)
  * fixed extra period in patch 2 commit msg (Eric)
  * added comments to note reasons why the interface is useable for
    dealloc visitor, but may require extra QAPI groundwork and
    interface parameters to be useful for other types of visitors
  * included Fam's qemu-iotest test case to exercise bug fix in a
    user-triggerable scenario.

 include/qapi/visitor-impl.h             |  2 ++
 include/qapi/visitor.h                  |  2 ++
 qapi/qapi-dealloc-visitor.c             | 26 ++++++++++++++++++++++++++
 qapi/qapi-visit-core.c                  | 15 +++++++++++++++
 scripts/qapi-visit.py                   |  6 ++++++
 tests/qapi-schema/qapi-schema-test.json | 10 ++++++++++
 tests/qapi-schema/qapi-schema-test.out  |  3 +++
 tests/qemu-iotests/087                  | 17 +++++++++++++++++
 tests/qemu-iotests/087.out              | 13 +++++++++++++
 tests/test-qmp-input-strict.c           | 17 +++++++++++++++++
 10 files changed, 111 insertions(+)

[Qemu-devel] [PATCH v2 1/4] qapi: add visit_start_union and visit_end_union

[Michael Roth]

In some cases an input visitor might bail out on filling out a
struct for various reasons, such as missing fields when running
in strict mode. In the case of a QAPI Union type, this may lead
to cases where the .kind field which encodes the union type
is uninitialized. Subsequently, other visitors, such as the
dealloc visitor, may use this .kind value as if it were
initialized, leading to assumptions about the union type which
in this case may lead to segfaults. For example, freeing an
integer value.

However, we can generally rely on the fact that the always-present
.data void * field that we generate for these union types will
always be NULL in cases where .kind is uninitialized (at least,
there shouldn't be a reason where we'd do this purposefully).

So pass this information on to Visitor implementation via these
optional start_union/end_union interfaces so this information
can be used to guard against the situation above. We will make
use of this information in a subsequent patch for the dealloc
visitor.

Cc: qemu-stable@nongnu.org
Reported-by: Fam Zheng <famz@redhat.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 include/qapi/visitor-impl.h |  2 ++
 include/qapi/visitor.h      |  2 ++
 qapi/qapi-visit-core.c      | 15 +++++++++++++++
 scripts/qapi-visit.py       |  6 ++++++
 4 files changed, 25 insertions(+)

diff --git a/include/qapi/visitor-impl.h b/include/qapi/visitor-impl.h
index ecc0183..09bb0fd 100644
--- a/include/qapi/visitor-impl.h
+++ b/include/qapi/visitor-impl.h
@@ -55,6 +55,8 @@ struct Visitor
     void (*type_int64)(Visitor *v, int64_t *obj, const char *name, Error **errp);
     /* visit_type_size() falls back to (*type_uint64)() if type_size is unset */
     void (*type_size)(Visitor *v, uint64_t *obj, const char *name, Error **errp);
+    bool (*start_union)(Visitor *v, bool data_present, Error **errp);
+    void (*end_union)(Visitor *v, bool data_present, Error **errp);
 };
 
 void input_type_enum(Visitor *v, int *obj, const char *strings[],
diff --git a/include/qapi/visitor.h b/include/qapi/visitor.h
index 4a0178f..5934f59 100644
--- a/include/qapi/visitor.h
+++ b/include/qapi/visitor.h
@@ -58,5 +58,7 @@ void visit_type_size(Visitor *v, uint64_t *obj, const char *name, Error **errp);
 void visit_type_bool(Visitor *v, bool *obj, const char *name, Error **errp);
 void visit_type_str(Visitor *v, char **obj, const char *name, Error **errp);
 void visit_type_number(Visitor *v, double *obj, const char *name, Error **errp);
+bool visit_start_union(Visitor *v, bool data_present, Error **errp);
+void visit_end_union(Visitor *v, bool data_present, Error **errp);
 
 #endif
diff --git a/qapi/qapi-visit-core.c b/qapi/qapi-visit-core.c
index 55f8d40..b66b93a 100644
--- a/qapi/qapi-visit-core.c
+++ b/qapi/qapi-visit-core.c
@@ -58,6 +58,21 @@ void visit_end_list(Visitor *v, Error **errp)
     v->end_list(v, errp);
 }
 
+bool visit_start_union(Visitor *v, bool data_present, Error **errp)
+{
+    if (v->start_union) {
+        return v->start_union(v, data_present, errp);
+    }
+    return true;
+}
+
+void visit_end_union(Visitor *v, bool data_present, Error **errp)
+{
+    if (v->end_union) {
+        v->end_union(v, data_present, errp);
+    }
+}
+
 void visit_optional(Visitor *v, bool *present, const char *name,
                     Error **errp)
 {
diff --git a/scripts/qapi-visit.py b/scripts/qapi-visit.py
index c129697..cfce31b 100644
--- a/scripts/qapi-visit.py
+++ b/scripts/qapi-visit.py
@@ -357,6 +357,9 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e
         if (err) {
             goto out_obj;
         }
+        if (!visit_start_union(m, !!(*obj)->data, &err) || err) {
+            goto out_obj;
+        }
         switch ((*obj)->kind) {
 ''',
                  disc_type = disc_type,
@@ -385,6 +388,9 @@ void visit_type_%(name)s(Visitor *m, %(name)s **obj, const char *name, Error **e
 out_obj:
         error_propagate(errp, err);
         err = NULL;
+        visit_end_union(m, !!(*obj)->data, &err);
+        error_propagate(errp, err);
+        err = NULL;
     }
     visit_end_struct(m, &err);
 out:
-- 
1.9.1

[Qemu-devel] [PATCH v2 2/4] qapi: dealloc visitor, implement visit_start_union

[Michael Roth]

If the .data field of a QAPI Union is NULL, we don't need to free
any of the union fields.

Make use of the new visit_start_union interface to access this
information and instruct the generated code to not visit these
fields when this occurs.

Cc: qemu-stable@nongnu.org
Reported-by: Fam Zheng <famz@redhat.com>
Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qapi/qapi-dealloc-visitor.c | 26 ++++++++++++++++++++++++++
 1 file changed, 26 insertions(+)

diff --git a/qapi/qapi-dealloc-visitor.c b/qapi/qapi-dealloc-visitor.c
index dc53545..a14a1c7 100644
--- a/qapi/qapi-dealloc-visitor.c
+++ b/qapi/qapi-dealloc-visitor.c
@@ -162,6 +162,31 @@ static void qapi_dealloc_type_enum(Visitor *v, int *obj, const char *strings[],
 {
 }
 
+/* If there's no data present, the dealloc visitor has nothing to free.
+ * Thus, indicate to visitor code that the subsequent union fields can
+ * be skipped. This is not an error condition, since the cleanup of the
+ * rest of an object can continue unhindered, so leave errp unset in
+ * these cases.
+ *
+ * NOTE: In cases where we're attempting to deallocate an object that
+ * may have missing fields, the field indicating the union type may
+ * be missing. In such a case, it's possible we don't have enough
+ * information to differentiate data_present == false from a case where
+ * data *is* present but happens to be a scalar with a value of 0.
+ * This is okay, since in the case of the dealloc visitor there's no
+ * work that needs to done in either situation.
+ *
+ * The current inability in QAPI code to more thoroughly verify a union
+ * type in such cases will likely need to be addressed if we wish to
+ * implement this interface for other types of visitors in the future,
+ * however.
+ */
+static bool qapi_dealloc_start_union(Visitor *v, bool data_present,
+                                     Error **errp)
+{
+    return data_present;
+}
+
 Visitor *qapi_dealloc_get_visitor(QapiDeallocVisitor *v)
 {
     return &v->visitor;
@@ -191,6 +216,7 @@ QapiDeallocVisitor *qapi_dealloc_visitor_new(void)
     v->visitor.type_str = qapi_dealloc_type_str;
     v->visitor.type_number = qapi_dealloc_type_number;
     v->visitor.type_size = qapi_dealloc_type_size;
+    v->visitor.start_union = qapi_dealloc_start_union;
 
     QTAILQ_INIT(&v->stack);
 
-- 
1.9.1

[Qemu-devel] [PATCH v2 3/4] tests: add QMP input visitor test for unions with no discriminator

[Michael Roth]

This more of an exercise of the dealloc visitor, where it may
erroneously use an uninitialized discriminator field as indication
that union fields corresponding to that discriminator field/type are
present, which can lead to attempts to free random chunks of heap
memory.

Cc: qemu-stable@nongnu.org
Reviewed-by: Eric Blake <eblake@redhat.com>
Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 tests/qapi-schema/qapi-schema-test.json | 10 ++++++++++
 tests/qapi-schema/qapi-schema-test.out  |  3 +++
 tests/test-qmp-input-strict.c           | 17 +++++++++++++++++
 3 files changed, 30 insertions(+)

diff --git a/tests/qapi-schema/qapi-schema-test.json b/tests/qapi-schema/qapi-schema-test.json
index ab4d3d9..d43b5fd 100644
--- a/tests/qapi-schema/qapi-schema-test.json
+++ b/tests/qapi-schema/qapi-schema-test.json
@@ -33,6 +33,9 @@
 { 'type': 'UserDefB',
   'data': { 'integer': 'int' } }
 
+{ 'type': 'UserDefC',
+  'data': { 'string1': 'str', 'string2': 'str' } }
+
 { 'union': 'UserDefUnion',
   'base': 'UserDefZero',
   'data': { 'a' : 'UserDefA', 'b' : 'UserDefB' } }
@@ -47,6 +50,13 @@
 # FIXME generated struct UserDefFlatUnion has members for direct base
 # UserDefOne, but lacks members for indirect base UserDefZero
 
+# this variant of UserDefFlatUnion defaults to a union that uses fields with
+# allocated types to test corner cases in the cleanup/dealloc visitor
+{ 'union': 'UserDefFlatUnion2',
+  'base': 'UserDefUnionBase',
+  'discriminator': 'enum1',
+  'data': { 'value1' : 'UserDefC', 'value2' : 'UserDefB', 'value3' : 'UserDefA' } }
+
 { 'union': 'UserDefAnonUnion',
   'discriminator': {},
   'data': { 'uda': 'UserDefA', 's': 'str', 'i': 'int' } }
diff --git a/tests/qapi-schema/qapi-schema-test.out b/tests/qapi-schema/qapi-schema-test.out
index 95e9899..08d7304 100644
--- a/tests/qapi-schema/qapi-schema-test.out
+++ b/tests/qapi-schema/qapi-schema-test.out
@@ -6,9 +6,11 @@
  OrderedDict([('type', 'UserDefNested'), ('data', OrderedDict([('string0', 'str'), ('dict1', OrderedDict([('string1', 'str'), ('dict2', OrderedDict([('userdef1', 'UserDefOne'), ('string2', 'str')])), ('*dict3', OrderedDict([('userdef2', 'UserDefOne'), ('string3', 'str')]))]))]))]),
  OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
  OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
+ OrderedDict([('type', 'UserDefC'), ('data', OrderedDict([('string1', 'str'), ('string2', 'str')]))]),
  OrderedDict([('union', 'UserDefUnion'), ('base', 'UserDefZero'), ('data', OrderedDict([('a', 'UserDefA'), ('b', 'UserDefB')]))]),
  OrderedDict([('type', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
  OrderedDict([('union', 'UserDefFlatUnion'), ('base', 'UserDefUnionBase'), ('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'UserDefA'), ('value2', 'UserDefB'), ('value3', 'UserDefB')]))]),
+ OrderedDict([('union', 'UserDefFlatUnion2'), ('base', 'UserDefUnionBase'), ('discriminator', 'enum1'), ('data', OrderedDict([('value1', 'UserDefC'), ('value2', 'UserDefB'), ('value3', 'UserDefA')]))]),
  OrderedDict([('union', 'UserDefAnonUnion'), ('discriminator', OrderedDict()), ('data', OrderedDict([('uda', 'UserDefA'), ('s', 'str'), ('i', 'int')]))]),
  OrderedDict([('union', 'UserDefNativeListUnion'), ('data', OrderedDict([('integer', ['int']), ('s8', ['int8']), ('s16', ['int16']), ('s32', ['int32']), ('s64', ['int64']), ('u8', ['uint8']), ('u16', ['uint16']), ('u32', ['uint32']), ('u64', ['uint64']), ('number', ['number']), ('boolean', ['bool']), ('string', ['str'])]))]),
  OrderedDict([('command', 'user_def_cmd'), ('data', OrderedDict())]),
@@ -32,6 +34,7 @@
  OrderedDict([('type', 'UserDefNested'), ('data', OrderedDict([('string0', 'str'), ('dict1', OrderedDict([('string1', 'str'), ('dict2', OrderedDict([('userdef1', 'UserDefOne'), ('string2', 'str')])), ('*dict3', OrderedDict([('userdef2', 'UserDefOne'), ('string3', 'str')]))]))]))]),
  OrderedDict([('type', 'UserDefA'), ('data', OrderedDict([('boolean', 'bool')]))]),
  OrderedDict([('type', 'UserDefB'), ('data', OrderedDict([('integer', 'int')]))]),
+ OrderedDict([('type', 'UserDefC'), ('data', OrderedDict([('string1', 'str'), ('string2', 'str')]))]),
  OrderedDict([('type', 'UserDefUnionBase'), ('data', OrderedDict([('string', 'str'), ('enum1', 'EnumOne')]))]),
  OrderedDict([('type', 'UserDefOptions'), ('data', OrderedDict([('*i64', ['int']), ('*u64', ['uint64']), ('*u16', ['uint16']), ('*i64x', 'int'), ('*u64x', 'uint64')]))]),
  OrderedDict([('type', 'EventStructOne'), ('data', OrderedDict([('struct1', 'UserDefOne'), ('string', 'str'), ('*enum2', 'EnumOne')]))])]
diff --git a/tests/test-qmp-input-strict.c b/tests/test-qmp-input-strict.c
index 0f77003..d5360c6 100644
--- a/tests/test-qmp-input-strict.c
+++ b/tests/test-qmp-input-strict.c
@@ -260,6 +260,21 @@ static void test_validate_fail_union_flat(TestInputVisitorData *data,
     qapi_free_UserDefFlatUnion(tmp);
 }
 
+static void test_validate_fail_union_flat_no_discrim(TestInputVisitorData *data,
+                                                     const void *unused)
+{
+    UserDefFlatUnion2 *tmp = NULL;
+    Error *err = NULL;
+    Visitor *v;
+
+    /* test situation where discriminator field ('enum1' here) is missing */
+    v = validate_test_init(data, "{ 'string': 'c', 'string1': 'd', 'string2': 'e' }");
+
+    visit_type_UserDefFlatUnion2(v, &tmp, NULL, &err);
+    g_assert(err);
+    qapi_free_UserDefFlatUnion2(tmp);
+}
+
 static void test_validate_fail_union_anon(TestInputVisitorData *data,
                                           const void *unused)
 {
@@ -310,6 +325,8 @@ int main(int argc, char **argv)
                        &testdata, test_validate_fail_union);
     validate_test_add("/visitor/input-strict/fail/union-flat",
                        &testdata, test_validate_fail_union_flat);
+    validate_test_add("/visitor/input-strict/fail/union-flat-no-discriminator",
+                       &testdata, test_validate_fail_union_flat_no_discrim);
     validate_test_add("/visitor/input-strict/fail/union-anon",
                        &testdata, test_validate_fail_union_anon);
 
-- 
1.9.1

[Qemu-devel] [PATCH v2 4/4] qemu-iotests: Test missing "driver" key for blockdev-add

[Michael Roth]

From: Fam Zheng <famz@redhat.com>

Signed-off-by: Fam Zheng <famz@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Cc: qemu-stable@nongnu.org
Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 tests/qemu-iotests/087     | 17 +++++++++++++++++
 tests/qemu-iotests/087.out | 13 +++++++++++++
 2 files changed, 30 insertions(+)

diff --git a/tests/qemu-iotests/087 b/tests/qemu-iotests/087
index 82c56b1..d7454d1 100755
--- a/tests/qemu-iotests/087
+++ b/tests/qemu-iotests/087
@@ -218,6 +218,23 @@ run_qemu <<EOF
 { "execute": "quit" }
 EOF
 
+echo
+echo === Missing driver ===
+echo
+
+_make_test_img -o encryption=on $size
+run_qemu -S <<EOF
+{ "execute": "qmp_capabilities" }
+{ "execute": "blockdev-add",
+  "arguments": {
+      "options": {
+        "id": "disk"
+      }
+    }
+  }
+{ "execute": "quit" }
+EOF
+
 # success, all done
 echo "*** done"
 rm -f $seq.full
diff --git a/tests/qemu-iotests/087.out b/tests/qemu-iotests/087.out
index 7fbee3f..f16bad0 100644
--- a/tests/qemu-iotests/087.out
+++ b/tests/qemu-iotests/087.out
@@ -64,4 +64,17 @@ QMP_VERSION
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
 {"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}
 
+
+=== Missing driver ===
+
+Formatting 'TEST_DIR/t.IMGFMT', fmt=IMGFMT size=134217728 encryption=on 
+Testing: -S
+QMP_VERSION
+{"return": {}}
+{"error": {"class": "GenericError", "desc": "Invalid parameter type for 'driver', expected: string"}}
+{"return": {}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "SHUTDOWN"}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "ide1-cd0", "tray-open": true}}
+{"timestamp": {"seconds":  TIMESTAMP, "microseconds":  TIMESTAMP}, "event": "DEVICE_TRAY_MOVED", "data": {"device": "floppy0", "tray-open": true}}
+
 *** done
-- 
1.9.1

Re: [Qemu-devel] [PATCH v2 2/4] qapi: dealloc visitor, implement visit_start_union

[Eric Blake]

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

On 09/17/2014 03:32 PM, Michael Roth wrote:
> If the .data field of a QAPI Union is NULL, we don't need to free
> any of the union fields.
> 
> Make use of the new visit_start_union interface to access this
> information and instruct the generated code to not visit these
> fields when this occurs.
> 
> Cc: qemu-stable@nongnu.org
> Reported-by: Fam Zheng <famz@redhat.com>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  qapi/qapi-dealloc-visitor.c | 26 ++++++++++++++++++++++++++
>  1 file changed, 26 insertions(+)
> 

The comment helps.

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: 539 bytes --]

Re: [Qemu-devel] [PATCH v2 3/4] tests: add QMP input visitor test for unions with no discriminator

[Eric Blake]

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

On 09/17/2014 03:32 PM, Michael Roth wrote:
> This more of an exercise of the dealloc visitor, where it may

s/This more/This is more/

> erroneously use an uninitialized discriminator field as indication
> that union fields corresponding to that discriminator field/type are
> present, which can lead to attempts to free random chunks of heap
> memory.
> 
> Cc: qemu-stable@nongnu.org
> Reviewed-by: Eric Blake <eblake@redhat.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  tests/qapi-schema/qapi-schema-test.json | 10 ++++++++++
>  tests/qapi-schema/qapi-schema-test.out  |  3 +++
>  tests/test-qmp-input-strict.c           | 17 +++++++++++++++++
>  3 files changed, 30 insertions(+)
> 

R-b still stands.

-- 
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: 539 bytes --]

Re: [Qemu-devel] [PATCH v2 1/4] qapi: add visit_start_union and visit_end_union

[Eric Blake]

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

On 09/17/2014 03:32 PM, Michael Roth wrote:
> In some cases an input visitor might bail out on filling out a
> struct for various reasons, such as missing fields when running
> in strict mode. In the case of a QAPI Union type, this may lead
> to cases where the .kind field which encodes the union type
> is uninitialized. Subsequently, other visitors, such as the
> dealloc visitor, may use this .kind value as if it were
> initialized, leading to assumptions about the union type which
> in this case may lead to segfaults. For example, freeing an
> integer value.
> 
> However, we can generally rely on the fact that the always-present
> .data void * field that we generate for these union types will
> always be NULL in cases where .kind is uninitialized (at least,
> there shouldn't be a reason where we'd do this purposefully).
> 
> So pass this information on to Visitor implementation via these
> optional start_union/end_union interfaces so this information
> can be used to guard against the situation above. We will make
> use of this information in a subsequent patch for the dealloc
> visitor.
> 
> Cc: qemu-stable@nongnu.org
> Reported-by: Fam Zheng <famz@redhat.com>
> Suggested-by: Paolo Bonzini <pbonzini@redhat.com>
> Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---

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

>  
> +bool visit_start_union(Visitor *v, bool data_present, Error **errp)
> +{
> +    if (v->start_union) {
> +        return v->start_union(v, data_present, errp);
> +    }
> +    return true;
> +}

So we default to returning true (which implies safe to visit the union
fields), and patch 2 creates the only case where this returns false
(when data_present is false).  I also note that errp is never set by
this series, but it's fine to wire it up in anticipation of any future
need.  Took me a couple reads to get what's happening, but I agree with
the results.

-- 
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: 539 bytes --]

Re: [Qemu-devel] [PATCH v2 3/4] tests: add QMP input visitor test for unions with no discriminator

[Michael Roth]

Quoting Eric Blake (2014-09-17 17:33:36)
> On 09/17/2014 03:32 PM, Michael Roth wrote:
> > This more of an exercise of the dealloc visitor, where it may
> 
> s/This more/This is more/
> 
> > erroneously use an uninitialized discriminator field as indication
> > that union fields corresponding to that discriminator field/type are
> > present, which can lead to attempts to free random chunks of heap
> > memory.
> > 
> > Cc: qemu-stable@nongnu.org
> > Reviewed-by: Eric Blake <eblake@redhat.com>
> > Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
> > Signed-off-by: Michael Roth <mdroth@linux.vnet.ibm.com>
> > ---
> >  tests/qapi-schema/qapi-schema-test.json | 10 ++++++++++
> >  tests/qapi-schema/qapi-schema-test.out  |  3 +++
> >  tests/test-qmp-input-strict.c           | 17 +++++++++++++++++
> >  3 files changed, 30 insertions(+)
> > 
> 
> R-b still stands.

Thanks, will send a v3 in a moment just to keep things clean.

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