From 1abebbf11f518fcd8a70133245be7b8ea4b094ea Mon Sep 17 00:00:00 2001
Message-Id: <1abebbf11f518fcd8a70133245be7b8ea4b094ea@dist-git>
From: Jonathon Jongsma <jjongsma@redhat.com>
Date: Fri, 1 May 2020 16:53:39 -0500
Subject: [PATCH] qemu: don't hold monitor and agent job when setting time

We have to assume that the guest agent may be malicious so we don't want
to allow any agent queries to block any other libvirt API. By holding
a monitor job while we're querying the agent, we open ourselves up to a
DoS.

Split the function so that the portion issuing the agent command only
holds an agent job and the portion issuing the monitor command holds
only a monitor job.

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit e005c95f56fee9ed780be7f8db103d690bd34cbd)

CVE-2019-20485

Signed-off-by: Jonathon Jongsma <jjongsma@redhat.com>
Message-Id: <20200501215341.27683-4-jjongsma@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
---
 src/qemu/qemu_driver.c | 54 +++++++++++++++++++++++++++---------------
 1 file changed, 35 insertions(+), 19 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 888c586a79..0f6641702d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19455,6 +19455,35 @@ qemuDomainGetTime(virDomainPtr dom,
     return ret;
 }
 
+static int
+qemuDomainSetTimeAgent(virQEMUDriverPtr driver,
+                       virDomainObjPtr vm,
+                       long long seconds,
+                       unsigned int nseconds,
+                       bool rtcSync)
+{
+    qemuAgentPtr agent;
+    int ret = -1;
+
+    if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_MODIFY) < 0)
+        return -1;
+
+    if (virDomainObjCheckActive(vm) < 0)
+        goto endjob;
+
+    if (!qemuDomainAgentAvailable(vm, true))
+        goto endjob;
+
+    agent = qemuDomainObjEnterAgent(vm);
+    ret = qemuAgentSetTime(agent, seconds, nseconds, rtcSync);
+    qemuDomainObjExitAgent(vm, agent);
+
+ endjob:
+    qemuDomainObjEndJob(driver, vm);
+    return ret;
+}
+
+
 static int
 qemuDomainSetTime(virDomainPtr dom,
                   long long seconds,
@@ -19464,7 +19493,6 @@ qemuDomainSetTime(virDomainPtr dom,
     virQEMUDriverPtr driver = dom->conn->privateData;
     qemuDomainObjPrivatePtr priv;
     virDomainObjPtr vm;
-    qemuAgentPtr agent;
     bool rtcSync = flags & VIR_DOMAIN_TIME_SYNC;
     int ret = -1;
     int rv;
@@ -19479,14 +19507,6 @@ qemuDomainSetTime(virDomainPtr dom,
 
     priv = vm->privateData;
 
-    if (qemuDomainObjBeginJobWithAgent(driver, vm,
-                                       QEMU_JOB_MODIFY,
-                                       QEMU_AGENT_JOB_MODIFY) < 0)
-        goto cleanup;
-
-    if (virDomainObjCheckActive(vm) < 0)
-        goto endjob;
-
     /* On x86, the rtc-reset-reinjection QMP command must be called after
      * setting the time to avoid trouble down the line. If the command is
      * not available, don't set the time at all and report an error */
@@ -19496,18 +19516,14 @@ qemuDomainSetTime(virDomainPtr dom,
         virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
                        _("cannot set time: qemu doesn't support "
                          "rtc-reset-reinjection command"));
-        goto endjob;
+        goto cleanup;
     }
 
-    if (!qemuDomainAgentAvailable(vm, true))
-        goto endjob;
-
-    agent = qemuDomainObjEnterAgent(vm);
-    rv = qemuAgentSetTime(agent, seconds, nseconds, rtcSync);
-    qemuDomainObjExitAgent(vm, agent);
+    if (qemuDomainSetTimeAgent(driver, vm, seconds, nseconds, rtcSync) < 0)
+        goto cleanup;
 
-    if (rv < 0)
-        goto endjob;
+    if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+        goto cleanup;
 
     if (virDomainObjCheckActive(vm) < 0)
         goto endjob;
@@ -19526,7 +19542,7 @@ qemuDomainSetTime(virDomainPtr dom,
     ret = 0;
 
  endjob:
-    qemuDomainObjEndJobWithAgent(driver, vm);
+    qemuDomainObjEndJob(driver, vm);
 
  cleanup:
     virDomainObjEndAPI(&vm);
-- 
2.26.2