[PATCH 4/7] qemu: Move 'shmem' device size validation to qemu_validate

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(a)redhat.com&gt; --- 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