[PATCH] vircgroup: Don't leak @parent in virCgroupEnableMissingControllers()

Michal Privoznik posted 1 patch 3 years, 1 month ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/1d6d449cff3e97d7c511388fc41db1a7bd374d13.1612382812.git.mprivozn@redhat.com
src/util/vircgroup.c | 1 +
1 file changed, 1 insertion(+)
[PATCH] vircgroup: Don't leak @parent in virCgroupEnableMissingControllers()
Posted by Michal Privoznik 3 years, 1 month ago
A memory leak was identified in
virCgroupEnableMissingControllers():

==11680==    at 0x483EAE5: calloc (vg_replace_malloc.c:760)
==11680==    by 0x4E51780: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.6701.0)
==11680==    by 0x4908618: virCgroupNew (vircgroup.c:701)
==11680==    by 0x49096F4: virCgroupEnableMissingControllers (vircgroup.c:1146)
==11680==    by 0x4909B17: virCgroupNewMachineSystemd (vircgroup.c:1228)
==11680==    by 0x4909E94: virCgroupNewMachine (vircgroup.c:1313)
==11680==    by 0x1694FDBC: qemuInitCgroup (qemu_cgroup.c:946)
==11680==    by 0x1695046B: qemuSetupCgroup (qemu_cgroup.c:1083)
==11680==    by 0x16A60126: qemuProcessLaunch (qemu_process.c:7077)
==11680==    by 0x16A61504: qemuProcessStart (qemu_process.c:7384)
==11680==    by 0x169B84C2: qemuDomainObjStart (qemu_driver.c:6590)
==11680==    by 0x169B8776: qemuDomainCreateWithFlags (qemu_driver.c:6641)

What happens is that new virCgroup is created and stored into
@parent. Then, if @tokens is not empty the for() loop is entered
into where another virCgroup is created and @parent is replaced
with this new virCgroup. But nothing freed the old @parent.

Fixes: 77291414c7a8745cf4d2b06d3c38d269cfbcfe32
Reported-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/util/vircgroup.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c
index 15071d8b1b..9fb1b45a8f 100644
--- a/src/util/vircgroup.c
+++ b/src/util/vircgroup.c
@@ -1159,6 +1159,7 @@ virCgroupEnableMissingControllers(char *path,
         if (virCgroupMakeGroup(parent, tmp, true, VIR_CGROUP_SYSTEMD) < 0)
             return -1;
 
+        virCgroupFree(parent);
         parent = g_steal_pointer(&tmp);
     }
 
-- 
2.26.2

Re: [PATCH] vircgroup: Don't leak @parent in virCgroupEnableMissingControllers()
Posted by Pavel Hrdina 3 years, 1 month ago
On Wed, Feb 03, 2021 at 09:06:52PM +0100, Michal Privoznik wrote:
> A memory leak was identified in
> virCgroupEnableMissingControllers():
> 
> ==11680==    at 0x483EAE5: calloc (vg_replace_malloc.c:760)
> ==11680==    by 0x4E51780: g_malloc0 (in /usr/lib64/libglib-2.0.so.0.6701.0)
> ==11680==    by 0x4908618: virCgroupNew (vircgroup.c:701)
> ==11680==    by 0x49096F4: virCgroupEnableMissingControllers (vircgroup.c:1146)
> ==11680==    by 0x4909B17: virCgroupNewMachineSystemd (vircgroup.c:1228)
> ==11680==    by 0x4909E94: virCgroupNewMachine (vircgroup.c:1313)
> ==11680==    by 0x1694FDBC: qemuInitCgroup (qemu_cgroup.c:946)
> ==11680==    by 0x1695046B: qemuSetupCgroup (qemu_cgroup.c:1083)
> ==11680==    by 0x16A60126: qemuProcessLaunch (qemu_process.c:7077)
> ==11680==    by 0x16A61504: qemuProcessStart (qemu_process.c:7384)
> ==11680==    by 0x169B84C2: qemuDomainObjStart (qemu_driver.c:6590)
> ==11680==    by 0x169B8776: qemuDomainCreateWithFlags (qemu_driver.c:6641)
> 
> What happens is that new virCgroup is created and stored into
> @parent. Then, if @tokens is not empty the for() loop is entered
> into where another virCgroup is created and @parent is replaced
> with this new virCgroup. But nothing freed the old @parent.
> 
> Fixes: 77291414c7a8745cf4d2b06d3c38d269cfbcfe32
> Reported-by: Andrea Bolognani <abologna@redhat.com>
> Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
> ---
>  src/util/vircgroup.c | 1 +
>  1 file changed, 1 insertion(+)

Oops, thanks for fixing this.

Reviewed-by: Pavel Hrdina <phrdina@redhat.com>