From a95291007b2478fcf32a2d71bf133b688bb4b675 Mon Sep 17 00:00:00 2001 From: Markus Armbruster Date: Tue, 18 Dec 2018 19:22:21 +0100 Subject: qapi: Eliminate indirection through qmp_event_get_func_emit() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Message-Id: <20181218182234.28876-3-armbru@redhat.com> Reviewed-by: Marc-André Lureau [Commit message typos fixed] --- tests/Makefile.include | 4 ++-- tests/test-qmp-event.c | 6 +----- 2 files changed, 3 insertions(+), 7 deletions(-) (limited to 'tests') 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); -- cgit v1.2.3 From bbc0586ced6e9ffdfd29d89fcc917b3d90ac3938 Mon Sep 17 00:00:00 2001 From: Christophe Fergeau Date: Wed, 2 Jan 2019 15:05:35 +0100 Subject: json: Fix % handling when not interpolating 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 Message-Id: <20190102140535.11512-1-cfergeau@redhat.com> Reviewed-by: Eric Blake Tested-by: Richard W.M. Jones [Commit message extended to discuss impact] Signed-off-by: Markus Armbruster --- tests/check-qjson.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'tests') 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 */ -- cgit v1.2.3