On 12/5/19 5:08 PM, Jonathon Jongsma wrote:
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.
This function does not issue any monitor commands, so we can drop the
monitor job and only hold an agent job.
While this is true, the reason I've added BeginJobWithAgent() call is
that qemuAgentGetFSInfo() works with vm->def which may change beneath
our hands since we wouldn't be taking a vm job. This is potentially
dangerous and may lead to a crash (as @vm is unlocked and not guarded by
any job). What we need to do is to create a copy of vm->def and pass
that to qemuAgentGetFSInfo(). However, creating a copy of domain
definition is very expensive - esp. when the agent monitor function
needs only a list of disk targets. So we might construct the list
beforehand and pass that to the function. Then taking only agent job is
going to be okay.
Signed-off-by: Jonathon Jongsma <jjongsma@xxxxxxxxxx>
---
src/qemu/qemu_driver.c | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 10fad8d75d..e1a91c5049 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -21623,9 +21623,8 @@ qemuDomainGetFSInfo(virDomainPtr dom,
if (virDomainGetFSInfoEnsureACL(dom->conn, vm->def) < 0)
goto cleanup;
- if (qemuDomainObjBeginJobWithAgent(driver, vm,
- QEMU_JOB_QUERY,
- QEMU_AGENT_JOB_QUERY) < 0)
+ if (qemuDomainObjBeginAgentJob(driver, vm,
+ QEMU_AGENT_JOB_QUERY) < 0)
goto cleanup;
if (virDomainObjCheckActive(vm) < 0)
@@ -21639,7 +21638,7 @@ qemuDomainGetFSInfo(virDomainPtr dom,
qemuDomainObjExitAgent(vm, agent);
endjob:
- qemuDomainObjEndJobWithAgent(driver, vm);
+ qemuDomainObjEndAgentJob(vm);
cleanup:
virDomainObjEndAPI(&vm);
I won't push this one, sorry.
Michal
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list
