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

[Michal Privoznik]

On 23.11.2016 08:08, Nikolay Shirokovskiy wrote:
> 
> 
> On 22.11.2016 17:49, Michal Privoznik wrote:
>> 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?
> 
> qemuProcessHandleAgentEOF for example can zero out priv->agent after 
> we call qemuDomainObjEnterAgent and before we call qemuAgentGetTime(priv->agent, seconds, nseconds).
> Because we drop domain lock in qemuDomainObjEnterAgent and 
> qemuProcessHandleAgentEOF does not acquire job condition, only domain lock. 
> At the same time qemuAgentGetTime is not ready for NULL agent and will crash.
> It could get even worse as priv->agent is not atomic and instead of
> NULL we can get garbage there. Long story short we just should
> not access domain state without lock. Thats why I change all the places
> so we get copy of priv->agent before we drop domain lock.

Ah. Sounds reasonable. Mind adding it to the commit message?

ACK then.

Michal