From 8f91ad8a1b4702966d91ea58cd90bbde1faea1b3 Mon Sep 17 00:00:00 2001 From: Luiz Capitulino Date: Thu, 11 Jul 2013 14:26:56 -0400 Subject: [PATCH 1/2] qapi: qapi-commands: fix possible leaks on visitor dealloc In qmp-marshal.c the dealloc visitor calls use the same errp pointer of the input visitor calls. This means that if any of the input visitor calls fails, then the dealloc visitor will return early, before freeing the object's memory. Here's an example, consider this code: int qmp_marshal_input_block_passwd(Monitor *mon, const QDict *qdict, QObject **ret) { [...] char * device = NULL; char * password = NULL; mi = qmp_input_visitor_new_strict(QOBJECT(args)); v = qmp_input_get_visitor(mi); visit_type_str(v, &device, "device", errp); visit_type_str(v, &password, "password", errp); qmp_input_visitor_cleanup(mi); if (error_is_set(errp)) { goto out; } qmp_block_passwd(device, password, errp); out: md = qapi_dealloc_visitor_new(); v = qapi_dealloc_get_visitor(md); visit_type_str(v, &device, "device", errp); visit_type_str(v, &password, "password", errp); qapi_dealloc_visitor_cleanup(md); [...] return 0; } Consider errp != NULL when the out label is reached, we're going to leak device and password. This patch fixes this by always passing errp=NULL for dealloc visitors, meaning that we always try to free them regardless of any previous failure. The above example would then be: out: md = qapi_dealloc_visitor_new(); v = qapi_dealloc_get_visitor(md); visit_type_str(v, &device, "device", NULL); visit_type_str(v, &password, "password", NULL); qapi_dealloc_visitor_cleanup(md); Signed-off-by: Luiz Capitulino Reviewed-by: Laszlo Ersek Reviewed-by: Michael Roth --- scripts/qapi-commands.py | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/scripts/qapi-commands.py b/scripts/qapi-commands.py index e06332bd55..b12b6964ef 100644 --- a/scripts/qapi-commands.py +++ b/scripts/qapi-commands.py @@ -128,12 +128,15 @@ bool has_%(argname)s = false; def gen_visitor_input_block(args, obj, dealloc=False): ret = "" + errparg = 'errp' + if len(args) == 0: return ret push_indent() if dealloc: + errparg = 'NULL' ret += mcgen(''' md = qapi_dealloc_visitor_new(); v = qapi_dealloc_get_visitor(md); @@ -148,22 +151,22 @@ v = qmp_input_get_visitor(mi); for argname, argtype, optional, structured in parse_args(args): if optional: ret += mcgen(''' -visit_start_optional(v, &has_%(c_name)s, "%(name)s", errp); +visit_start_optional(v, &has_%(c_name)s, "%(name)s", %(errp)s); if (has_%(c_name)s) { ''', - c_name=c_var(argname), name=argname) + c_name=c_var(argname), name=argname, errp=errparg) push_indent() ret += mcgen(''' -%(visitor)s(v, &%(c_name)s, "%(name)s", errp); +%(visitor)s(v, &%(c_name)s, "%(name)s", %(errp)s); ''', c_name=c_var(argname), name=argname, argtype=argtype, - visitor=type_visitor(argtype)) + visitor=type_visitor(argtype), errp=errparg) if optional: pop_indent() ret += mcgen(''' } -visit_end_optional(v, errp); -''') +visit_end_optional(v, %(errp)s); +''', errp=errparg) if dealloc: ret += mcgen(''' @@ -194,7 +197,7 @@ static void qmp_marshal_output_%(c_name)s(%(c_ret_type)s ret_in, QObject **ret_o } qmp_output_visitor_cleanup(mo); v = qapi_dealloc_get_visitor(md); - %(visitor)s(v, &ret_in, "unused", errp); + %(visitor)s(v, &ret_in, "unused", NULL); qapi_dealloc_visitor_cleanup(md); } ''', From f9b1d9b20f5d25b95f67a498e312f625d168fc51 Mon Sep 17 00:00:00 2001 From: Amos Kong Date: Tue, 16 Jul 2013 19:52:14 +0800 Subject: [PATCH 2/2] qmp: update send-key document commit 9f328977 changes qmp_send_key() to accept key codes in hex, but the document wasn't updated. The items of keys list is union now, not enum. Signed-off-by: Amos Kong Reviewed-by: Eric Blake Signed-off-by: Luiz Capitulino --- qmp-commands.hx | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/qmp-commands.hx b/qmp-commands.hx index e075df423a..b8e77581ae 100644 --- a/qmp-commands.hx +++ b/qmp-commands.hx @@ -346,7 +346,8 @@ Send keys to VM. Arguments: keys array: - - "key": key sequence (a json-array of key enum values) + - "key": key sequence (a json-array of key union values, + union can be number or qcode enum) - hold-time: time to delay key up events, milliseconds. Defaults to 100 (json-int, optional) @@ -354,7 +355,9 @@ keys array: Example: -> { "execute": "send-key", - "arguments": { 'keys': [ 'ctrl', 'alt', 'delete' ] } } + "arguments": { "keys": [ { "type": "qcode", "data": "ctrl" }, + { "type": "qcode", "data": "alt" }, + { "type": "qcode", "data": "delete" } ] } } <- { "return": {} } EQMP