[libvirt] [PATCH] do not unref obj in qemuDomainObjExitMonitorWithDriver

[Laine Stump]

On 03/17/2011 10:25 PM, Wen Congyang wrote:
> At 03/17/2011 07:11 PM, Daniel P. Berrange Write:
>> On Wed, Mar 16, 2011 at 05:01:23PM +0800, Wen Congyang wrote:
>>> Steps to reproduce this bug:
>>> # cat test.sh
>>>    #! /bin/bash -x
>>>    virsh start domain
>>>    sleep 5
>>>    virsh qemu-monitor-command domain 'cpu_set 2 online' --hmp
>>> # while true; do ./test.sh ; done
>>>
>>> Then libvirtd will crash.
>>>
>>> The reason is that:
>>> we add a reference of obj when we open the monitor. We will reduce this
>>> reference when we free the monitor.
>>>
>>> If the reference of monitor is 0, we will free monitor automatically and
>>> the reference of obj is reduced.
>>>
>>> But in the function qemuDomainObjExitMonitorWithDriver(), we reduce this
>>> reference again when the reference of monitor is 0.
>>>
>>> It will cause the obj be freed in the function qemuDomainObjEndJob().
>>>
>>> Then we start the domain again, and libvirtd will crash in the function
>>> virDomainObjListSearchName(), because we pass a null pointer(obj->def->name)
>>> to strcmp().
>>>
>>> Signed-off-by: Wen Congyang<wency at cn.fujitsu.com>
>>>
>>> ---
>>>   src/qemu/qemu_domain.c |    1 -
>>>   1 files changed, 0 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
>>> index 8a2b9cc..ae28b1c 100644
>>> --- a/src/qemu/qemu_domain.c
>>> +++ b/src/qemu/qemu_domain.c
>>> @@ -634,7 +634,6 @@ void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver,
>>>       virDomainObjLock(obj);
>>>
>>>       if (refs == 0) {
>>> -        virDomainObjUnref(obj);
>>>           priv->mon = NULL;
>>>       }
>>>   }
>> ACK,  ExitMonitorWithDriver should not be touching the virDomainObjPtr
>> refs at all. The virDomainObjPtr refs should only be touched by the
>> BeginJob/EndJob calls.
>>
>> This same fix also needs to be done in qemuDomainObjExitMonitor()
> Yes, there is the same problem in qemuDomainObjExitMonitor().
> I have updated this patch.


ACK to this V2 (partially based on Dan's approval). I just pushed it.


>>
>> Daniel
>
> > From 6c30534f7e6c5d2e5661c38397fc5062a8288ef3 Mon Sep 17 00:00:00 2001
> From: Wen Congyang<wency at cn.fujitsu.com>
> Date: Fri, 18 Mar 2011 09:51:07 +0800
> Subject: [PATCH 3/3] do not unref obj in qemuDomainObjExitMonitor*
>
> ---
>   src/qemu/qemu_domain.c |    2 --
>   1 files changed, 0 insertions(+), 2 deletions(-)
>
> diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c
> index 8a2b9cc..cc137d2 100644
> --- a/src/qemu/qemu_domain.c
> +++ b/src/qemu/qemu_domain.c
> @@ -588,7 +588,6 @@ void qemuDomainObjExitMonitor(virDomainObjPtr obj)
>       virDomainObjLock(obj);
>
>       if (refs == 0) {
> -        virDomainObjUnref(obj);
>           priv->mon = NULL;
>       }
>   }
> @@ -634,7 +633,6 @@ void qemuDomainObjExitMonitorWithDriver(struct qemud_driver *driver,
>       virDomainObjLock(obj);
>
>       if (refs == 0) {
> -        virDomainObjUnref(obj);
>           priv->mon = NULL;
>       }
>   }