3:25 a.m.
One of the attributes that original virCgroupFree() had was it
set passed pointer to NULL. For instance in the following code
the latter call would be practically a no-op:
virCgroupFree(&var);
virCgroupFree(&var);
However, this behaviour of the function was changed in
0f80c71822d824 but corresponding 'var = NULL' lines were not
added leading to double free:
Invalid read of size 8
at 0x52CA3DA: virFree (viralloc.c:582)
by 0x52D5272: virCgroupFree (vircgroup.c:1700)
by 0x230CE113: qemuDomainObjPrivateDataClear (qemu_domain.c:1923)
by 0x230CE2F3: qemuDomainObjPrivateFree (qemu_domain.c:1973)
by 0x53922D7: virDomainObjDispose (domain_conf.c:3192)
by 0x533B8ED: virObjectUnref (virobject.c:350)
by 0x533BE39: virObjectFreeHashData (virobject.c:591)
by 0x5305C23: virHashFree (virhash.c:304)
by 0x53EAFA7: virDomainObjListDispose (virdomainobjlist.c:92)
by 0x533B8ED: virObjectUnref (virobject.c:350)
by 0x2315E2AE: qemuStateCleanup (qemu_driver.c:1067)
by 0x557CFF6: virStateCleanup (libvirt.c:699)
Address 0x29fbbdd0 is 16 bytes inside a block of size 328 free'd
at 0x4C2E13B: free (vg_replace_malloc.c:530)
by 0x52CA3E4: virFree (viralloc.c:582)
by 0x52D52D4: virCgroupFree (vircgroup.c:1706)
by 0x230CE113: qemuDomainObjPrivateDataClear (qemu_domain.c:1923)
by 0x2311CFE9: qemuProcessStop (qemu_process.c:7144)
by 0x2311BF1C: qemuProcessStart (qemu_process.c:6745)
by 0x2316E634: qemuDomainObjStart (qemu_driver.c:7293)
by 0x2316E8A2: qemuDomainCreateWithFlags (qemu_driver.c:7346)
by 0x2316E925: qemuDomainCreate (qemu_driver.c:7365)
by 0x5594E9F: virDomainCreate (libvirt-domain.c:6534)
by 0x1367D1: remoteDispatchDomainCreate (remote_daemon_dispatch_stubs.h:4434)
by 0x1366EA: remoteDispatchDomainCreateHelper (remote_daemon_dispatch_stubs.h:4410)
Signed-off-by: Michal Privoznik <mprivozn(a)redhat.com>
---
src/lxc/lxc_process.c | 1 +
src/qemu/qemu_cgroup.c | 2 ++
src/qemu/qemu_domain.c | 1 +
src/util/vircgroup.c | 5 +++++
4 files changed, 9 insertions(+)
diff --git a/src/lxc/lxc_process.c b/src/lxc/lxc_process.c
index 5d8fa63c67..4d118cb6fd 100644
--- a/src/lxc/lxc_process.c
+++ b/src/lxc/lxc_process.c
@@ -221,6 +221,7 @@ static void virLXCProcessCleanup(virLXCDriverPtr driver,
if (priv->cgroup) {
virCgroupRemove(priv->cgroup);
virCgroupFree(priv->cgroup);
+ priv->cgroup = NULL;
}
/* Get machined to terminate the machine as it may not have cleaned it
diff --git a/src/qemu/qemu_cgroup.c b/src/qemu/qemu_cgroup.c
index 8a00ffcd45..cd1e01262b 100644
--- a/src/qemu/qemu_cgroup.c
+++ b/src/qemu/qemu_cgroup.c
@@ -921,6 +921,7 @@ qemuInitCgroup(virDomainObjPtr vm,
goto done;
virCgroupFree(priv->cgroup);
+ priv->cgroup = NULL;
if (!vm->def->resource) {
virDomainResourceDefPtr res;
@@ -1058,6 +1059,7 @@ qemuConnectCgroup(virDomainObjPtr vm)
goto done;
virCgroupFree(priv->cgroup);
+ priv->cgroup = NULL;
if (virCgroupNewDetectMachine(vm->def->name,
"qemu",
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index bda53814a3..bfffd3da27 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -1921,6 +1921,7 @@ qemuDomainObjPrivateDataClear(qemuDomainObjPrivatePtr priv)
priv->qemuDevices = NULL;
virCgroupFree(priv->cgroup);
+ priv->cgroup = NULL;
virPerfFree(priv->perf);
priv->perf = NULL;
diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 472a8167f5..6e2e06bae3 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -1371,6 +1371,7 @@ virCgroupNewDomainPartition(virCgroupPtr partition,
VIR_CGROUP_MEM_HIERACHY) < 0) {
virCgroupRemove(*group);
virCgroupFree(*group);
+ *group = NULL;
return -1;
}
@@ -1428,6 +1429,7 @@ virCgroupNewThread(virCgroupPtr domain,
if (virCgroupMakeGroup(domain, *group, create, VIR_CGROUP_NONE) < 0) {
virCgroupRemove(*group);
virCgroupFree(*group);
+ *group = NULL;
return -1;
}
@@ -1466,6 +1468,7 @@ virCgroupNewDetectMachine(const char *name,
VIR_DEBUG("Failed to validate machine name for '%s' driver
'%s'",
name, drivername);
virCgroupFree(*group);
+ *group = NULL;
return 0;
}
@@ -1566,6 +1569,7 @@ virCgroupNewMachineSystemd(const char *name,
virErrorPtr saved = virSaveLastError();
virCgroupRemove(*group);
virCgroupFree(*group);
+ *group = NULL;
if (saved) {
virSetError(saved);
virFreeError(saved);
@@ -1617,6 +1621,7 @@ virCgroupNewMachineManual(const char *name,
virErrorPtr saved = virSaveLastError();
virCgroupRemove(*group);
virCgroupFree(*group);
+ *group = NULL;
if (saved) {
virSetError(saved);
virFreeError(saved);
--
2.16.4