|
|
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 |
|