From 228ae70938d0cb85353e35f744fbc494de619481 Mon Sep 17 00:00:00 2001
From: John Ferlan <jferlan@redhat.com>
Date: Tue, 5 Jun 2018 16:20:59 -0400
Subject: [PATCH] qemu: Fix double free in qemuDomainSecretAESClear

Commit id 02b031a4 added a secondary path from which the
incoming @secinfo would not be free'd until the private
data was freed in qemuDomainStorageSourcePrivateDispose.

However, by doing this the original intention to free
@*secinfo afterwards is lost and thus the pass by value
of the secinfo->s.aes (or secinfo->s.plain for its method)
results in not keeping the NULL setting in the various
secret.{username|iv|ciphertext} fields upon return to
qemuDomainSecretInfoClear and eventually will result in
a double free at domain destroy:

    raise ()
    abort ()
    __libc_message ()
    malloc_printerr ()
    _int_free ()
    virFree
    qemuDomainSecretAESClear
    qemuDomainSecretInfoClear
    qemuDomainSecretInfoFree
    qemuDomainStorageSourcePrivateDispose
    virObjectUnref
    virStorageSourceClear
    virStorageSourceFree
    virDomainDiskDefFree
    virDomainDefFree
    virDomainObjRemoveTransientDef
    qemuProcessStop
    qemuDomainDestroyFlags
    virDomainDestroy

Signed-off-by: John Ferlan <jferlan@redhat.com>
ACKed-by: Peter Krempa <pkrempa@redhat.com>
---
 src/qemu/qemu_domain.c | 20 ++++++++++----------
 1 file changed, 10 insertions(+), 10 deletions(-)

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index f135117a95..1fb1ef1deb 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -945,23 +945,23 @@ qemuDomainMasterKeyCreate(virDomainObjPtr vm)
 
 
 static void
-qemuDomainSecretPlainClear(qemuDomainSecretPlain secret)
+qemuDomainSecretPlainClear(qemuDomainSecretPlainPtr secret)
 {
-    VIR_FREE(secret.username);
-    VIR_DISPOSE_N(secret.secret, secret.secretlen);
+    VIR_FREE(secret->username);
+    VIR_DISPOSE_N(secret->secret, secret->secretlen);
 }
 
 
 static void
-qemuDomainSecretAESClear(qemuDomainSecretAES secret,
+qemuDomainSecretAESClear(qemuDomainSecretAESPtr secret,
                          bool keepAlias)
 {
     if (!keepAlias)
-        VIR_FREE(secret.alias);
+        VIR_FREE(secret->alias);
 
-    VIR_FREE(secret.username);
-    VIR_FREE(secret.iv);
-    VIR_FREE(secret.ciphertext);
+    VIR_FREE(secret->username);
+    VIR_FREE(secret->iv);
+    VIR_FREE(secret->ciphertext);
 }
 
 
@@ -974,11 +974,11 @@ qemuDomainSecretInfoClear(qemuDomainSecretInfoPtr secinfo,
 
     switch ((qemuDomainSecretInfoType) secinfo->type) {
     case VIR_DOMAIN_SECRET_INFO_TYPE_PLAIN:
-        qemuDomainSecretPlainClear(secinfo->s.plain);
+        qemuDomainSecretPlainClear(&secinfo->s.plain);
         break;
 
     case VIR_DOMAIN_SECRET_INFO_TYPE_AES:
-        qemuDomainSecretAESClear(secinfo->s.aes, keepAlias);
+        qemuDomainSecretAESClear(&secinfo->s.aes, keepAlias);
         break;
 
     case VIR_DOMAIN_SECRET_INFO_TYPE_LAST:
-- 
GitLab