qemu: Prevent storage causing too much nested XML

Since libvirt stores the backing chain into the XML in a nested way it
is the prime possibility to hit libxml2's parsing limit of 256 layers.

Introduce code which will crawl the backing chain and verify that it's
not too deep. The maximum nesting is set to 200 layers so that there's
still some space left for additional properties or nesting into snapshot
XMLs.

The check is applied to all disk use cases (starting, hotplug, media
change) as well as block copy which changes image and snapshots.

We simply report an error and refuse the operation.

Without this check a restart of libvirtd would result in the status XML
failing to be parsed and thus losing the VM.

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

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index 0ebae773c26b59a8186b1a02cebc2ad8c8b63575..2c75a79670123d1d76d2acfcfeb58112622f6125 100644 (file)
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -9984,6 +9984,53 @@ qemuDomainStorageAlias(const char *device, int depth)
 }
 
 
+/**
+ * qemuDomainStorageSourceValidateDepth:
+ * @src: storage source chain to validate
+ * @add: offsets the calculated number of images
+ * @diskdst: optional disk target to use in error message
+ *
+ * The XML parser limits the maximum element nesting to 256 layers. As libvirt
+ * reports the chain into the status and in some cases the config XML we must
+ * validate that any user-provided chains will not exceed the XML nesting limit
+ * when formatted to the XML.
+ *
+ * This function validates that the storage source chain starting @src is at
+ * most 200 layers deep. @add modifies the calculated value to offset the number
+ * to allow checking cases when new layers are going to be added to the chain.
+ *
+ * Returns 0 on success and -1 if the chain is too deep. Error is reported.
+ */
+int
+qemuDomainStorageSourceValidateDepth(virStorageSourcePtr src,
+                                     int add,
+                                     const char *diskdst)
+{
+    virStorageSourcePtr n;
+    size_t nlayers = 0;
+
+    for (n = src; virStorageSourceIsBacking(n); n = n->backingStore)
+        nlayers++;
+
+    nlayers += add;
+
+    if (nlayers > 200) {
+        if (diskdst)
+            virReportError(VIR_ERR_OPERATION_UNSUPPORTED,
+                           _("backing chains more than 200 layers deep are not "
+                             "supported for disk '%s'"), diskdst);
+        else
+            virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
+                           _("backing chains more than 200 layers deep are not "
+                             "supported"));
+
+        return -1;
+    }
+
+    return 0;
+}
+
+
 /**
  * qemuDomainDetermineDiskChain:
  * @driver: qemu driver object
@@ -10073,8 +10120,12 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
 
     /* We skipped to the end of the chain. Skip detection if there's the
      * terminator. (An allocated but empty backingStore) */
-    if (src->backingStore)
+    if (src->backingStore) {
+        if (qemuDomainStorageSourceValidateDepth(disksrc, 0, disk->dst) < 0)
+            return -1;
+
         return 0;
+    }
 
     qemuDomainGetImageIds(cfg, vm, src, disksrc, &uid, &gid);
 
@@ -10093,6 +10144,9 @@ qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
             return -1;
     }
 
+    if (qemuDomainStorageSourceValidateDepth(disksrc, 0, disk->dst) < 0)
+        return -1;
+
     return 0;
 }
 

diff --git a/src/qemu/qemu_domain.h b/src/qemu/qemu_domain.h
index d097f233428c2e3588b322e247433b9855b41b09..33eb501e2a4c6faf1c0a833c0d0db5aa4c868af5 100644 (file)
--- a/src/qemu/qemu_domain.h
+++ b/src/qemu/qemu_domain.h
@@ -802,6 +802,10 @@ int qemuDomainCheckDiskPresence(virQEMUDriverPtr driver,
                                 virDomainObjPtr vm,
                                 unsigned int flags);
 
+int qemuDomainStorageSourceValidateDepth(virStorageSourcePtr src,
+                                         int add,
+                                         const char *diskdst);
+
 int qemuDomainDetermineDiskChain(virQEMUDriverPtr driver,
                                  virDomainObjPtr vm,
                                  virDomainDiskDefPtr disk,

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 78f5471b794bde1db237852f16937a14dc7da64c..aa9efc684f38b5226f3978e7436dc9fc93d24930 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -15315,6 +15315,9 @@ qemuDomainSnapshotDiskDataCollectOne(virQEMUDriverPtr driver,
 
     dd->disk = disk;
 
+    if (qemuDomainStorageSourceValidateDepth(disk->src, 1, disk->dst) < 0)
+        return -1;
+
     if (!(dd->src = virStorageSourceCopy(snapdisk->src, false)))
         return -1;
 
@@ -18330,6 +18333,9 @@ qemuDomainBlockCopyCommonValidateUserMirrorBackingStore(virStorageSourcePtr mirr
                            _("backingStore of mirror without VIR_DOMAIN_BLOCK_COPY_SHALLOW doesn't make sense"));
             return -1;
         }
+
+        if (qemuDomainStorageSourceValidateDepth(mirror, 0, NULL) < 0)
+            return -1;
     }
 
     return 0;