]> git.ipfire.org Git - thirdparty/libvirt.git/commitdiff
qemu: Fix double free in qemuDomainSecretAESClear
authorJohn Ferlan <jferlan@redhat.com>
Tue, 5 Jun 2018 20:20:59 +0000 (16:20 -0400)
committerJohn Ferlan <jferlan@redhat.com>
Wed, 6 Jun 2018 12:20:27 +0000 (08:20 -0400)
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

index f135117a959126c60271ace27b22fcc8084d3c24..1fb1ef1deb802e407ab8aac675118222d20ac13c 100644 (file)
@@ -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: