[libvirt] [PATCH v2 0/2] don't crash on a tpm device with no backends
v2: added tests v1: https://www.redhat.com/archives/libvir-list/2013-May/msg00651.html Ján Tomko (2): tests: files named '.*-invalid.xml' should fail validation conf: don't crash on a tpm device with no backends src/conf/domain_conf.c | 6 +++++ .../qemuxml2argv-tpm-no-backend-invalid.xml | 27 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ tests/schematestutils.sh | 12 ++++++++++ 4 files changed, 47 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-tpm-no-backend-invalid.xml -- 1.8.1.5
Currently, using an invalid XML in tests fails, because the schema test expects all of them to be valid. Treat files with -invalid.xml suffix as invalid and expect them to fail validation. --- tests/schematestutils.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/tests/schematestutils.sh b/tests/schematestutils.sh index 4361221..e739b99 100644 --- a/tests/schematestutils.sh +++ b/tests/schematestutils.sh @@ -20,6 +20,18 @@ do result=`$cmd 2>&1` ret=$? + grep -- '-invalid.xml$' <<< "$xml" 2>&1 >/dev/null + invalid=$? + + # per xmllint man page, the return codes for validation error + # are 3 and 4 + if test $invalid -eq 0; then + if test $ret -eq 4 || test $ret -eq 3; then + ret=0 + elif test $ret -eq 0; then + ret=3 + fi + fi test_result $n $(basename $(dirname $xml))"/"$(basename $xml) $ret if test "$verbose" = "1" && test $ret != 0 ; then printf '%s\n' "$cmd" "$result" -- 1.8.1.5
On Thu, May 09, 2013 at 02:18:12PM +0200, Ján Tomko wrote:
Currently, using an invalid XML in tests fails, because the schema test expects all of them to be valid.
Treat files with -invalid.xml suffix as invalid and expect them to fail validation. --- tests/schematestutils.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/tests/schematestutils.sh b/tests/schematestutils.sh index 4361221..e739b99 100644 --- a/tests/schematestutils.sh +++ b/tests/schematestutils.sh @@ -20,6 +20,18 @@ do result=`$cmd 2>&1` ret=$?
+ grep -- '-invalid.xml$' <<< "$xml" 2>&1 >/dev/null + invalid=$? + + # per xmllint man page, the return codes for validation error + # are 3 and 4 + if test $invalid -eq 0; then + if test $ret -eq 4 || test $ret -eq 3; then + ret=0 + elif test $ret -eq 0; then + ret=3 + fi + fi test_result $n $(basename $(dirname $xml))"/"$(basename $xml) $ret if test "$verbose" = "1" && test $ret != 0 ; then printf '%s\n' "$cmd" "$result"
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 05/09/2013 06:18 AM, Ján Tomko wrote:
Currently, using an invalid XML in tests fails, because the schema test expects all of them to be valid.
Treat files with -invalid.xml suffix as invalid and expect them to fail validation. --- tests/schematestutils.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+)
diff --git a/tests/schematestutils.sh b/tests/schematestutils.sh index 4361221..e739b99 100644 --- a/tests/schematestutils.sh +++ b/tests/schematestutils.sh @@ -20,6 +20,18 @@ do result=`$cmd 2>&1` ret=$?
+ grep -- '-invalid.xml$' <<< "$xml" 2>&1 >/dev/null
<<< is not portable shell, but this script wants to use /bin/sh. You need to either rewrite the script to use /bin/bash, or avoid <<<. And I think it's easier to just use portable shell: invalid=false case $xml in *-invalid.xml) invalid=: ;; esac
+ invalid=$? + + # per xmllint man page, the return codes for validation error + # are 3 and 4 + if test $invalid -eq 0; then + if test $ret -eq 4 || test $ret -eq 3; then + ret=0 + elif test $ret -eq 0; then + ret=3 + fi + fi
Or even make the case statement check invalid and exit status all at the same time. I see you already pushed, so I'll write the followup. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
Print an error instead of crashing when a TPM device without a backend is specified. Add a test for tpm device with no backend, which should fail with a parse error. https://bugzilla.redhat.com/show_bug.cgi?id=961252 --- src/conf/domain_conf.c | 6 +++++ .../qemuxml2argv-tpm-no-backend-invalid.xml | 27 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 35 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-tpm-no-backend-invalid.xml diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 6b71727..b7e253e 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -6988,6 +6988,12 @@ virDomainTPMDefParseXML(const xmlNodePtr node, goto error; } + if (nbackends == 0) { + virReportError(VIR_ERR_XML_ERROR, "%s", + _("missing TPM device backend")); + goto error; + } + if (!(backend = virXMLPropString(backends[0], "type"))) { virReportError(VIR_ERR_XML_ERROR, "%s", _("missing TPM device backend type")); diff --git a/tests/qemuxml2argvdata/qemuxml2argv-tpm-no-backend-invalid.xml b/tests/qemuxml2argvdata/qemuxml2argv-tpm-no-backend-invalid.xml new file mode 100644 index 0000000..3b17ff7 --- /dev/null +++ b/tests/qemuxml2argvdata/qemuxml2argv-tpm-no-backend-invalid.xml @@ -0,0 +1,27 @@ +<domain type='qemu'> + <name>TPM-VM</name> + <uuid>11d7cd22-da89-3094-6212-079a48a309a1</uuid> + <memory unit='KiB'>2097152</memory> + <currentMemory unit='KiB'>512288</currentMemory> + <vcpu placement='static'>1</vcpu> + <os> + <type arch='x86_64' machine='pc-0.12'>hvm</type> + <boot dev='hd'/> + <bootmenu enable='yes'/> + </os> + <features> + <acpi/> + </features> + <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='usb' index='0'/> + <controller type='pci' index='0' model='pci-root'/> + <tpm model='tpm-tis'> + </tpm> + <memballoon model='virtio'/> + </devices> +</domain> diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c index 1286273..b1bf9db 100644 --- a/tests/qemuxml2argvtest.c +++ b/tests/qemuxml2argvtest.c @@ -970,6 +970,8 @@ mymain(void) DO_TEST("tpm-passthrough", QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_TIS); + DO_TEST_PARSE_ERROR("tpm-no-backend-invalid", QEMU_CAPS_DEVICE, + QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_TIS); DO_TEST("pci-autoadd-addr", QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE); DO_TEST("pci-autoadd-idx", QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_PCI_BRIDGE); -- 1.8.1.5
On Thu, May 09, 2013 at 02:18:13PM +0200, Ján Tomko wrote:
Print an error instead of crashing when a TPM device without a backend is specified.
Add a test for tpm device with no backend, which should fail with a parse error.
https://bugzilla.redhat.com/show_bug.cgi?id=961252 --- src/conf/domain_conf.c | 6 +++++ .../qemuxml2argv-tpm-no-backend-invalid.xml | 27 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 35 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-tpm-no-backend-invalid.xml
ACK Daniel -- |: http://berrange.com -o- http://www.flickr.com/photos/dberrange/ :| |: http://libvirt.org -o- http://virt-manager.org :| |: http://autobuild.org -o- http://search.cpan.org/~danberr/ :| |: http://entangle-photo.org -o- http://live.gnome.org/gtk-vnc :|
On 05/09/2013 02:20 PM, Daniel P. Berrange wrote:
On Thu, May 09, 2013 at 02:18:13PM +0200, Ján Tomko wrote:
Print an error instead of crashing when a TPM device without a backend is specified.
Add a test for tpm device with no backend, which should fail with a parse error.
https://bugzilla.redhat.com/show_bug.cgi?id=961252 --- src/conf/domain_conf.c | 6 +++++ .../qemuxml2argv-tpm-no-backend-invalid.xml | 27 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 35 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-tpm-no-backend-invalid.xml
ACK
Thanks, I've pushed the series. Jan
On 05/09/2013 06:18 AM, Ján Tomko wrote:
Print an error instead of crashing when a TPM device without a backend is specified.
Add a test for tpm device with no backend, which should fail with a parse error.
https://bugzilla.redhat.com/show_bug.cgi?id=961252 --- src/conf/domain_conf.c | 6 +++++ .../qemuxml2argv-tpm-no-backend-invalid.xml | 27 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 35 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-tpm-no-backend-invalid.xml
Awesome that we've enhanced the testsuite to check RNG parser rejection of invalid xml!
DO_TEST("tpm-passthrough", QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_TIS); + DO_TEST_PARSE_ERROR("tpm-no-backend-invalid", QEMU_CAPS_DEVICE, + QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_TIS);
Should we be tweaking things for the other xml files (renaming files and/or tightening RNG) so that other uses of DO_TEST_PARSE_ERROR can also stress RNG validation? tests/qemuxml2argvtest.c: DO_TEST_PARSE_ERROR("boot-dev+order", tests/qemuxml2argvtest.c: DO_TEST_PARSE_ERROR("usb-ich9-no-companion", tests/qemuxml2argvtest.c: DO_TEST_PARSE_ERROR("usb-none-other", tests/qemuxml2argvtest.c: DO_TEST_PARSE_ERROR("usb-none-hub", tests/qemuxml2argvtest.c: DO_TEST_PARSE_ERROR("usb-none-usbtablet", -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
On 05/09/2013 07:22 PM, Eric Blake wrote:
On 05/09/2013 06:18 AM, Ján Tomko wrote:
Print an error instead of crashing when a TPM device without a backend is specified.
Add a test for tpm device with no backend, which should fail with a parse error.
https://bugzilla.redhat.com/show_bug.cgi?id=961252 --- src/conf/domain_conf.c | 6 +++++ .../qemuxml2argv-tpm-no-backend-invalid.xml | 27 ++++++++++++++++++++++ tests/qemuxml2argvtest.c | 2 ++ 3 files changed, 35 insertions(+) create mode 100644 tests/qemuxml2argvdata/qemuxml2argv-tpm-no-backend-invalid.xml
Awesome that we've enhanced the testsuite to check RNG parser rejection of invalid xml!
DO_TEST("tpm-passthrough", QEMU_CAPS_DEVICE, QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_TIS); + DO_TEST_PARSE_ERROR("tpm-no-backend-invalid", QEMU_CAPS_DEVICE, + QEMU_CAPS_DEVICE_TPM_PASSTHROUGH, QEMU_CAPS_DEVICE_TPM_TIS);
Should we be tweaking things for the other xml files (renaming files and/or tightening RNG) so that other uses of DO_TEST_PARSE_ERROR can also stress RNG validation?
tests/qemuxml2argvtest.c: DO_TEST_PARSE_ERROR("boot-dev+order", tests/qemuxml2argvtest.c: DO_TEST_PARSE_ERROR("usb-ich9-no-companion", tests/qemuxml2argvtest.c: DO_TEST_PARSE_ERROR("usb-none-other", tests/qemuxml2argvtest.c: DO_TEST_PARSE_ERROR("usb-none-hub", tests/qemuxml2argvtest.c: DO_TEST_PARSE_ERROR("usb-none-usbtablet",
I think none of these errors can be found by RNG validation, so this is only good for new files. Jan
participants (3)
-
Daniel P. Berrange -
Eric Blake -
Ján Tomko