[libvirt] [PATCH 2/2] qemu: agent: fix unsafe agent access

[Michal Privoznik]

On 14.11.2016 15:24, Nikolay Shirokovskiy wrote:
> qemuDomainObjExitAgent is unsafe.
> 
> First it accesses domain object without domain lock.
> Second it uses outdated logic that goes back to commit 79533da1 of
> year 2009 when code was quite different. (unref function
> instead of unreferencing only unlocked and disposed object
> in case of last reference and leaved unlocking to the caller otherwise).
> Nowadays this logic may lead to disposing locked object
> i guess.

Well, I agree that the order of those two steps should be reversed, so
ACK to that.

> 
> Another problem is that the callers of qemuDomainObjEnterAgent
> use domain object again (namely priv->agent) without domain lock.

But why is this a problem?

> 
> This patch address these two problems.
> 
> qemuDomainGetAgent is dropped as unused.
> ---
> 
> I wonder should we make qemuDomainObj(Enter|Exit)Agent be macros. So 
> we don't need to declare a variable to hold agent reference and zero
> it out at the end for safety.
> 
> Also the qemu monitor code has probably same problems as the agent monitor code
> seems to be copied from there.
> 

Well, qemu monitor can't come and go. I mean, if it does, the domain is
killed. That's not the case with agent monitor.

>  src/qemu/qemu_domain.c |  41 ++++---------
>  src/qemu/qemu_domain.h |   7 +--
>  src/qemu/qemu_driver.c | 153 ++++++++++++++++++++++++-------------------------
>  3 files changed, 91 insertions(+), 110 deletions(-)

Michal