[libvirt] [PATCH 3/5] qemuDomainObjPrivateFree: Free @masterKey too

[Peter Krempa]

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 at 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.