8:11 a.m.
Peter Krempa (2):
schema: Allow > UINT_MAX KiB of memory for NUMA nodes
qemu: domain: Prevent overflows in memory alignment code
docs/schemas/domaincommon.rng | 4 +--
src/qemu/qemu_domain.c | 27 ++++++++++++++++++++
.../qemuxml2argv-memory-align-fail.xml | 29 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 1 +
4 files changed, 58 insertions(+), 3 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-align-fail.xml
--
2.6.2
8:11 a.m.
New subject: [libvirt] [PATCH 1/2] schema: Allow > UINT_MAX KiB of memory for NUMA nodes
Using more than 4TiB of memory per NUMA node would not be possible to
express in the XML without violating the schema. Not that such boxes
would be common, but we should use a longer type at this point.
The pattern is not necessary since libvirt redefines the type already in
basictypes.rng with the same pattern.
---
docs/schemas/domaincommon.rng | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/docs/schemas/domaincommon.rng b/docs/schemas/domaincommon.rng
index 8d12606..4804c69 100644
--- a/docs/schemas/domaincommon.rng
+++ b/docs/schemas/domaincommon.rng
@@ -5218,9 +5218,7 @@
</define>
<!-- Memory as an attribute is in KiB, no way to express a unit -->
<define name="memoryKB">
- <data type="unsignedInt">
- <param name="pattern">[0-9]+</param>
- </data>
+ <data type="unsignedLong"/>
</define>
<define name="domainName">
<data type="string">
--
2.6.2
8:11 a.m.
New subject: [libvirt] [PATCH 2/2] 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 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
11:26 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: domain: Prevent overflows in memory alignment code
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&...
(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(a)redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
2:39 a.m.
New subject: [libvirt] [PATCH 2/2] qemu: domain: Prevent overflows in memory alignment code
On Sun, Jan 03, 2016 at 18:26:56 +0100, Guido Günther wrote:
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&...
(search for memory-align-fail)
I did not investigate further yet though.
This should be already fixed ...
commit ace1ee225f5cd87fb095054a6a19bdcd0fa57518
Author: Peter Krempa <pkrempa(a)redhat.com>
Date: Thu Dec 10 14:36:51 2015 +0100
test: qemuxml2argv: Mock virMemoryMaxValue to remove 32/64 bit difference
Always return LLONG_MAX even on 32 bit systems. The limitation
originates from our use of "unsigned long" in several APIs. The internal
data type is unsigned long long. Make the test suite deterministic by
removing the architecture difference.
Flaw was introduced in 645881139b3d2c86acf9d644c3a1471520bc9e57 where
I've added a test that uses too large numbers.
5:05 p.m.
New subject: [libvirt] [PATCH 2/2] qemu: domain: Prevent overflows in memory alignment code
Hi,
On Mon, Jan 04, 2016 at 09:39:20AM +0100, Peter Krempa wrote:
On Sun, Jan 03, 2016 at 18:26:56 +0100, Guido Günther wrote:
> 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&...
> (search for memory-align-fail)
>
> I did not investigate further yet though.
This should be already fixed ...
commit ace1ee225f5cd87fb095054a6a19bdcd0fa57518
Author: Peter Krempa <pkrempa(a)redhat.com>
Date: Thu Dec 10 14:36:51 2015 +0100
test: qemuxml2argv: Mock virMemoryMaxValue to remove 32/64 bit difference
Always return LLONG_MAX even on 32 bit systems. The limitation
originates from our use of "unsigned long" in several APIs. The internal
data type is unsigned long long. Make the test suite deterministic by
removing the architecture difference.
Flaw was introduced in 645881139b3d2c86acf9d644c3a1471520bc9e57 where
I've added a test that uses too large numbers.
That fixes it, thanks for pointing this out!
Cheers,
-- Guido
8:18 a.m.
On 01.12.2015 15:11, Peter Krempa wrote:
Peter Krempa (2):
schema: Allow > UINT_MAX KiB of memory for NUMA nodes
qemu: domain: Prevent overflows in memory alignment code
docs/schemas/domaincommon.rng | 4 +--
src/qemu/qemu_domain.c | 27 ++++++++++++++++++++
.../qemuxml2argv-memory-align-fail.xml | 29 ++++++++++++++++++++++
tests/qemuxml2argvtest.c | 1 +
4 files changed, 58 insertions(+), 3 deletions(-)
create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-align-fail.xml
ACK and safe for the freeze.
Michal
8:48 a.m.
On Fri, Dec 04, 2015 at 15:18:16 +0100, Michal Privoznik wrote:
On 01.12.2015 15:11, Peter Krempa wrote:
> Peter Krempa (2):
> schema: Allow > UINT_MAX KiB of memory for NUMA nodes
> qemu: domain: Prevent overflows in memory alignment code
>
> docs/schemas/domaincommon.rng | 4 +--
> src/qemu/qemu_domain.c | 27 ++++++++++++++++++++
> .../qemuxml2argv-memory-align-fail.xml | 29 ++++++++++++++++++++++
> tests/qemuxml2argvtest.c | 1 +
> 4 files changed, 58 insertions(+), 3 deletions(-)
> create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-memory-align-fail.xml
>
ACK and safe for the freeze.
Pushed; Thanks
Peter
3247
days inactive
3282
days old
7 comments
3 participants
participants (3)
-
Guido Günther -
Michal Privoznik -
Peter Krempa