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