[PATCH v2 0/8] Don't hold both monitor and agent jobs at the same time

[Michal Privoznik]

On 1/11/20 12:32 AM, 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 and an agent job while we're querying the agent, any other threads will be
> blocked from using the monitor while the agent is unresponsive. Because libvirt
> waits forever for an agent response, this makes us vulnerable to a denial of
> service from a malicious (or simply buggy) guest agent.
> 
> Most of the patches in the first series were already reviewed and pushed, but a
> couple remain: the filesystem info functions. The problem with these functions
> is that the agent functions access the vm definition (owned by the domain). If
> a monitor job is not held while this is done, the vm definition could change
> while we are looking up the disk alias, leading to a potentional crash.
> 
> This series tries to fix this by moving the disk alias searching up a level
> from qemu_agent.c to qemu_driver.c. The code in qemu_agent.c will only return
> the raw data returned from the agent command response. After the agent response
> is returned and the agent job is ended, we can then look up the disk alias from
> the vm definition while the domain object is locked.
> 
> In addition, a few nearby cleanups are included in this series, notably
> changing to glib allocation API in a couple of places.
> 
> Jonathon Jongsma (8):
>    qemu: rename qemuAgentGetFSInfoInternalDisk()
>    qemu: store complete agent filesystem information
>    qemu: Don't store disk alias in qemuAgentDiskInfo
>    qemu: don't access vmdef within qemu_agent.c
>    qemu: remove qemuDomainObjBegin/EndJobWithAgent()
>    qemu: use glib alloc in qemuAgentGetFSInfoFillDisks()
>    qemu: use glib allocation apis for qemuAgentFSInfo
>    Use glib alloc API for virDomainFSInfo
> 
>   src/libvirt-domain.c                |  12 +-
>   src/qemu/THREADS.txt                |  58 +-----
>   src/qemu/qemu_agent.c               | 268 ++++------------------------
>   src/qemu/qemu_agent.h               |  33 +++-
>   src/qemu/qemu_domain.c              |  56 +-----
>   src/qemu/qemu_domain.h              |   7 -
>   src/qemu/qemu_driver.c              | 246 +++++++++++++++++++++++--
>   src/remote/remote_daemon_dispatch.c |   2 +-
>   tests/qemuagenttest.c               | 196 ++++----------------
>   9 files changed, 336 insertions(+), 542 deletions(-)
> 

Reviewed-by: Michal Privoznik <mprivozn at redhat.com>

and pushed. Thanks for fixing this.

Michal