[PATCH for v4.6.0] cgroups: Don't leave stale pointers around after virCgroupFree

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

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

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list