From 645881139b3d2c86acf9d644c3a1471520bc9e57 Mon Sep 17 00:00:00 2001
From: Peter Krempa <pkrempa@redhat.com>
Date: Tue, 1 Dec 2015 14:08:37 +0100
Subject: [PATCH] qemu: domain: Prevent overflows in memory alignment code

Since libvirt for dubious historical reasons stores memory size as
kibibytes, it's possible that the alignments done in the qemu code
overflow the the maximum representable size in bytes. The XML parser
code handles them in bytes in some stages. Prevent this by doing
overflow checks when alinging the size and add a test case.

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1260576
---
 src/qemu/qemu_domain.c                        | 27 +++++++++++++++++
 .../qemuxml2argv-memory-align-fail.xml        | 29 +++++++++++++++++++
 tests/qemuxml2argvtest.c                      |  1 +
 3 files changed, 57 insertions(+)
 create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-align-fail.xml

diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
index ed212456ae..a872598965 100644
--- a/src/qemu/qemu_domain.c
+++ b/src/qemu/qemu_domain.c
@@ -3521,6 +3521,8 @@ qemuDomainGetMemoryModuleSizeAlignment(const virDomainDef *def,
 int
 qemuDomainAlignMemorySizes(virDomainDefPtr def)
 {
+    unsigned long long maxmemkb = virMemoryMaxValue(false) >> 10;
+    unsigned long long maxmemcapped = virMemoryMaxValue(true) >> 10;
     unsigned long long initialmem = 0;
     unsigned long long mem;
     unsigned long long align = qemuDomainGetMemorySizeAlignment(def);
@@ -3531,6 +3533,13 @@ qemuDomainAlignMemorySizes(virDomainDefPtr def)
     for (i = 0; i < ncells; i++) {
         mem = VIR_ROUND_UP(virDomainNumaGetNodeMemorySize(def->numa, i), align);
         initialmem += mem;
+
+        if (mem > maxmemkb) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("memory size of NUMA node '%zu' overflowed after "
+                             "alignment"), i);
+            return -1;
+        }
         virDomainNumaSetNodeMemorySize(def->numa, i, mem);
     }
 
@@ -3539,14 +3548,32 @@ qemuDomainAlignMemorySizes(virDomainDefPtr def)
     if (initialmem == 0)
         initialmem = VIR_ROUND_UP(virDomainDefGetMemoryInitial(def), align);
 
+    if (initialmem > maxmemcapped) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("initial memory size overflowed after alignment"));
+        return -1;
+    }
+
     virDomainDefSetMemoryInitial(def, initialmem);
 
     def->mem.max_memory = VIR_ROUND_UP(def->mem.max_memory, align);
+    if (def->mem.max_memory > maxmemkb) {
+        virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
+                       _("maximum memory size overflowed after alignment"));
+        return -1;
+    }
 
     /* Align memory module sizes */
     for (i = 0; i < def->nmems; i++) {
         align = qemuDomainGetMemoryModuleSizeAlignment(def, def->mems[i]);
         def->mems[i]->size = VIR_ROUND_UP(def->mems[i]->size, align);
+
+        if (def->mems[i]->size > maxmemkb) {
+            virReportError(VIR_ERR_CONFIG_UNSUPPORTED,
+                           _("size of memory module '%zu' overflowed after "
+                             "alignment"), i);
+            return -1;
+        }
     }
 
     return 0;
diff --git a/tests/qemuxml2argvdata/qemuxml2argv-memory-align-fail.xml b/tests/qemuxml2argvdata/qemuxml2argv-memory-align-fail.xml
new file mode 100644
index 0000000000..bdbdc5bbde
--- /dev/null
+++ b/tests/qemuxml2argvdata/qemuxml2argv-memory-align-fail.xml
@@ -0,0 +1,29 @@
+<domain type='qemu'>
+  <name>QEMUGuest1</name>
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
+  <maxMemory slots='16' unit='KiB'>9007199254740991</maxMemory>
+  <memory unit='KiB'>9007199254740991</memory>
+  <currentMemory unit='KiB'>9007199254740991</currentMemory>
+  <vcpu placement='static' cpuset='0-1'>2</vcpu>
+  <os>
+    <type arch='i686' machine='pc'>hvm</type>
+    <boot dev='hd'/>
+  </os>
+  <cpu>
+    <topology sockets='2' cores='1' threads='1'/>
+    <numa>
+      <cell id='0' cpus='0-1' memory='9007199254740991' unit='KiB'/>
+    </numa>
+  </cpu>
+  <clock offset='utc'/>
+  <on_poweroff>destroy</on_poweroff>
+  <on_reboot>restart</on_reboot>
+  <on_crash>destroy</on_crash>
+  <devices>
+    <emulator>/usr/bin/qemu</emulator>
+    <controller type='ide' index='0'/>
+    <controller type='usb' index='0'/>
+    <controller type='pci' index='0' model='pci-root'/>
+    <memballoon model='virtio'/>
+  </devices>
+</domain>
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
index 6513651853..37f806edd3 100644
--- a/tests/qemuxml2argvtest.c
+++ b/tests/qemuxml2argvtest.c
@@ -1660,6 +1660,7 @@ mymain(void)
     DO_TEST_PARSE_ERROR("shmem-msi-only", NONE);
     DO_TEST("cpu-host-passthrough-features", QEMU_CAPS_KVM, QEMU_CAPS_CPU_HOST);
 
+    DO_TEST_FAILURE("memory-align-fail", NONE);
     DO_TEST_FAILURE("memory-hotplug-nonuma", QEMU_CAPS_DEVICE_PC_DIMM);
     DO_TEST_FAILURE("memory-hotplug", NONE);
     DO_TEST("memory-hotplug", QEMU_CAPS_DEVICE_PC_DIMM, QEMU_CAPS_NUMA);
-- 
GitLab