When storage secret is parsed in virStorageEncryptionSecretParse(), virSecretLookupParseSecret() which allocates some memory. This is however never freed. ==21711== 134 bytes in 6 blocks are definitely lost in loss record 70 of 85 ==21711== at 0x4C29F80: malloc (vg_replace_malloc.c:296) ==21711== by 0xBCA0356: xmlStrndup (in /usr/lib64/libxml2.so.2.9.4) ==21711== by 0xA9F432E: virXMLPropString (virxml.c:479) ==21711== by 0xA9D25B0: virSecretLookupParseSecret (virsecret.c:70) ==21711== by 0xA9D616E: virStorageEncryptionSecretParse (virstorageencryption.c:172) ==21711== by 0xA9D66B2: virStorageEncryptionParseXML (virstorageencryption.c:281) ==21711== by 0xA9D68DF: virStorageEncryptionParseNode (virstorageencryption.c:338) ==21711== by 0xAA12575: virDomainDiskDefParseXML (domain_conf.c:7606) ==21711== by 0xAA2CAC6: virDomainDefParseXML (domain_conf.c:16658) ==21711== by 0xAA2FC75: virDomainDefParseNode (domain_conf.c:17472) ==21711== by 0xAA2FAE4: virDomainDefParse (domain_conf.c:17419) ==21711== by 0xAA2FB72: virDomainDefParseFile (domain_conf.c:17443) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/util/virstorageencryption.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/util/virstorageencryption.c b/src/util/virstorageencryption.c index 4dab8bb..116a235 100644 --- a/src/util/virstorageencryption.c +++ b/src/util/virstorageencryption.c @@ -62,6 +62,7 @@ virStorageEncryptionSecretFree(virStorageEncryptionSecretPtr secret) { if (!secret) return; + virSecretLookupDefClear(&secret->seclookupdef); VIR_FREE(secret); } -- 2.8.4

Just like every other qemuBuild*CommandLine() function, this uses a buffer to hold partial cmd line strings too. However, if there's an error, the control jumps to 'cleanup' label leaving the buffer behind and thus leaking it. ==2013== 1,006 bytes in 1 blocks are definitely lost in loss record 701 of 711 ==2013== at 0x4C29F80: malloc (vg_replace_malloc.c:296) ==2013== by 0x4C2C32F: realloc (vg_replace_malloc.c:692) ==2013== by 0xAD925A8: virReallocN (viralloc.c:245) ==2013== by 0xAD95EA8: virBufferGrow (virbuffer.c:130) ==2013== by 0xAD95F78: virBufferAdd (virbuffer.c:165) ==2013== by 0x5097F5: qemuBuildCpuModelArgStr (qemu_command.c:6339) ==2013== by 0x509CC3: qemuBuildCpuCommandLine (qemu_command.c:6437) ==2013== by 0x51142C: qemuBuildCommandLine (qemu_command.c:9174) ==2013== by 0x47CA3A: qemuProcessCreatePretendCmd (qemu_process.c:5546) ==2013== by 0x433698: testCompareXMLToArgvFiles (qemuxml2argvtest.c:332) ==2013== by 0x4339AC: testCompareXMLToArgvHelper (qemuxml2argvtest.c:413) ==2013== by 0x446E7A: virTestRun (testutils.c:179) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_command.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c index 667691b..c97d0f6 100644 --- a/src/qemu/qemu_command.c +++ b/src/qemu/qemu_command.c @@ -6604,6 +6604,7 @@ qemuBuildCpuCommandLine(virCommandPtr cmd, cleanup: VIR_FREE(cpu); + virBufferFreeAndReset(&buf); return ret; } -- 2.8.4

This one's a bit more complicated. In qemuProcessPrepareDomain() a master key for encrypting secret for ciphered disks is created. This object lives within qemuDomainObjPrivate object. It is freed in qemuProcessStop(), but if nobody calls it (for instance like our qemuxml2argvtest does), the key object leaks. ==17078== 32 bytes in 1 blocks are definitely lost in loss record 633 of 707 ==17078== at 0x4C2C070: calloc (vg_replace_malloc.c:623) ==17078== by 0xAD924DF: virAllocN (viralloc.c:191) ==17078== by 0x5050BA6: virCryptoGenerateRandom (qemuxml2argvmock.c:166) ==17078== by 0x453DC8: qemuDomainMasterKeyCreate (qemu_domain.c:678) ==17078== by 0x47A36B: qemuProcessPrepareDomain (qemu_process.c:4913) ==17078== by 0x47C728: qemuProcessCreatePretendCmd (qemu_process.c:5542) ==17078== by 0x433698: testCompareXMLToArgvFiles (qemuxml2argvtest.c:332) ==17078== by 0x4339AC: testCompareXMLToArgvHelper (qemuxml2argvtest.c:413) ==17078== by 0x446E7A: virTestRun (testutils.c:179) ==17078== by 0x445BD9: mymain (qemuxml2argvtest.c:2022) ==17078== by 0x44886F: virTestMain (testutils.c:969) ==17078== by 0x445D9B: main (qemuxml2argvtest.c:2036) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_domain.c | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index 930e0b7..04aeaf2 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -547,6 +547,18 @@ qemuDomainWriteMasterKeyFile(virQEMUDriverPtr driver, } +static void +qemuDomainMasterKeyFree(qemuDomainObjPrivatePtr priv) +{ + if (!priv->masterKey) + return; + + if (priv->masterKeyLen > 0) + memset(priv->masterKey, 0, priv->masterKeyLen); + VIR_FREE(priv->masterKey); + priv->masterKeyLen = 0; +} + /* qemuDomainMasterKeyReadFile: * @priv: pointer to domain private object * @@ -619,9 +631,7 @@ qemuDomainMasterKeyReadFile(qemuDomainObjPrivatePtr priv) return 0; error: - if (masterKeyLen > 0) - memset(masterKey, 0, masterKeyLen); - VIR_FREE(masterKey); + qemuDomainMasterKeyFree(priv); VIR_FORCE_CLOSE(fd); VIR_FREE(path); @@ -645,9 +655,7 @@ qemuDomainMasterKeyRemove(qemuDomainObjPrivatePtr priv) return; /* Clear the contents */ - memset(priv->masterKey, 0, priv->masterKeyLen); - VIR_FREE(priv->masterKey); - priv->masterKeyLen = 0; + qemuDomainMasterKeyFree(priv); /* Delete the master key file */ path = qemuDomainGetMasterKeyFilePath(priv->libDir); @@ -1278,6 +1286,7 @@ qemuDomainObjPrivateFree(void *data) VIR_FREE(priv->libDir); VIR_FREE(priv->channelTargetDir); + qemuDomainMasterKeyFree(priv); VIR_FREE(priv); } -- 2.8.4

It's just test, but why leak it? ==26971== 20 bytes in 1 blocks are definitely lost in loss record 623 of 704 ==26971== at 0x4C29F80: malloc (vg_replace_malloc.c:296) ==26971== by 0xE560447: vasprintf (vasprintf.c:76) ==26971== by 0xAE0DEE2: virVasprintfInternal (virstring.c:480) ==26971== by 0xAE0DFF7: virAsprintfInternal (virstring.c:501) ==26971== by 0x4751F3: qemuProcessPrepareMonitorChr (qemu_process.c:2651) ==26971== by 0x4334B1: testCompareXMLToArgvFiles (qemuxml2argvtest.c:297) ==26971== by 0x4339AC: testCompareXMLToArgvHelper (qemuxml2argvtest.c:413) ==26971== by 0x446E7A: virTestRun (testutils.c:179) ==26971== by 0x445D33: mymain (qemuxml2argvtest.c:2029) ==26971== by 0x44886F: virTestMain (testutils.c:969) ==26971== by 0x445D9B: main (qemuxml2argvtest.c:2036) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuxml2argvtest.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 45525a2..2bfd1ca 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -265,6 +265,8 @@ static int testCompareXMLToArgvFiles(const char *xml, size_t i; qemuDomainObjPrivatePtr priv = NULL; + memset(&monitor_chr, 0, sizeof(monitor_chr)); + if (!(conn = virGetConnect())) goto out; conn->secretDriver = &fakeSecretDriver; @@ -292,8 +294,6 @@ static int testCompareXMLToArgvFiles(const char *xml, vm->def->id = -1; - - memset(&monitor_chr, 0, sizeof(monitor_chr)); if (qemuProcessPrepareMonitorChr(&monitor_chr, priv->libDir) < 0) goto out; @@ -363,6 +363,7 @@ static int testCompareXMLToArgvFiles(const char *xml, out: VIR_FREE(log); VIR_FREE(actualargv); + virDomainChrSourceDefClear(&monitor_chr); virCommandFree(cmd); virObjectUnref(vm); virObjectUnref(conn); -- 2.8.4

In the mock, we have a stub for virNetDevTapCreate(). However, the mocked version does not exactly as it's native counterpart. The function receives a string, which is an interface name that caller would like to have, but it's not guaranteed that they will get just that one. If they don't, the function free()-s the one passed and returns the new one. Just like the mocked version. But what is the mocked version missing is the free(). ==1068== 6 bytes in 1 blocks are definitely lost in loss record 9 of 132 ==1068== at 0x4C29F80: malloc (vg_replace_malloc.c:296) ==1068== by 0xDE13356: xmlStrndup (in /usr/lib64/libxml2.so.2.9.4) ==1068== by 0xAE2333E: virXMLPropString (virxml.c:479) ==1068== by 0xAE45975: virDomainNetDefParseXML (domain_conf.c:9038) ==1068== by 0xAE5C0BB: virDomainDefParseXML (domain_conf.c:16734) ==1068== by 0xAE5EB96: virDomainDefParseNode (domain_conf.c:17444) ==1068== by 0xAE5EA05: virDomainDefParse (domain_conf.c:17391) ==1068== by 0xAE5EA93: virDomainDefParseFile (domain_conf.c:17415) ==1068== by 0x433430: testCompareXMLToArgvFiles (qemuxml2argvtest.c:278) ==1068== by 0x433A18: testCompareXMLToArgvHelper (qemuxml2argvtest.c:414) ==1068== by 0x446ED4: virTestRun (testutils.c:179) ==1068== by 0x43A099: mymain (qemuxml2argvtest.c:1016) Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- tests/qemuxml2argvmock.c | 1 + 1 file changed, 1 insertion(+) diff --git a/tests/qemuxml2argvmock.c b/tests/qemuxml2argvmock.c index e0ec2db..78a224b 100644 --- a/tests/qemuxml2argvmock.c +++ b/tests/qemuxml2argvmock.c @@ -118,6 +118,7 @@ virNetDevTapCreate(char **ifname, for (i = 0; i < tapfdSize; i++) tapfd[i] = STDERR_FILENO + 1 + i; + VIR_FREE(*ifname); return VIR_STRDUP(*ifname, "vnet0"); } -- 2.8.4