* [Qemu-devel] [PATCH v3 0/4] qapi: fix crash in dealloc visitor for union types
@ 2014-09-18 20:36 Michael Roth
2014-09-18 20:36 ` [Qemu-devel] [PATCH v3 1/4] qapi: add visit_start_union and visit_end_union Michael Roth
` (4 more replies)
0 siblings, 5 replies; 7+ messages in thread
From: Michael Roth @ 2014-09-18 20:36 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, armbru, qemu-stable, lcapitulino, pbonzini
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
v3:
* fixed up commit msg in patch #2 (Eric)
* rebased on latest master
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(+)
^ permalink raw reply [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v3 1/4] qapi: add visit_start_union and visit_end_union
2014-09-18 20:36 [Qemu-devel] [PATCH v3 0/4] qapi: fix crash in dealloc visitor for union types Michael Roth
@ 2014-09-18 20:36 ` Michael Roth
2014-09-18 20:36 ` [Qemu-devel] [PATCH v3 2/4] qapi: dealloc visitor, implement visit_start_union Michael Roth
` (3 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Michael Roth @ 2014-09-18 20:36 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, armbru, qemu-stable, lcapitulino, pbonzini
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>
Reviewed-by: Eric Blake <eblake@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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v3 2/4] qapi: dealloc visitor, implement visit_start_union
2014-09-18 20:36 [Qemu-devel] [PATCH v3 0/4] qapi: fix crash in dealloc visitor for union types Michael Roth
2014-09-18 20:36 ` [Qemu-devel] [PATCH v3 1/4] qapi: add visit_start_union and visit_end_union Michael Roth
@ 2014-09-18 20:36 ` Michael Roth
2014-09-18 20:36 ` [Qemu-devel] [PATCH v3 3/4] tests: add QMP input visitor test for unions with no discriminator Michael Roth
` (2 subsequent siblings)
4 siblings, 0 replies; 7+ messages in thread
From: Michael Roth @ 2014-09-18 20:36 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, armbru, qemu-stable, lcapitulino, pbonzini
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>
Reviewed-by: Eric Blake <eblake@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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v3 3/4] tests: add QMP input visitor test for unions with no discriminator
2014-09-18 20:36 [Qemu-devel] [PATCH v3 0/4] qapi: fix crash in dealloc visitor for union types Michael Roth
2014-09-18 20:36 ` [Qemu-devel] [PATCH v3 1/4] qapi: add visit_start_union and visit_end_union Michael Roth
2014-09-18 20:36 ` [Qemu-devel] [PATCH v3 2/4] qapi: dealloc visitor, implement visit_start_union Michael Roth
@ 2014-09-18 20:36 ` Michael Roth
2014-09-18 20:36 ` [Qemu-devel] [PATCH v3 4/4] qemu-iotests: Test missing "driver" key for blockdev-add Michael Roth
2014-09-19 17:52 ` [Qemu-devel] [PATCH v3 0/4] qapi: fix crash in dealloc visitor for union types Luiz Capitulino
4 siblings, 0 replies; 7+ messages in thread
From: Michael Roth @ 2014-09-18 20:36 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, armbru, qemu-stable, lcapitulino, pbonzini
This is 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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* [Qemu-devel] [PATCH v3 4/4] qemu-iotests: Test missing "driver" key for blockdev-add
2014-09-18 20:36 [Qemu-devel] [PATCH v3 0/4] qapi: fix crash in dealloc visitor for union types Michael Roth
` (2 preceding siblings ...)
2014-09-18 20:36 ` [Qemu-devel] [PATCH v3 3/4] tests: add QMP input visitor test for unions with no discriminator Michael Roth
@ 2014-09-18 20:36 ` Michael Roth
2014-09-19 17:52 ` [Qemu-devel] [PATCH v3 0/4] qapi: fix crash in dealloc visitor for union types Luiz Capitulino
4 siblings, 0 replies; 7+ messages in thread
From: Michael Roth @ 2014-09-18 20:36 UTC (permalink / raw)
To: qemu-devel; +Cc: famz, armbru, qemu-stable, lcapitulino, pbonzini
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
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/4] qapi: fix crash in dealloc visitor for union types
2014-09-18 20:36 [Qemu-devel] [PATCH v3 0/4] qapi: fix crash in dealloc visitor for union types Michael Roth
` (3 preceding siblings ...)
2014-09-18 20:36 ` [Qemu-devel] [PATCH v3 4/4] qemu-iotests: Test missing "driver" key for blockdev-add Michael Roth
@ 2014-09-19 17:52 ` Luiz Capitulino
2014-09-19 18:21 ` Michael Roth
4 siblings, 1 reply; 7+ messages in thread
From: Luiz Capitulino @ 2014-09-19 17:52 UTC (permalink / raw)
To: Michael Roth; +Cc: famz, qemu-stable, armbru, qemu-devel, pbonzini
On Thu, 18 Sep 2014 15:36:39 -0500
Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
> 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
Applied to the qmp branch, thanks.
PS: Michael, I'm assuming you were not planning to send a pull yourself.
If that's not the case, please let me know.
>
> v3:
> * fixed up commit msg in patch #2 (Eric)
> * rebased on latest master
>
> 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(+)
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Qemu-devel] [PATCH v3 0/4] qapi: fix crash in dealloc visitor for union types
2014-09-19 17:52 ` [Qemu-devel] [PATCH v3 0/4] qapi: fix crash in dealloc visitor for union types Luiz Capitulino
@ 2014-09-19 18:21 ` Michael Roth
0 siblings, 0 replies; 7+ messages in thread
From: Michael Roth @ 2014-09-19 18:21 UTC (permalink / raw)
To: Luiz Capitulino; +Cc: famz, qemu-stable, armbru, qemu-devel, pbonzini
Quoting Luiz Capitulino (2014-09-19 12:52:17)
> On Thu, 18 Sep 2014 15:36:39 -0500
> Michael Roth <mdroth@linux.vnet.ibm.com> wrote:
>
> > 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
>
> Applied to the qmp branch, thanks.
>
> PS: Michael, I'm assuming you were not planning to send a pull yourself.
> If that's not the case, please let me know.
Correct, was planning to check with you first. Thanks!
>
> >
> > v3:
> > * fixed up commit msg in patch #2 (Eric)
> > * rebased on latest master
> >
> > 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(+)
> >
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2014-09-19 18:22 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-09-18 20:36 [Qemu-devel] [PATCH v3 0/4] qapi: fix crash in dealloc visitor for union types Michael Roth
2014-09-18 20:36 ` [Qemu-devel] [PATCH v3 1/4] qapi: add visit_start_union and visit_end_union Michael Roth
2014-09-18 20:36 ` [Qemu-devel] [PATCH v3 2/4] qapi: dealloc visitor, implement visit_start_union Michael Roth
2014-09-18 20:36 ` [Qemu-devel] [PATCH v3 3/4] tests: add QMP input visitor test for unions with no discriminator Michael Roth
2014-09-18 20:36 ` [Qemu-devel] [PATCH v3 4/4] qemu-iotests: Test missing "driver" key for blockdev-add Michael Roth
2014-09-19 17:52 ` [Qemu-devel] [PATCH v3 0/4] qapi: fix crash in dealloc visitor for union types Luiz Capitulino
2014-09-19 18:21 ` Michael Roth
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).