The 'size' of a 'shmem' device is parsed and formatted as a "scaled"
value, stored in bytes, but the formatting scale is mebibytes. This
precission loss combined with the fact that the value was validated only
when starting and the size is formatted only when non-zero meant that
on first parse a value < 1 MiB would be accepted, but would be formatted
to the XML as 0 MiB as it was non-zero but truncated and a subsequent
parse would parse of such XML would parse it as 0 bytes, which in turn
would be interpreted as 'default' size.
Fix the issue by moving the validator, which ensures that the number is
a power of two and more than 1 MiB to the validator code so that it'll
be rejected at XML parsing time.
Signed-off-by: Peter Krempa <pkrempa@xxxxxxxxxx>
---
src/qemu/qemu_command.c | 19 -----------
src/qemu/qemu_validate.c | 9 +++++
.../shmem-invalid-size.x86_64-latest.err | 2 +-
.../shmem-small-size.x86_64-latest.err | 2 +-
.../shmem-invalid-size.x86_64-latest.xml | 34 -------------------
.../shmem-small-size.x86_64-latest.xml | 34 -------------------
tests/qemuxmlconftest.c | 4 +--
7 files changed, 13 insertions(+), 91 deletions(-)
delete mode 100644 tests/qemuxml2xmloutdata/shmem-invalid-size.x86_64-latest.xml
delete mode 100644 tests/qemuxml2xmloutdata/shmem-small-size.x86_64-latest.xml
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index 653817173b..e3d728aa82 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -9078,25 +9078,6 @@ qemuBuildShmemCommandLine(virCommand *cmd,
g_autoptr(virJSONValue) memProps = NULL;
g_autoptr(virJSONValue) devProps = NULL;
- if (shmem->size) {
- /*
- * Thanks to our parsing code, we have a guarantee that the
- * size is power of two and is at least a mebibyte in size.
- * But because it may change in the future, the checks are
- * doubled in here.
- */
- if (shmem->size & (shmem->size - 1)) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("shmem size must be a power of two"));
- return -1;
- }
- if (shmem->size < 1024 * 1024) {
- virReportError(VIR_ERR_XML_ERROR, "%s",
- _("shmem size must be at least 1 MiB (1024 KiB)"));
- return -1;
- }
- }
-
if (shmem->info.type != VIR_DOMAIN_DEVICE_ADDRESS_TYPE_PCI) {
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
_("only 'pci' addresses are supported for the shared memory device"));
diff --git a/src/qemu/qemu_validate.c b/src/qemu/qemu_validate.c
index b22d3618fe..01f65c0866 100644
--- a/src/qemu/qemu_validate.c
+++ b/src/qemu/qemu_validate.c
@@ -5118,6 +5118,15 @@ static int
qemuValidateDomainDeviceDefShmem(virDomainShmemDef *shmem,
virQEMUCaps *qemuCaps)
{
+ if (shmem->size > 0) {
+ if (shmem->size < 1024 * 1024 ||
+ !VIR_IS_POW2(shmem->size)) {
+ virReportError(VIR_ERR_XML_ERROR, "%s",
+ _("shmem size must be a power of 2 and at least 1 MiB (1024 KiB)"));
+ return -1;
+ }
+ }
+
switch (shmem->model) {
case VIR_DOMAIN_SHMEM_MODEL_IVSHMEM:
virReportError(VIR_ERR_CONFIG_UNSUPPORTED, "%s",
diff --git a/tests/qemuxml2argvdata/shmem-invalid-size.x86_64-latest.err b/tests/qemuxml2argvdata/shmem-invalid-size.x86_64-latest.err
index 623bd8e5dd..5409cb73c2 100644
--- a/tests/qemuxml2argvdata/shmem-invalid-size.x86_64-latest.err
+++ b/tests/qemuxml2argvdata/shmem-invalid-size.x86_64-latest.err
@@ -1 +1 @@
-XML error: shmem size must be a power of two
+XML error: shmem size must be a power of 2 and at least 1 MiB (1024 KiB)
diff --git a/tests/qemuxml2argvdata/shmem-small-size.x86_64-latest.err b/tests/qemuxml2argvdata/shmem-small-size.x86_64-latest.err
index b5fcd8b4cf..5409cb73c2 100644
--- a/tests/qemuxml2argvdata/shmem-small-size.x86_64-latest.err
+++ b/tests/qemuxml2argvdata/shmem-small-size.x86_64-latest.err
@@ -1 +1 @@
-XML error: shmem size must be at least 1 MiB (1024 KiB)
+XML error: shmem size must be a power of 2 and at least 1 MiB (1024 KiB)
diff --git a/tests/qemuxml2xmloutdata/shmem-invalid-size.x86_64-latest.xml b/tests/qemuxml2xmloutdata/shmem-invalid-size.x86_64-latest.xml
deleted file mode 100644
index cc7b63da94..0000000000
--- a/tests/qemuxml2xmloutdata/shmem-invalid-size.x86_64-latest.xml
+++ /dev/null
@@ -1,34 +0,0 @@
-<domain type='qemu'>
- <name>QEMUGuest1</name>
- <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
- <memory unit='KiB'>219136</memory>
- <currentMemory unit='KiB'>219136</currentMemory>
- <vcpu placement='static'>1</vcpu>
- <os>
- <type arch='x86_64' machine='pc'>hvm</type>
- <boot dev='hd'/>
- </os>
- <cpu mode='custom' match='exact' check='none'>
- <model fallback='forbid'>qemu64</model>
- </cpu>
- <clock offset='utc'/>
- <on_poweroff>destroy</on_poweroff>
- <on_reboot>restart</on_reboot>
- <on_crash>destroy</on_crash>
- <devices>
- <emulator>/usr/bin/qemu-system-x86_64</emulator>
- <controller type='usb' index='0' model='piix3-uhci'>
- <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
- </controller>
- <controller type='pci' index='0' model='pci-root'/>
- <input type='mouse' bus='ps2'/>
- <input type='keyboard' bus='ps2'/>
- <audio id='1' type='none'/>
- <memballoon model='none'/>
- <shmem name='shmem0'>
- <model type='ivshmem-plain'/>
- <size unit='M'>12</size>
- <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
- </shmem>
- </devices>
-</domain>
diff --git a/tests/qemuxml2xmloutdata/shmem-small-size.x86_64-latest.xml b/tests/qemuxml2xmloutdata/shmem-small-size.x86_64-latest.xml
deleted file mode 100644
index fc6f9dda7d..0000000000
--- a/tests/qemuxml2xmloutdata/shmem-small-size.x86_64-latest.xml
+++ /dev/null
@@ -1,34 +0,0 @@
-<domain type='qemu'>
- <name>QEMUGuest1</name>
- <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
- <memory unit='KiB'>219136</memory>
- <currentMemory unit='KiB'>219136</currentMemory>
- <vcpu placement='static'>1</vcpu>
- <os>
- <type arch='x86_64' machine='pc'>hvm</type>
- <boot dev='hd'/>
- </os>
- <cpu mode='custom' match='exact' check='none'>
- <model fallback='forbid'>qemu64</model>
- </cpu>
- <clock offset='utc'/>
- <on_poweroff>destroy</on_poweroff>
- <on_reboot>restart</on_reboot>
- <on_crash>destroy</on_crash>
- <devices>
- <emulator>/usr/bin/qemu-system-x86_64</emulator>
- <controller type='usb' index='0' model='piix3-uhci'>
- <address type='pci' domain='0x0000' bus='0x00' slot='0x01' function='0x2'/>
- </controller>
- <controller type='pci' index='0' model='pci-root'/>
- <input type='mouse' bus='ps2'/>
- <input type='keyboard' bus='ps2'/>
- <audio id='1' type='none'/>
- <memballoon model='none'/>
- <shmem name='shmem0'>
- <model type='ivshmem-plain'/>
- <size unit='M'>0</size>
- <address type='pci' domain='0x0000' bus='0x00' slot='0x02' function='0x0'/>
- </shmem>
- </devices>
-</domain>
diff --git a/tests/qemuxmlconftest.c b/tests/qemuxmlconftest.c
index 267e4e579e..49fe9c0cae 100644
--- a/tests/qemuxmlconftest.c
+++ b/tests/qemuxmlconftest.c
@@ -2455,9 +2455,9 @@ mymain(void)
DO_TEST_CAPS_ARCH_LATEST_FULL("fips-enabled", "x86_64", ARG_FLAGS, FLAG_FIPS_HOST);
DO_TEST_CAPS_LATEST("shmem-plain-doorbell");
- DO_TEST_CAPS_LATEST_FAILURE("shmem-invalid-size");
+ DO_TEST_CAPS_LATEST_PARSE_ERROR("shmem-invalid-size");
DO_TEST_CAPS_LATEST_FAILURE("shmem-invalid-address");
- DO_TEST_CAPS_LATEST_FAILURE("shmem-small-size");
+ DO_TEST_CAPS_LATEST_PARSE_ERROR("shmem-small-size");
DO_TEST_CAPS_LATEST_PARSE_ERROR("shmem-msi-only");
DO_TEST_CAPS_LATEST("cpu-host-passthrough-features");
--
2.43.0
_______________________________________________
Devel mailing list -- devel@xxxxxxxxxxxxxxxxx
To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxx
