edecca
From 27213f01f9320cf0fec49980f78a100e64025ba4 Mon Sep 17 00:00:00 2001
edecca
Message-Id: <27213f01f9320cf0fec49980f78a100e64025ba4@dist-git>
edecca
From: Andrea Bolognani <abologna@redhat.com>
edecca
Date: Fri, 7 Sep 2018 17:53:32 +0200
edecca
Subject: [PATCH] conf: Fix check for chardev source path
edecca
MIME-Version: 1.0
edecca
Content-Type: text/plain; charset=UTF-8
edecca
Content-Transfer-Encoding: 8bit
edecca
edecca
Attempting to use a chardev definition like
edecca
edecca
  <serial type='unix'>
edecca
    <target type='isa-serial'/>
edecca
  </serial>
edecca
edecca
correctly results in an error being reported, since the source
edecca
path - a required piece of information - is missing; however,
edecca
the very similar
edecca
edecca
  <serial type='unix'>
edecca
    <target type='pci-serial'/>
edecca
  </serial>
edecca
edecca
was happily accepted by libvirt, only to result in libvirtd
edecca
crashing as soon as the guest was started.
edecca
edecca
The issue was caused by checking the chardev's targetType
edecca
against whitelisted values from virDomainChrChannelTargetType
edecca
without first checking the chardev's deviceType to make sure
edecca
it is actually a channel, for which the check makes sense,
edecca
rather than a different type of chardev.
edecca
edecca
The only reason this wasn't spotted earlier is that the
edecca
whitelisted values just so happen to correspond to USB and
edecca
PCI serial devices and Xen and UML consoles respectively,
edecca
all of which are fairly uncommon.
edecca
edecca
https://bugzilla.redhat.com/show_bug.cgi?id=1609720
edecca
edecca
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
edecca
Reviewed-by: Ján Tomko <jtomko@redhat.com>
edecca
(cherry picked from commit 614193fac67445a7e92bf620ffef726ed1bd6f07)
edecca
edecca
https://bugzilla.redhat.com/show_bug.cgi?id=1609723
edecca
edecca
Signed-off-by: Andrea Bolognani <abologna@redhat.com>
edecca
Reviewed-by: Erik Skultety <eskultet@redhat.com>
edecca
---
edecca
 src/conf/domain_conf.c                            | 11 +++++++----
edecca
 .../serial-unix-missing-source.xml                | 15 +++++++++++++++
edecca
 tests/qemuxml2argvtest.c                          |  1 +
edecca
 3 files changed, 23 insertions(+), 4 deletions(-)
edecca
 create mode 100644 tests/qemuxml2argvdata/serial-unix-missing-source.xml
edecca
edecca
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
edecca
index a881b43b51..240b33f28c 100644
edecca
--- a/src/conf/domain_conf.c
edecca
+++ b/src/conf/domain_conf.c
edecca
@@ -5523,11 +5523,14 @@ virDomainChrSourceDefValidate(const virDomainChrSourceDef *def,
edecca
         break;
edecca
 
edecca
     case VIR_DOMAIN_CHR_TYPE_UNIX:
edecca
-        /* path can be auto generated */
edecca
+        /* The source path can be auto generated for certain specific
edecca
+         * types of channels, but in most cases we should report an
edecca
+         * error if the user didn't provide it */
edecca
         if (!def->data.nix.path &&
edecca
-            (!chr_def ||
edecca
-             (chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN &&
edecca
-              chr_def->targetType != VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO))) {
edecca
+            !(chr_def &&
edecca
+              chr_def->deviceType == VIR_DOMAIN_CHR_DEVICE_TYPE_CHANNEL &&
edecca
+              (chr_def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_XEN ||
edecca
+               chr_def->targetType == VIR_DOMAIN_CHR_CHANNEL_TARGET_TYPE_VIRTIO))) {
edecca
             virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
edecca
                            _("Missing source path attribute for char device"));
edecca
             return -1;
edecca
diff --git a/tests/qemuxml2argvdata/serial-unix-missing-source.xml b/tests/qemuxml2argvdata/serial-unix-missing-source.xml
edecca
new file mode 100644
edecca
index 0000000000..1e1221f12d
edecca
--- /dev/null
edecca
+++ b/tests/qemuxml2argvdata/serial-unix-missing-source.xml
edecca
@@ -0,0 +1,15 @@
edecca
+<domain type='qemu'>
edecca
+  <name>guest</name>
edecca
+  <uuid>c7a5fdbd-edaf-9455-926a-d65c16db1809</uuid>
edecca
+  <memory unit='KiB'>1048576</memory>
edecca
+  <vcpu placement='static'>1</vcpu>
edecca
+  <os>
edecca
+    <type arch='aarch64' machine='virt'>hvm</type>
edecca
+  </os>
edecca
+  <devices>
edecca
+    <emulator>/usr/bin/qemu-system-aarch64</emulator>
edecca
+    <serial type='unix'>
edecca
+      <target type='pci-serial'/>
edecca
+    </serial>
edecca
+  </devices>
edecca
+</domain>
edecca
diff --git a/tests/qemuxml2argvtest.c b/tests/qemuxml2argvtest.c
edecca
index 608a2b6ce3..ebe9c8a131 100644
edecca
--- a/tests/qemuxml2argvtest.c
edecca
+++ b/tests/qemuxml2argvtest.c
edecca
@@ -1363,6 +1363,7 @@ mymain(void)
edecca
     DO_TEST("serial-unix-chardev",
edecca
             QEMU_CAPS_DEVICE_ISA_SERIAL);
edecca
     DO_TEST_CAPS_LATEST("serial-unix-chardev");
edecca
+    DO_TEST_PARSE_ERROR("serial-unix-missing-source", NONE);
edecca
     DO_TEST("serial-tcp-chardev",
edecca
             QEMU_CAPS_DEVICE_ISA_SERIAL);
edecca
     DO_TEST("serial-udp-chardev",
edecca
-- 
edecca
2.19.1
edecca