[libvirt] [PATCH] qemu: Fix crash hot plugging luks volume

[John Ferlan]

https://bugzilla.redhat.com/show_bug.cgi?id=1367259

Crash occurs because 'secrets' is being dereferenced in call:

        if (qemuDomainSecretSetup(conn, priv, secinfo, disk->info.alias,
                                  VIR_SECRET_USAGE_TYPE_VOLUME, NULL,
                                  &src->encryption->secrets[0]->seclookupdef,
                                  true) < 0)

(gdb) p *src->encryption
$1 = {format = 2, nsecrets = 0, secrets = 0x0, encinfo = {cipher_size = 0,
    cipher_name = 0x0, cipher_mode = 0x0, cipher_hash = 0x0, ivgen_name = 0x0,
    ivgen_hash = 0x0}}
(gdb) bt
    priv=priv at entry=0x7fffc03be160, disk=disk at entry=0x7fffb4002ae0)
    at qemu/qemu_domain.c:1087
    disk=0x7fffb4002ae0, vm=0x7fffc03a2580, driver=0x7fffc02ca390,
    conn=0x7fffb00009a0) at qemu/qemu_hotplug.c:355

Upon entry to qemuDomainAttachVirtioDiskDevice, src->encryption points
at a valid 'secret' buffer w/ nsecrets == 1; however, the call to
qemuDomainDetermineDiskChain will call virStorageFileGetMetadata
and eventually virStorageFileGetMetadataInternal where the src->encryption
was overwritten when probing the volume.

Commit id 'a48c7141' added code to virStorageFileGetMetadataInternal
to determine if the disk/volume would use/need encryption and allocated
a meta->encryption. This overwrote an existing encryption buffer
already provided by the XML

This patch adds an argument to virStorageFileGetMetadataInternal in
order to avoid the encryption probe of the volume. This will only be
be set to true for the storage volume buffer/fd parsing.

Signed-off-by: John Ferlan <jferlan at redhat.com>
---

 NB: This I believe the bare bones that could be done. If desired, I could
     also modify virStorageFileGetMetadata[Recurse] and all the callers in
     order to specifically add a 'probe_encryption' variable there as well.
     For this path, the "root" callers would add the 'false', hence the
     reason I chose to just modify the *Recurse function to pass 'false'.

     I have tested creating a luks volume and of course hot plugging with
     this patch...


 src/storage/storage_driver.c | 2 +-
 src/util/virstoragefile.c    | 8 +++++---
 src/util/virstoragefile.h    | 3 ++-
 3 files changed, 8 insertions(+), 5 deletions(-)

diff --git a/src/storage/storage_driver.c b/src/storage/storage_driver.c
index 6f1e372..fe0e164 100644
--- a/src/storage/storage_driver.c
+++ b/src/storage/storage_driver.c
@@ -3239,7 +3239,7 @@ virStorageFileGetMetadataRecurse(virStorageSourcePtr src,
                                               &buf)) < 0)
         goto cleanup;
 
-    if (virStorageFileGetMetadataInternal(src, buf, headerLen,
+    if (virStorageFileGetMetadataInternal(src, buf, headerLen, false,
                                           &backingFormat) < 0)
         goto cleanup;
 
diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 471aa1f..4b6bde3 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -928,6 +928,7 @@ int
 virStorageFileGetMetadataInternal(virStorageSourcePtr meta,
                                   char *buf,
                                   size_t len,
+                                  bool probe_encryption,
                                   int *backingFormat)
 {
     int ret = -1;
@@ -946,7 +947,7 @@ virStorageFileGetMetadataInternal(virStorageSourcePtr meta,
         goto cleanup;
     }
 
-    if (fileTypeInfo[meta->format].cryptInfo != NULL) {
+    if (probe_encryption && fileTypeInfo[meta->format].cryptInfo != NULL) {
         for (i = 0; fileTypeInfo[meta->format].cryptInfo[i].format != 0; i++) {
             if (virStorageFileHasEncryptionFormat(&fileTypeInfo[meta->format].cryptInfo[i],
                                                   buf, len)) {
@@ -1129,7 +1130,7 @@ virStorageFileGetMetadataFromBuf(const char *path,
     if (!(ret = virStorageFileMetadataNew(path, format)))
         return NULL;
 
-    if (virStorageFileGetMetadataInternal(ret, buf, len,
+    if (virStorageFileGetMetadataInternal(ret, buf, len, true,
                                           backingFormat) < 0) {
         virStorageSourceFree(ret);
         return NULL;
@@ -1200,7 +1201,8 @@ virStorageFileGetMetadataFromFD(const char *path,
         goto cleanup;
     }
 
-    if (virStorageFileGetMetadataInternal(meta, buf, len, backingFormat) < 0)
+    if (virStorageFileGetMetadataInternal(meta, buf, len, true,
+                                          backingFormat) < 0)
         goto cleanup;
 
     if (S_ISREG(sb.st_mode))
diff --git a/src/util/virstoragefile.h b/src/util/virstoragefile.h
index 3d09468..bb71372 100644
--- a/src/util/virstoragefile.h
+++ b/src/util/virstoragefile.h
@@ -289,8 +289,9 @@ int virStorageFileProbeFormatFromBuf(const char *path,
 int virStorageFileGetMetadataInternal(virStorageSourcePtr meta,
                                       char *buf,
                                       size_t len,
+                                      bool probe_encryption,
                                       int *backingFormat)
-    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(4);
+    ATTRIBUTE_NONNULL(1) ATTRIBUTE_NONNULL(2) ATTRIBUTE_NONNULL(5);
 
 virStorageSourcePtr virStorageFileGetMetadataFromFD(const char *path,
                                                     int fd,
-- 
2.7.4