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@redhat.com>
> ---
> 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@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list