[Qemu-devel] [PATCH 01/21] qga: Fix crash on non-dictionary QMP argument

Markus Armbruster posted 21 patches 8 years, 11 months ago
There is a newer version of this series
[Qemu-devel] [PATCH 01/21] qga: Fix crash on non-dictionary QMP argument
Posted by Markus Armbruster 8 years, 11 months ago
The value of key 'arguments' must be a JSON object.  qemu-ga neglects
to check, and crashes.  To reproduce, send

    { 'execute': 'guest-sync', 'arguments': [] }

to qemu-ga.

do_qmp_dispatch() uses qdict_get_qdict() to get the arguments.  When
not a JSON object, this gets a null pointer, which flows through the
generated marshalling function to qobject_input_visitor_new(), where
it fails the assertion.  qmp_dispatch_check_obj() needs to catch this
error.

QEMU isn't affected, because it runs qmp_check_input_obj() first,
which basically duplicates qmp_check_input_obj()'s checks, plus the
missing one.

Fix by copying the missing one from qmp_check_input_obj() to
qmp_dispatch_check_obj().

Signed-off-by: Markus Armbruster <armbru@redhat.com>
Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
---
 qapi/qmp-dispatch.c | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/qapi/qmp-dispatch.c b/qapi/qmp-dispatch.c
index 48bec20..621922f 100644
--- a/qapi/qmp-dispatch.c
+++ b/qapi/qmp-dispatch.c
@@ -47,7 +47,13 @@ static QDict *qmp_dispatch_check_obj(const QObject *request, Error **errp)
                 return NULL;
             }
             has_exec_key = true;
-        } else if (strcmp(arg_name, "arguments")) {
+        } else if (!strcmp(arg_name, "arguments")) {
+            if (qobject_type(arg_obj) != QTYPE_QDICT) {
+                error_setg(errp, QERR_QMP_BAD_INPUT_OBJECT_MEMBER,
+                           "arguments", "object");
+                return NULL;
+            }
+        } else {
             error_setg(errp, QERR_QMP_EXTRA_MEMBER, arg_name);
             return NULL;
         }
-- 
2.7.4


Re: [Qemu-devel] [PATCH 01/21] qga: Fix crash on non-dictionary QMP argument
Posted by Eric Blake 8 years, 11 months ago
On 02/23/2017 03:44 PM, Markus Armbruster wrote:
> The value of key 'arguments' must be a JSON object.  qemu-ga neglects
> to check, and crashes.  To reproduce, send
> 
>     { 'execute': 'guest-sync', 'arguments': [] }
> 
> to qemu-ga.
> 
> do_qmp_dispatch() uses qdict_get_qdict() to get the arguments.  When
> not a JSON object, this gets a null pointer, which flows through the
> generated marshalling function to qobject_input_visitor_new(), where
> it fails the assertion.  qmp_dispatch_check_obj() needs to catch this
> error.
> 
> QEMU isn't affected, because it runs qmp_check_input_obj() first,
> which basically duplicates qmp_check_input_obj()'s checks, plus the
> missing one.
> 
> Fix by copying the missing one from qmp_check_input_obj() to
> qmp_dispatch_check_obj().
> 
> Signed-off-by: Markus Armbruster <armbru@redhat.com>
> Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
> ---
>  qapi/qmp-dispatch.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)

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

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

Re: [Qemu-devel] [PATCH 01/21] qga: Fix crash on non-dictionary QMP argument
Posted by Eric Blake 8 years, 11 months ago
On 02/23/2017 04:46 PM, Eric Blake wrote:
> On 02/23/2017 03:44 PM, Markus Armbruster wrote:
>> The value of key 'arguments' must be a JSON object.  qemu-ga neglects
>> to check, and crashes.  To reproduce, send
>>
>>     { 'execute': 'guest-sync', 'arguments': [] }
>>
>> to qemu-ga.
>>
>> do_qmp_dispatch() uses qdict_get_qdict() to get the arguments.  When
>> not a JSON object, this gets a null pointer, which flows through the
>> generated marshalling function to qobject_input_visitor_new(), where
>> it fails the assertion.  qmp_dispatch_check_obj() needs to catch this
>> error.
>>
>> QEMU isn't affected, because it runs qmp_check_input_obj() first,
>> which basically duplicates qmp_check_input_obj()'s checks, plus the

This sentence is weird (func A can't duplicate func A's checks; you're
missing a func B, but I'm not sure which instance is wrong, nor what B
should be).

>> missing one.
>>
>> Fix by copying the missing one from qmp_check_input_obj() to
>> qmp_dispatch_check_obj().
>>
>> Signed-off-by: Markus Armbruster <armbru@redhat.com>
>> Cc: Michael Roth <mdroth@linux.vnet.ibm.com>
>> ---
>>  qapi/qmp-dispatch.c | 8 +++++++-
>>  1 file changed, 7 insertions(+), 1 deletion(-)
> 
> Reviewed-by: Eric Blake <eblake@redhat.com>
> 

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