[libvirt] [PATCH 5/8] qemu: don't hold monitor job for fsinfo

[Michal Privoznik]

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 at 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