[PATCH 4/8] qemu: don't hold monitor and agent job when setting time

[Jonathon Jongsma <jjongsma@xxxxxxxxxx>]

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@xxxxxxxxxx>
---
 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 e39ee2acc9..10fad8d75d 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -20256,6 +20256,35 @@ qemuDomainGetTime(virDomainPtr dom,
 }
 
 
+static int
+qemuDomainSetTimeAgent(virQEMUDriverPtr driver,
+                       virDomainObjPtr vm,
+                       long long seconds,
+                       unsigned int nseconds,
+                       bool rtcSync)
+{
+    qemuAgentPtr agent;
+    int rv = -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);
+    rv = qemuAgentSetTime(agent, seconds, nseconds, rtcSync);
+    qemuDomainObjExitAgent(vm, agent);
+
+ endjob:
+    qemuDomainObjEndJob(driver, vm);
+
+    return rv;
+}
+
 static int
 qemuDomainSetTime(virDomainPtr dom,
                   long long seconds,
@@ -20265,7 +20294,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;
@@ -20280,14 +20308,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 */
@@ -20297,18 +20317,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;
@@ -20327,7 +20343,7 @@ qemuDomainSetTime(virDomainPtr dom,
     ret = 0;
 
  endjob:
-    qemuDomainObjEndJobWithAgent(driver, vm);
+    qemuDomainObjEndJob(driver, vm);
 
  cleanup:
     virDomainObjEndAPI(&vm);
-- 
2.21.0

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list