26ba25
From 03f6fac8ac03d391fdbd7353ffd7c6eb1bd30bea Mon Sep 17 00:00:00 2001
26ba25
From: Markus Armbruster <armbru@redhat.com>
26ba25
Date: Fri, 31 Aug 2018 13:59:22 +0100
26ba25
Subject: [PATCH 2/3] i386: Fix arch_query_cpu_model_expansion() leak
26ba25
26ba25
RH-Author: Markus Armbruster <armbru@redhat.com>
26ba25
Message-id: <20180831135922.6073-3-armbru@redhat.com>
26ba25
Patchwork-id: 81980
26ba25
O-Subject: [qemu-kvm RHEL8/virt212 PATCH 2/2] i386: Fix arch_query_cpu_model_expansion() leak
26ba25
Bugzilla: 1615717
26ba25
RH-Acked-by: Eduardo Habkost <ehabkost@redhat.com>
26ba25
RH-Acked-by: Laszlo Ersek <lersek@redhat.com>
26ba25
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
26ba25
26ba25
From: Eduardo Habkost <ehabkost@redhat.com>
26ba25
26ba25
Reported by Coverity:
26ba25
26ba25
Error: RESOURCE_LEAK (CWE-772): [#def439]
26ba25
qemu-2.12.0/target/i386/cpu.c:3179: alloc_fn: Storage is returned from allocation function "qdict_new".
26ba25
qemu-2.12.0/qobject/qdict.c:34:5: alloc_fn: Storage is returned from allocation function "g_malloc0".
26ba25
qemu-2.12.0/qobject/qdict.c:34:5: var_assign: Assigning: "qdict" = "g_malloc0(4120UL)".
26ba25
qemu-2.12.0/qobject/qdict.c:37:5: return_alloc: Returning allocated memory "qdict".
26ba25
qemu-2.12.0/target/i386/cpu.c:3179: var_assign: Assigning: "props" = storage returned from "qdict_new()".
26ba25
qemu-2.12.0/target/i386/cpu.c:3217: leaked_storage: Variable "props" going out of scope leaks the storage it points to.
26ba25
26ba25
This was introduced by commit b8097deb359b ("i386: Improve
26ba25
query-cpu-model-expansion full mode").
26ba25
26ba25
The leak is only theoretical: if ret->model->props is set to
26ba25
props, the qapi_free_CpuModelExpansionInfo() call will free props
26ba25
too in case of errors.  The only way for this to not happen is if
26ba25
we enter the default branch of the switch statement, which would
26ba25
never happen because all CpuModelExpansionType values are being
26ba25
handled.
26ba25
26ba25
It's still worth to change this to make the allocation logic
26ba25
easier to follow and make the Coverity error go away.  To make
26ba25
everything simpler, initialize ret->model and ret->model->props
26ba25
earlier in the function.
26ba25
26ba25
While at it, remove redundant check for !prop because prop is
26ba25
always initialized at the beginning of the function.
26ba25
26ba25
Fixes: b8097deb359bbbd92592b9670adfe9e245b2d0bd
26ba25
Signed-off-by: Eduardo Habkost <ehabkost@redhat.com>
26ba25
Message-Id: <20180816183509.8231-1-ehabkost@redhat.com>
26ba25
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
26ba25
(cherry picked from commit e38bf612477fca62b205ebd909b1372a7e45a8c0)
26ba25
Signed-off-by: Danilo C. L. de Paula <ddepaula@redhat.com>
26ba25
---
26ba25
 target/i386/cpu.c | 9 +++------
26ba25
 1 file changed, 3 insertions(+), 6 deletions(-)
26ba25
26ba25
diff --git a/target/i386/cpu.c b/target/i386/cpu.c
26ba25
index caab4e2..605d0fa 100644
26ba25
--- a/target/i386/cpu.c
26ba25
+++ b/target/i386/cpu.c
26ba25
@@ -3727,6 +3727,9 @@ arch_query_cpu_model_expansion(CpuModelExpansionType type,
26ba25
     }
26ba25
 
26ba25
     props = qdict_new();
26ba25
+    ret->model = g_new0(CpuModelInfo, 1);
26ba25
+    ret->model->props = QOBJECT(props);
26ba25
+    ret->model->has_props = true;
26ba25
 
26ba25
     switch (type) {
26ba25
     case CPU_MODEL_EXPANSION_TYPE_STATIC:
26ba25
@@ -3747,15 +3750,9 @@ arch_query_cpu_model_expansion(CpuModelExpansionType type,
26ba25
         goto out;
26ba25
     }
26ba25
 
26ba25
-    if (!props) {
26ba25
-        props = qdict_new();
26ba25
-    }
26ba25
     x86_cpu_to_dict(xc, props);
26ba25
 
26ba25
-    ret->model = g_new0(CpuModelInfo, 1);
26ba25
     ret->model->name = g_strdup(base_name);
26ba25
-    ret->model->props = QOBJECT(props);
26ba25
-    ret->model->has_props = true;
26ba25
 
26ba25
 out:
26ba25
     object_unref(OBJECT(xc));
26ba25
-- 
26ba25
1.8.3.1
26ba25