1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
|
From a663a860819287e041c3de672aad1d8543098ecc Mon Sep 17 00:00:00 2001
From: Jonathon Jongsma <jjongsma@redhat.com>
Date: Thu, 5 Dec 2019 10:08:52 -0600
Subject: [PATCH] qemu: don't hold both jobs for suspend
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@redhat.com>
Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Michal Privoznik <mprivozn@redhat.com>
---
src/qemu/qemu_driver.c | 94 ++++++++++++++++++++++++++++++------------------
1 files changed, 59 insertions(+), 35 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 2891faf..52cf27f 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19759,6 +19759,59 @@ 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)
+{
+ qemuDomainObjPrivatePtr priv = vm->privateData;
+ int ret = -1;
+
+ if (!virQEMUCapsGet(priv->qemuCaps, QEMU_CAPS_QUERY_CURRENT_MACHINE))
+ return -1;
+
+ if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_MODIFY) < 0)
+ return -1;
+
+ 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,
@@ -19766,11 +19819,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);
@@ -19795,17 +19846,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
@@ -19813,16 +19853,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;
}
}
@@ -19832,29 +19867,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);
--
1.7.1
|
