Reported by Coverity:

Error: RESOURCE_LEAK (CWE-772): [#def439]
qemu-2.12.0/target/i386/cpu.c:3179: alloc_fn: Storage is returned from allocation function "qdict_new".
qemu-2.12.0/qobject/qdict.c:34:5: alloc_fn: Storage is returned from allocation function "g_malloc0".
qemu-2.12.0/qobject/qdict.c:34:5: var_assign: Assigning: "qdict" = "g_malloc0(4120UL)".
qemu-2.12.0/qobject/qdict.c:37:5: return_alloc: Returning allocated memory "qdict".
qemu-2.12.0/target/i386/cpu.c:3179: var_assign: Assigning: "props" = storage returned from "qdict_new()".
qemu-2.12.0/target/i386/cpu.c:3217: leaked_storage: Variable "props" going out of scope leaks the storage it points to.

This was introduced by commit b8097deb359b ("i386: Improve
query-cpu-model-expansion full mode").

The leak is only theoretical: if ret->model->props is set to
props, the qapi_free_CpuModelExpansionInfo() call will free props
too in case of errors.  The only way for this to not happen is if
we enter the default branch of the switch statement, which would
never happen because all CpuModelExpansionType values are being
handled.

It's still worth to change this to make the allocation logic
easier to follow and make the Coverity error go away.  To make
everything simpler, initialize ret->model and ret->model->props
earlier in the function.

While at it, remove redundant check for !prop because prop is
always initialized at the beginning of the function.

Fixes: b8097deb359bbbd92592b9670adfe9e245b2d0bd
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
---
 target/i386/cpu.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

diff --git a/target/i386/cpu.c b/target/i386/cpu.c
index 723e02221e..e23c05c4e1 100644
--- a/target/i386/cpu.c
+++ b/target/i386/cpu.c
@@ -3758,6 +3758,9 @@ arch_query_cpu_model_expansion(CpuModelExpansionType type,
     }
 
     props = qdict_new();
+    ret->model = g_new0(CpuModelInfo, 1);
+    ret->model->props = QOBJECT(props);
+    ret->model->has_props = true;
 
     switch (type) {
     case CPU_MODEL_EXPANSION_TYPE_STATIC:
@@ -3778,15 +3781,9 @@ arch_query_cpu_model_expansion(CpuModelExpansionType type,
         goto out;
     }
 
-    if (!props) {
-        props = qdict_new();
-    }
     x86_cpu_to_dict(xc, props);
 
-    ret->model = g_new0(CpuModelInfo, 1);
     ret->model->name = g_strdup(base_name);
-    ret->model->props = QOBJECT(props);
-    ret->model->has_props = true;
 
 out:
     object_unref(OBJECT(xc));
-- 
2.18.0.rc1.1.g3f1ff2140

On 16/08/2018 20:35, Eduardo Habkost wrote:
> Reported by Coverity:
> 
> Error: RESOURCE_LEAK (CWE-772): [#def439]
> qemu-2.12.0/target/i386/cpu.c:3179: alloc_fn: Storage is returned from allocation function "qdict_new".
> qemu-2.12.0/qobject/qdict.c:34:5: alloc_fn: Storage is returned from allocation function "g_malloc0".
> qemu-2.12.0/qobject/qdict.c:34:5: var_assign: Assigning: "qdict" = "g_malloc0(4120UL)".
> qemu-2.12.0/qobject/qdict.c:37:5: return_alloc: Returning allocated memory "qdict".
> qemu-2.12.0/target/i386/cpu.c:3179: var_assign: Assigning: "props" = storage returned from "qdict_new()".
> qemu-2.12.0/target/i386/cpu.c:3217: leaked_storage: Variable "props" going out of scope leaks the storage it points to.
> 
> This was introduced by commit b8097deb359b ("i386: Improve
> query-cpu-model-expansion full mode").
> 
> The leak is only theoretical: if ret->model->props is set to
> props, the qapi_free_CpuModelExpansionInfo() call will free props
> too in case of errors.  The only way for this to not happen is if
> we enter the default branch of the switch statement, which would
> never happen because all CpuModelExpansionType values are being
> handled.
> 
> It's still worth to change this to make the allocation logic
> easier to follow and make the Coverity error go away.  To make
> everything simpler, initialize ret->model and ret->model->props
> earlier in the function.
> 
> While at it, remove redundant check for !prop because prop is
> always initialized at the beginning of the function.
> 
> Fixes: b8097deb359bbbd92592b9670adfe9e245b2d0bd
> Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
> ---
>  target/i386/cpu.c | 9 +++------
>  1 file changed, 3 insertions(+), 6 deletions(-)
> 
> diff --git a/target/i386/cpu.c b/target/i386/cpu.c
> index 723e02221e..e23c05c4e1 100644
> --- a/target/i386/cpu.c
> +++ b/target/i386/cpu.c
> @@ -3758,6 +3758,9 @@ arch_query_cpu_model_expansion(CpuModelExpansionType type,
>      }
>  
>      props = qdict_new();
> +    ret->model = g_new0(CpuModelInfo, 1);
> +    ret->model->props = QOBJECT(props);
> +    ret->model->has_props = true;
>  
>      switch (type) {
>      case CPU_MODEL_EXPANSION_TYPE_STATIC:
> @@ -3778,15 +3781,9 @@ arch_query_cpu_model_expansion(CpuModelExpansionType type,
>          goto out;
>      }
>  
> -    if (!props) {
> -        props = qdict_new();
> -    }
>      x86_cpu_to_dict(xc, props);
>  
> -    ret->model = g_new0(CpuModelInfo, 1);
>      ret->model->name = g_strdup(base_name);
> -    ret->model->props = QOBJECT(props);
> -    ret->model->has_props = true;
>  
>  out:
>      object_unref(OBJECT(xc));
> 

Queued, thanks.

Paolo