10:08 a.m.
These memory leaks were found by llvm's address sanitizer asan.
Tim Wiederhake (3):
xenParseHypervisorFeatures: Fix memory leak
virDomainFeaturesDefParse: Fix memory leak
virQEMUCapsSetHostModel: Fix memory leak
src/conf/domain_conf.c | 2 +-
src/libxl/xen_common.c | 1 +
src/qemu/qemu_capabilities.c | 8 ++++++++
3 files changed, 10 insertions(+), 1 deletion(-)
--
2.26.2
10:08 a.m.
New subject: [libvirt PATCH 1/3] xenParseHypervisorFeatures: Fix memory leak
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/libxl/xen_common.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c
index 12a44280cb..ad5a3de116 100644
--- a/src/libxl/xen_common.c
+++ b/src/libxl/xen_common.c
@@ -567,6 +567,7 @@ xenParseHypervisorFeatures(virConf *conf, virDomainDef *def)
timer->mode = VIR_DOMAIN_TIMER_MODE_PARAVIRT;
def->clock.timers[def->clock.ntimers - 1] = timer;
+ VIR_FREE(strval);
}
if (xenConfigGetString(conf, "passthrough", &strval, NULL) < 0)
--
2.26.2
3:25 a.m.
New subject: [libvirt PATCH 1/3] xenParseHypervisorFeatures: Fix memory leak
On Thu, Apr 15, 2021 at 17:08:26 +0200, Tim Wiederhake wrote:
Fixes: b523e22521a
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/libxl/xen_common.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c
index 12a44280cb..ad5a3de116 100644
--- a/src/libxl/xen_common.c
+++ b/src/libxl/xen_common.c
@@ -567,6 +567,7 @@ xenParseHypervisorFeatures(virConf *conf, virDomainDef *def)
timer->mode = VIR_DOMAIN_TIMER_MODE_PARAVIRT;
def->clock.timers[def->clock.ntimers - 1] = timer;
+ VIR_FREE(strval);
While this fixes the specific problem, it still leaves possibility for
making a mistake again since 'strval' isn't explicitly freed after the
second use.
A preferrable fix is to use one variable per use when used with
g_autofree or remove g_autofree and explicitly free it after both uses.
CCing Jim to state his preferred solution.
}
if (xenConfigGetString(conf, "passthrough", &strval, NULL) < 0)
--
2.26.2
5:31 p.m.
New subject: [libvirt PATCH 1/3] xenParseHypervisorFeatures: Fix memory leak
On 4/16/21 2:25 AM, Peter Krempa wrote:
On Thu, Apr 15, 2021 at 17:08:26 +0200, Tim Wiederhake wrote:
Fixes: b523e22521a
> Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
> ---
> src/libxl/xen_common.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/src/libxl/xen_common.c b/src/libxl/xen_common.c
> index 12a44280cb..ad5a3de116 100644
> --- a/src/libxl/xen_common.c
> +++ b/src/libxl/xen_common.c
> @@ -567,6 +567,7 @@ xenParseHypervisorFeatures(virConf *conf, virDomainDef *def)
> timer->mode = VIR_DOMAIN_TIMER_MODE_PARAVIRT;
>
> def->clock.timers[def->clock.ntimers - 1] = timer;
> + VIR_FREE(strval);
While this fixes the specific problem, it still leaves possibility for
making a mistake again since 'strval' isn't explicitly freed after the
second use.
A preferrable fix is to use one variable per use when used with
g_autofree or remove g_autofree and explicitly free it after both uses.
Since there are only two uses, I prefer this approach as well. E.g.
g_autofree char *tscmode = NULL;
g_autofree char *passthrough = NULL;
Regards,
Jim
10:08 a.m.
New subject: [libvirt PATCH 2/3] virDomainFeaturesDefParse: Fix memory leak
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/domain_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 029f2d8d9c..5591681283 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -18994,12 +18994,12 @@ virDomainFeaturesDefParse(virDomainDef *def,
int feature;
int value;
g_autofree char *ptval = NULL;
- g_autofree char *tmp = NULL;
if ((n = virXPathNodeSet("./features/xen/*", ctxt, &nodes)) <
0)
return -1;
for (i = 0; i < n; i++) {
+ g_autofree char *tmp = NULL;
feature = virDomainXenTypeFromString((const char *)nodes[i]->name);
if (feature < 0) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
--
2.26.2
3:16 a.m.
New subject: [libvirt PATCH 2/3] virDomainFeaturesDefParse: Fix memory leak
On Thu, Apr 15, 2021 at 17:08:27 +0200, Tim Wiederhake wrote:
Fixes: 94013ee04e3
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/conf/domain_conf.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Reviewed-by: Peter Krempa <pkrempa(a)redhat.com>
10:08 a.m.
New subject: [libvirt PATCH 3/3] virQEMUCapsSetHostModel: Fix memory leak
virQEMUCapsSetHostModel is called by virQEMUCapsInitHostCPUModel,
which in turn is typically called twice (for KVM and QEMU), e.g. in
virQEMUCapsLoadCache and virQEMUCapsNewForBinaryInternal.
The second call leaks memory pointed to by "reported", "migratable"
and "full".
Signed-off-by: Tim Wiederhake <twiederh(a)redhat.com>
---
src/qemu/qemu_capabilities.c | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c
index f1a3c526ef..ff8877631c 100644
--- a/src/qemu/qemu_capabilities.c
+++ b/src/qemu/qemu_capabilities.c
@@ -2276,6 +2276,14 @@ virQEMUCapsSetHostModel(virQEMUCaps *qemuCaps,
virQEMUCapsHostCPUData *cpuData;
cpuData = &virQEMUCapsGetAccel(qemuCaps, type)->hostCPU;
+
+ if (cpuData->reported)
+ virCPUDefFree(cpuData->reported);
+ if (cpuData->migratable)
+ virCPUDefFree(cpuData->migratable);
+ if (cpuData->full)
+ virCPUDefFree(cpuData->full);
+
cpuData->reported = reported;
cpuData->migratable = migratable;
cpuData->full = full;
--
2.26.2
3:31 a.m.
New subject: [libvirt PATCH 3/3] virQEMUCapsSetHostModel: Fix memory leak
On Thu, Apr 15, 2021 at 17:08:28 +0200, Tim Wiederhake wrote:
virQEMUCapsSetHostModel is called by virQEMUCapsInitHostCPUModel,
which in turn is typically called twice (for KVM and QEMU), e.g. in
virQEMUCapsLoadCache and virQEMUCapsNewForBinaryInternal.
The second call leaks memory pointed to by "reported", "migratable"
and "full".
That sounds to me like a usage problem in the caller, where if it's
called twice it's either using a non-cleared variable, or the value will
be overwritten and thus calling it in the first place is probably
dubious, so the proposed patch feels to me like it's fixing a symptom
rather than the real problem.
1316
days inactive
1317
days old
7 comments
3 participants
participants (3)
-
Jim Fehlig -
Peter Krempa -
Tim Wiederhake