From b4478c16c02f28d88673709947124c6ea2fb4c7b Mon Sep 17 00:00:00 2001 From: John Ferlan Date: Wed, 17 Aug 2016 10:25:43 -0400 Subject: [PATCH] qemu: Fix crash hot plugging luks volume 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. --- src/util/virstoragefile.c | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/util/virstoragefile.c b/src/util/virstoragefile.c index 471aa1fcc4..feeb06173f 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; + int expt_fmt = fileTypeInfo[meta->format].cryptInfo[i].format; + if (!meta->encryption) { + if (VIR_ALLOC(meta->encryption) < 0) + goto cleanup; - meta->encryption->format = fileTypeInfo[meta->format].cryptInfo[i].format; + 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; + } + } } } }