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@xxxxxxxxxx
> https://www.redhat.com/mailman/listinfo/libvir-list
>
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list