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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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@entry=0x7fffc03be160, disk=disk@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 a check for meta->encryption already present before
just allocating and overwriting an existing buffer. It then checks the
existing encryption data to ensure the XML provided format for the
disk matches the expected format read from the disk and errors if there
is a mismatch.

Signed-off-by: John Ferlan <jferlan@xxxxxxxxxx>
---

 v1: http://www.redhat.com/archives/libvir-list/2016-August/msg00805.html

 Tested w/ delete luks volume, create luks volume, dump luks volume,
 attach to guest, detach from guest, and repeat...

 src/util/virstoragefile.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c
index 471aa1f..feeb061 100644
--- a/src/util/virstoragefile.c
+++ b/src/util/virstoragefile.c
@@ -950,10 +950,21 @@ virStorageFileGetMetadataInternal(virStorageSourcePtr meta,
         for (i = 0; fileTypeInfo[meta->format].cryptInfo[i].format != 0; i++) {
             if (virStorageFileHasEncryptionFormat(&fileTypeInfo[meta->format].cryptInfo[i],
                                                   buf, len)) {
-                if (VIR_ALLOC(meta->encryption) < 0)
-                    goto cleanup;
-
-                meta->encryption->format = fileTypeInfo[meta->format].cryptInfo[i].format;
+                int expt_fmt = fileTypeInfo[meta->format].cryptInfo[i].format;
+                if (!meta->encryption) {
+                    if (VIR_ALLOC(meta->encryption) < 0)
+                        goto cleanup;
+
+                    meta->encryption->format = expt_fmt;
+                } else {
+                    if (meta->encryption->format != expt_fmt) {
+                        virReportError(VIR_ERR_XML_ERROR,
+                                       _("encryption format %d doesn't match "
+                                         "expected format %d"),
+                                       meta->encryption->format, expt_fmt);
+                        goto cleanup;
+                    }
+                }
             }
         }
     }
-- 
2.7.4

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list



[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]