* [Qemu-devel] [PULL 2/4] qapi: Eliminate indirection through qmp_event_get_func_emit()
2019-01-24 14:38 [Qemu-devel] [PULL 0/4] QAPI patches for 2019-01-24 Markus Armbruster
2019-01-24 14:38 ` [Qemu-devel] [PULL 1/4] qapi: Belatedly update docs for commit 9c2f56e9f9d Markus Armbruster
@ 2019-01-24 14:38 ` Markus Armbruster
2019-01-24 14:38 ` [Qemu-devel] [PULL 3/4] qmp: Add examples to qom list, get, and set commands Markus Armbruster
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2019-01-24 14:38 UTC (permalink / raw)
To: qemu-devel
The qapi_event_send_FOO() functions emit events like this:
QMPEventFuncEmit emit;
emit = qmp_event_get_func_emit();
if (!emit) {
return;
}
qmp = qmp_event_build_dict("FOO");
[put event arguments into @qmp...]
emit(QAPI_EVENT_FOO, qmp);
The value of qmp_event_get_func_emit() depends only on the program:
* In qemu-system-FOO, it's always monitor_qapi_event_queue.
* In tests/test-qmp-event, it's always event_test_emit.
* In all other programs, it's always null.
This is exactly the kind of dependence the linker is supposed to
resolve; we don't actually need an indirection.
Note that things would fall apart if we linked more than one QAPI
schema into a single program: each set of qapi_event_send_FOO() uses
its own event enumeration, yet they share a single emit function.
Which takes the event enumeration as an argument. Which one if
there's more than one?
More seriously: how does this work even now? qemu-system-FOO wants
QAPIEvent, and passes a function taking that to
qmp_event_set_func_emit(). test-qmp-event wants test_QAPIEvent, and
passes a function taking that to qmp_event_set_func_emit().
It works by type trickery, of course:
typedef void (*QMPEventFuncEmit)(unsigned event, QDict *dict);
void qmp_event_set_func_emit(QMPEventFuncEmit emit);
QMPEventFuncEmit qmp_event_get_func_emit(void);
We use unsigned instead of the enumeration type. Relies on both
enumerations boiling down to unsigned, which happens to be true for
the compilers we use.
Clean this up as follows:
* Generate qapi_event_send_FOO() that call PREFIX_qapi_event_emit()
instead of the value of qmp_event_set_func_emit().
* Generate a prototype for PREFIX_qapi_event_emit() into
qapi-events.h.
* PREFIX_ is empty for qapi/qapi-schema.json, and test_ for
tests/qapi-schema/qapi-schema-test.json. It's qga_ for
qga/qapi-schema.json, and doc-good- for
tests/qapi-schema/doc-good.json, but those don't define any events.
* Rename monitor_qapi_event_queue() to qapi_event_emit() instead of
passing it to qmp_event_set_func_emit(). This takes care of
qemu-system-FOO.
* Rename event_test_emit() to test_qapi_event_emit() instead of
passing it to qmp_event_set_func_emit(). This takes care of
tests/test-qmp-event.
* Add a qapi_event_emit() that does nothing to stubs/monitor.c. This
takes care of all other programs that link code emitting QMP events.
* Drop qmp_event_set_func_emit(), qmp_event_get_func_emit().
Signed-off-by: Markus Armbruster <armbru@redhat.com>
Message-Id: <20181218182234.28876-3-armbru@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
[Commit message typos fixed]
---
docs/devel/qapi-code-gen.txt | 8 +-------
include/qapi/qmp-event.h | 6 ------
monitor.c | 4 +---
qapi/qmp-event.c | 12 ------------
scripts/qapi/events.py | 24 ++++++++++++++----------
stubs/monitor.c | 5 +++++
tests/Makefile.include | 4 ++--
tests/test-qmp-event.c | 6 +-----
8 files changed, 24 insertions(+), 45 deletions(-)
diff --git a/docs/devel/qapi-code-gen.txt b/docs/devel/qapi-code-gen.txt
index 418a607842..87183d3a09 100644
--- a/docs/devel/qapi-code-gen.txt
+++ b/docs/devel/qapi-code-gen.txt
@@ -1385,16 +1385,10 @@ Example:
void qapi_event_send_my_event(void)
{
QDict *qmp;
- QMPEventFuncEmit emit;
-
- emit = qmp_event_get_func_emit();
- if (!emit) {
- return;
- }
qmp = qmp_event_build_dict("MY_EVENT");
- emit(EXAMPLE_QAPI_EVENT_MY_EVENT, qmp);
+ example_qapi_event_emit(EXAMPLE_QAPI_EVENT_MY_EVENT, qmp);
qobject_unref(qmp);
}
diff --git a/include/qapi/qmp-event.h b/include/qapi/qmp-event.h
index 23e588ccf8..b60f1d3a89 100644
--- a/include/qapi/qmp-event.h
+++ b/include/qapi/qmp-event.h
@@ -14,11 +14,5 @@
#ifndef QMP_EVENT_H
#define QMP_EVENT_H
-typedef void (*QMPEventFuncEmit)(unsigned event, QDict *dict);
-
-void qmp_event_set_func_emit(QMPEventFuncEmit emit);
-
-QMPEventFuncEmit qmp_event_get_func_emit(void);
-
QDict *qmp_event_build_dict(const char *event_name);
#endif
diff --git a/monitor.c b/monitor.c
index eb39fb015b..c09fa63940 100644
--- a/monitor.c
+++ b/monitor.c
@@ -590,8 +590,7 @@ monitor_qapi_event_queue_no_reenter(QAPIEvent event, QDict *qdict)
qemu_mutex_unlock(&monitor_lock);
}
-static void
-monitor_qapi_event_queue(QAPIEvent event, QDict *qdict)
+void qapi_event_emit(QAPIEvent event, QDict *qdict)
{
/*
* monitor_qapi_event_queue_no_reenter() is not reentrant: it
@@ -704,7 +703,6 @@ static void monitor_qapi_event_init(void)
{
monitor_qapi_event_state = g_hash_table_new(qapi_event_throttle_hash,
qapi_event_throttle_equal);
- qmp_event_set_func_emit(monitor_qapi_event_queue);
}
static void handle_hmp_command(Monitor *mon, const char *cmdline);
diff --git a/qapi/qmp-event.c b/qapi/qmp-event.c
index 5b8854043e..81ddd5331f 100644
--- a/qapi/qmp-event.c
+++ b/qapi/qmp-event.c
@@ -19,18 +19,6 @@
#include "qapi/qmp/qdict.h"
#include "qapi/qmp/qjson.h"
-static QMPEventFuncEmit qmp_emit;
-
-void qmp_event_set_func_emit(QMPEventFuncEmit emit)
-{
- qmp_emit = emit;
-}
-
-QMPEventFuncEmit qmp_event_get_func_emit(void)
-{
- return qmp_emit;
-}
-
static void timestamp_put(QDict *qdict)
{
int err;
diff --git a/scripts/qapi/events.py b/scripts/qapi/events.py
index 37ee5de682..d86a2d2b3e 100644
--- a/scripts/qapi/events.py
+++ b/scripts/qapi/events.py
@@ -58,7 +58,7 @@ def gen_param_var(typ):
return ret
-def gen_event_send(name, arg_type, boxed, event_enum_name):
+def gen_event_send(name, arg_type, boxed, event_enum_name, event_emit):
# FIXME: Our declaration of local variables (and of 'errp' in the
# parameter list) can collide with exploded members of the event's
# data type passed in as parameters. If this collision ever hits in
@@ -70,7 +70,6 @@ def gen_event_send(name, arg_type, boxed, event_enum_name):
%(proto)s
{
QDict *qmp;
- QMPEventFuncEmit emit;
''',
proto=build_event_send_proto(name, arg_type, boxed))
@@ -86,11 +85,6 @@ def gen_event_send(name, arg_type, boxed, event_enum_name):
ret += mcgen('''
- emit = qmp_event_get_func_emit();
- if (!emit) {
- return;
- }
-
qmp = qmp_event_build_dict("%(name)s");
''',
@@ -121,9 +115,10 @@ def gen_event_send(name, arg_type, boxed, event_enum_name):
''')
ret += mcgen('''
- emit(%(c_enum)s, qmp);
+ %(event_emit)s(%(c_enum)s, qmp);
''',
+ event_emit=event_emit,
c_enum=c_enum_const(event_enum_name, name))
if arg_type and not arg_type.is_empty():
@@ -145,6 +140,7 @@ class QAPISchemaGenEventVisitor(QAPISchemaModularCVisitor):
' * Schema-defined QAPI/QMP events', __doc__)
self._event_enum_name = c_name(prefix + 'QAPIEvent', protect=False)
self._event_enum_members = []
+ self._event_emit_name = c_name(prefix + 'qapi_event_emit')
def _begin_module(self, name):
types = self._module_basename('qapi-types', name)
@@ -170,15 +166,23 @@ class QAPISchemaGenEventVisitor(QAPISchemaModularCVisitor):
def visit_end(self):
(genc, genh) = self._module[self._main_module]
- genh.add(gen_enum(self._event_enum_name, self._event_enum_members))
+ genh.add(gen_enum(self._event_enum_name,
+ self._event_enum_members))
genc.add(gen_enum_lookup(self._event_enum_name,
self._event_enum_members))
+ genh.add(mcgen('''
+
+void %(event_emit)s(%(event_enum)s event, QDict *qdict);
+''',
+ event_emit=self._event_emit_name,
+ event_enum=self._event_enum_name))
def visit_event(self, name, info, ifcond, arg_type, boxed):
with ifcontext(ifcond, self._genh, self._genc):
self._genh.add(gen_event_send_decl(name, arg_type, boxed))
self._genc.add(gen_event_send(name, arg_type, boxed,
- self._event_enum_name))
+ self._event_enum_name,
+ self._event_emit_name))
self._event_enum_members.append(QAPISchemaMember(name, ifcond))
diff --git a/stubs/monitor.c b/stubs/monitor.c
index 3890771bb5..32bd7012c3 100644
--- a/stubs/monitor.c
+++ b/stubs/monitor.c
@@ -1,5 +1,6 @@
#include "qemu/osdep.h"
#include "qapi/error.h"
+#include "qapi/qapi-events.h"
#include "qemu-common.h"
#include "monitor/monitor.h"
@@ -14,3 +15,7 @@ int monitor_get_fd(Monitor *mon, const char *name, Error **errp)
void monitor_init(Chardev *chr, int flags)
{
}
+
+void qapi_event_emit(QAPIEvent event, QDict *qdict)
+{
+}
diff --git a/tests/Makefile.include b/tests/Makefile.include
index 4eea38ae99..3453579af3 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -528,7 +528,7 @@ QEMU_CFLAGS += -I$(SRC_PATH)/tests
test-util-obj-y = libqemuutil.a
test-qom-obj-y = $(qom-obj-y) $(test-util-obj-y)
test-qapi-obj-y = tests/test-qapi-visit.o tests/test-qapi-types.o \
- tests/test-qapi-events.o tests/test-qapi-introspect.o \
+ tests/test-qapi-introspect.o \
$(test-qom-obj-y)
benchmark-crypto-obj-y = $(crypto-obj-y) $(test-qom-obj-y)
test-crypto-obj-y = $(crypto-obj-y) $(test-qom-obj-y)
@@ -620,7 +620,7 @@ tests/qapi-schema/doc-good.test.texi: $(SRC_PATH)/tests/qapi-schema/doc-good.jso
tests/test-string-output-visitor$(EXESUF): tests/test-string-output-visitor.o $(test-qapi-obj-y)
tests/test-string-input-visitor$(EXESUF): tests/test-string-input-visitor.o $(test-qapi-obj-y)
-tests/test-qmp-event$(EXESUF): tests/test-qmp-event.o $(test-qapi-obj-y)
+tests/test-qmp-event$(EXESUF): tests/test-qmp-event.o $(test-qapi-obj-y) tests/test-qapi-events.o
tests/test-qobject-output-visitor$(EXESUF): tests/test-qobject-output-visitor.o $(test-qapi-obj-y)
tests/test-clone-visitor$(EXESUF): tests/test-clone-visitor.o $(test-qapi-obj-y)
tests/test-qobject-input-visitor$(EXESUF): tests/test-qobject-input-visitor.o $(test-qapi-obj-y)
diff --git a/tests/test-qmp-event.c b/tests/test-qmp-event.c
index 9cddd72adb..bf900f14f4 100644
--- a/tests/test-qmp-event.c
+++ b/tests/test-qmp-event.c
@@ -93,9 +93,7 @@ static bool qdict_cmp_simple(QDict *a, QDict *b)
return d.result;
}
-/* This function is hooked as final emit function, which can verify the
- correctness. */
-static void event_test_emit(test_QAPIEvent event, QDict *d)
+void test_qapi_event_emit(test_QAPIEvent event, QDict *d)
{
QDict *t;
int64_t s, ms;
@@ -241,8 +239,6 @@ static void test_event_d(TestEventData *data,
int main(int argc, char **argv)
{
- qmp_event_set_func_emit(event_test_emit);
-
g_test_init(&argc, &argv, NULL);
event_test_add("/event/event_a", test_event_a);
--
2.17.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PULL 3/4] qmp: Add examples to qom list, get, and set commands
2019-01-24 14:38 [Qemu-devel] [PULL 0/4] QAPI patches for 2019-01-24 Markus Armbruster
2019-01-24 14:38 ` [Qemu-devel] [PULL 1/4] qapi: Belatedly update docs for commit 9c2f56e9f9d Markus Armbruster
2019-01-24 14:38 ` [Qemu-devel] [PULL 2/4] qapi: Eliminate indirection through qmp_event_get_func_emit() Markus Armbruster
@ 2019-01-24 14:38 ` Markus Armbruster
2019-01-24 14:38 ` [Qemu-devel] [PULL 4/4] json: Fix % handling when not interpolating Markus Armbruster
2019-01-25 12:30 ` [Qemu-devel] [PULL 0/4] QAPI patches for 2019-01-24 Peter Maydell
4 siblings, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2019-01-24 14:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Wainer dos Santos Moschetta
From: Wainer dos Santos Moschetta <wainersm@redhat.com>
Added examples for the qom-list, qom-get, and qom-set
commands in qapi misc JSON file.
Signed-off-by: Wainer dos Santos Moschetta <wainersm@redhat.com>
Message-Id: <20181228194442.3506-1-wainersm@redhat.com>
Reviewed-by: Markus Armbruster <armbru@redhat.com>
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qapi/misc.json | 36 ++++++++++++++++++++++++++++++++++++
1 file changed, 36 insertions(+)
diff --git a/qapi/misc.json b/qapi/misc.json
index 24d20a880a..426274ecf8 100644
--- a/qapi/misc.json
+++ b/qapi/misc.json
@@ -1380,6 +1380,16 @@
# object.
#
# Since: 1.2
+#
+# Example:
+#
+# -> { "execute": "qom-list",
+# "arguments": { "path": "/chardevs" } }
+# <- { "return": [ { "name": "type", "type": "string" },
+# { "name": "parallel0", "type": "child<chardev-vc>" },
+# { "name": "serial0", "type": "child<chardev-vc>" },
+# { "name": "mon0", "type": "child<chardev-stdio>" } ] }
+#
##
{ 'command': 'qom-list',
'data': { 'path': 'str' },
@@ -1417,6 +1427,23 @@
# returned as #int.
#
# Since: 1.2
+#
+# Example:
+#
+# 1. Use absolute path
+#
+# -> { "execute": "qom-get",
+# "arguments": { "path": "/machine/unattached/device[0]",
+# "property": "hotplugged" } }
+# <- { "return": false }
+#
+# 2. Use partial path
+#
+# -> { "execute": "qom-get",
+# "arguments": { "path": "unattached/sysbus",
+# "property": "type" } }
+# <- { "return": "System" }
+#
##
{ 'command': 'qom-get',
'data': { 'path': 'str', 'property': 'str' },
@@ -1436,6 +1463,15 @@
# for a description of type mapping.
#
# Since: 1.2
+#
+# Example:
+#
+# -> { "execute": "qom-set",
+# "arguments": { "path": "/machine",
+# "property": "graphics",
+# "value": false } }
+# <- { "return": {} }
+#
##
{ 'command': 'qom-set',
'data': { 'path': 'str', 'property': 'str', 'value': 'any' },
--
2.17.2
^ permalink raw reply related [flat|nested] 6+ messages in thread* [Qemu-devel] [PULL 4/4] json: Fix % handling when not interpolating
2019-01-24 14:38 [Qemu-devel] [PULL 0/4] QAPI patches for 2019-01-24 Markus Armbruster
` (2 preceding siblings ...)
2019-01-24 14:38 ` [Qemu-devel] [PULL 3/4] qmp: Add examples to qom list, get, and set commands Markus Armbruster
@ 2019-01-24 14:38 ` Markus Armbruster
2019-01-25 12:30 ` [Qemu-devel] [PULL 0/4] QAPI patches for 2019-01-24 Peter Maydell
4 siblings, 0 replies; 6+ messages in thread
From: Markus Armbruster @ 2019-01-24 14:38 UTC (permalink / raw)
To: qemu-devel; +Cc: Christophe Fergeau, qemu-stable
From: Christophe Fergeau <cfergeau@redhat.com>
Commit 8bca4613 added support for %% in json strings when interpolating,
but in doing so broke handling of % when not interpolating.
When parse_string() is fed a string token containing '%', it skips the
'%' regardless of ctxt->ap, i.e. even it's not interpolating. If the
'%' is the string's last character, it fails an assertion. Else, it
"merely" swallows the '%'.
Fix parse_string() to handle '%' specially only when interpolating.
To gauge the bug's impact, let's review non-interpolating users of this
parser, i.e. code passing NULL context to json_message_parser_init():
* tests/check-qjson.c, tests/test-qobject-input-visitor.c,
tests/test-visitor-serialization.c
Plenty of tests, but we still failed to cover the buggy case.
* monitor.c: QMP input
* qga/main.c: QGA input
* qobject_from_json():
- qobject-input-visitor.c: JSON command line option arguments of
-display and -blockdev
Reproducer: -blockdev '{"%"}'
- block.c: JSON pseudo-filenames starting with "json:"
Reproducer: https://bugzilla.redhat.com/show_bug.cgi?id=1668244#c3
- block/rbd.c: JSON key pairs
Pseudo-filenames starting with "rbd:".
Command line, QMP and QGA input are trusted.
Filenames are trusted when they come from command line, QMP or HMP.
They are untrusted when they come from from image file headers.
Example: QCOW2 backing file name. Note that this is *not* the security
boundary between host and guest. It's the boundary between host and an
image file from an untrusted source.
Neither failing an assertion nor skipping a character in a filename of
your choice looks exploitable. Note that we don't support compiling
with NDEBUG.
Fixes: 8bca4613e6cddd948895b8db3def05950463495b
Cc: qemu-stable@nongnu.org
Signed-off-by: Christophe Fergeau <cfergeau@redhat.com>
Message-Id: <20190102140535.11512-1-cfergeau@redhat.com>
Reviewed-by: Eric Blake <eblake@redhat.com>
Tested-by: Richard W.M. Jones <rjones@redhat.com>
[Commit message extended to discuss impact]
Signed-off-by: Markus Armbruster <armbru@redhat.com>
---
qobject/json-parser.c | 10 ++++++----
tests/check-qjson.c | 5 +++++
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/qobject/json-parser.c b/qobject/json-parser.c
index 7a7ae9e8d1..d8eb210c0c 100644
--- a/qobject/json-parser.c
+++ b/qobject/json-parser.c
@@ -208,11 +208,13 @@ static QString *parse_string(JSONParserContext *ctxt, JSONToken *token)
}
break;
case '%':
- if (ctxt->ap && ptr[1] != '%') {
- parse_error(ctxt, token, "can't interpolate into string");
- goto out;
+ if (ctxt->ap) {
+ if (ptr[1] != '%') {
+ parse_error(ctxt, token, "can't interpolate into string");
+ goto out;
+ }
+ ptr++;
}
- ptr++;
/* fall through */
default:
cp = mod_utf8_codepoint(ptr, 6, &end);
diff --git a/tests/check-qjson.c b/tests/check-qjson.c
index d876a7a96e..fa2afccb0a 100644
--- a/tests/check-qjson.c
+++ b/tests/check-qjson.c
@@ -175,6 +175,11 @@ static void utf8_string(void)
"\xCE\xBA\xE1\xBD\xB9\xCF\x83\xCE\xBC\xCE\xB5",
"\xCE\xBA\xE1\xBD\xB9\xCF\x83\xCE\xBC\xCE\xB5",
"\\u03BA\\u1F79\\u03C3\\u03BC\\u03B5",
+ },
+ /* '%' character when not interpolating */
+ {
+ "100%",
+ "100%",
},
/* 2 Boundary condition test cases */
/* 2.1 First possible sequence of a certain length */
--
2.17.2
^ permalink raw reply related [flat|nested] 6+ messages in thread