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