On Sat, Jul 09, 2016 at 10:19:03 +0200, Michal Privoznik wrote:
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(a)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);
Turn this into VIR_DISPOSE_N
+ 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);
This won't do what you've expected since priv->masterKey was not set
yet.
You can use VIR_DISPOSE_N directly here.
+ qemuDomainMasterKeyFree(priv);
VIR_FORCE_CLOSE(fd);
VIR_FREE(path);
ACK with the tweaks above.