mirror of
https://gitlab.com/qemu-project/qemu
synced 2024-09-06 18:05:37 +00:00
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 <lcapitulino@redhat.com> Reviewed-by: Laszlo Ersek <lersek@redhat.com> Reviewed-by: Michael Roth <mdroth@linux.vnet.ibm.com>
This commit is contained in:
parent
6453a3a694
commit
8f91ad8a1b
|
@ -128,12 +128,15 @@ def gen_visitor_input_vars_decl(args):
|
|||
|
||||
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 @@ def gen_visitor_input_block(args, obj, dealloc=False):
|
|||
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 @@ def gen_marshal_output(name, args, ret_type, middle_mode):
|
|||
}
|
||||
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);
|
||||
}
|
||||
''',
|
||||
|
|
Loading…
Reference in a new issue