Hi, On Tue, Dec 01, 2015 at 03:11:05PM +0100, Peter Krempa wrote:

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.

It seems this broke the build on i386: https://buildd.debian.org/status/fetch.php?pkg=libvirt&arch=i386&ver=1.3.0-1&stamp=1450436203 (search for memory-align-fail) I did not investigate further yet though. Cheers, -- Guido

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 ed21245..a872598 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 0000000..bdbdc5b --- /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 6513651..37f806e 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); -- 2.6.2

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