10:08 a.m.
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.
So split the function up a bit to only hold the monitor job while
querying qemu for whether the domain supports suspend. Then acquire only
an agent job while issuing the agent suspend command.
Signed-off-by: Jonathon Jongsma <jjongsma(a)redhat.com>
---
src/qemu/qemu_driver.c | 93 ++++++++++++++++++++++++++----------------
1 file changed, 58 insertions(+), 35 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index edd36f4a89..e39ee2acc9 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19713,6 +19713,58 @@ qemuDomainProbeQMPCurrentMachine(virQEMUDriverPtr driver,
}
+/* returns -1 on error, or if query is not supported, 0 if query was successful */
+static int
+qemuDomainQueryWakeupSuspendSupport(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ bool *wakeupSupported)
+{
+ int ret = -1;
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+
+ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_CURRENT_MACHINE))
+ return ret;
+
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+ return ret;
+
+ if ((ret = virDomainObjCheckActive(vm)) < 0)
+ goto endjob;
+
+ ret = qemuDomainProbeQMPCurrentMachine(driver, vm, wakeupSupported);
+
+ endjob:
+ qemuDomainObjEndJob(driver, vm);
+
+ return ret;
+}
+
+static int qemuDomainPMSuspendAgent(virQEMUDriverPtr driver,
+ virDomainObjPtr vm,
+ unsigned int target)
+{
+ qemuAgentPtr agent;
+ int ret = -1;
+
+ if (qemuDomainObjBeginAgentJob(driver, vm, QEMU_AGENT_JOB_MODIFY) < 0)
+ return -1;
+
+ if ((ret = virDomainObjCheckActive(vm)) < 0)
+ goto endjob;
+
+ if (!qemuDomainAgentAvailable(vm, true))
+ goto endjob;
+
+ agent = qemuDomainObjEnterAgent(vm);
+ ret = qemuAgentSuspend(agent, target);
+ qemuDomainObjExitAgent(vm, agent);
+
+ endjob:
+ qemuDomainObjEndAgentJob(vm);
+
+ return ret;
+}
+
static int
qemuDomainPMSuspendForDuration(virDomainPtr dom,
unsigned int target,
@@ -19720,11 +19772,9 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom,
unsigned int flags)
{
virQEMUDriverPtr driver = dom->conn->privateData;
- qemuDomainObjPrivatePtr priv;
virDomainObjPtr vm;
- qemuAgentPtr agent;
- qemuDomainJob job = QEMU_JOB_NONE;
int ret = -1;
+ bool wakeupSupported;
virCheckFlags(0, -1);
@@ -19749,17 +19799,6 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom,
if (virDomainPMSuspendForDurationEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
- priv = vm->privateData;
-
- if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_CURRENT_MACHINE))
- job = QEMU_JOB_MODIFY;
-
- if (qemuDomainObjBeginJobWithAgent(driver, vm, job, QEMU_AGENT_JOB_MODIFY) < 0)
- goto cleanup;
-
- if (virDomainObjCheckActive(vm) < 0)
- goto endjob;
-
/*
* The case we want to handle here is when QEMU has the API (i.e.
* QEMU_CAPS_QUERY_CURRENT_MACHINE is set). Otherwise, do not interfere
@@ -19767,16 +19806,11 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom,
* that don't know about this cap, will keep their old behavior of
* suspending 'in the dark'.
*/
- if (virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_CURRENT_MACHINE)) {
- bool wakeupSupported;
-
- if (qemuDomainProbeQMPCurrentMachine(driver, vm, &wakeupSupported) < 0)
- goto endjob;
-
+ if (qemuDomainQueryWakeupSuspendSupport(driver, vm, &wakeupSupported) == 0) {
if (!wakeupSupported) {
virReportError(VIR_ERR_OPERATION_UNSUPPORTED, "%s",
_("Domain does not have suspend support"));
- goto endjob;
+ goto cleanup;
}
}
@@ -19786,29 +19820,18 @@ qemuDomainPMSuspendForDuration(virDomainPtr dom,
target == VIR_NODE_SUSPEND_TARGET_HYBRID)) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("S3 state is disabled for this domain"));
- goto endjob;
+ goto cleanup;
}
if (vm->def->pm.s4 == VIR_TRISTATE_BOOL_NO &&
target == VIR_NODE_SUSPEND_TARGET_DISK) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
_("S4 state is disabled for this domain"));
- goto endjob;
+ goto cleanup;
}
}
- if (!qemuDomainAgentAvailable(vm, true))
- goto endjob;
-
- agent = qemuDomainObjEnterAgent(vm);
- ret = qemuAgentSuspend(agent, target);
- qemuDomainObjExitAgent(vm, agent);
-
- endjob:
- if (job)
- qemuDomainObjEndJobWithAgent(driver, vm);
- else
- qemuDomainObjEndAgentJob(vm);
+ ret = qemuDomainPMSuspendAgent(driver, vm, target);
cleanup:
virDomainObjEndAPI(&vm);
--
2.21.0