Re: [libvirt] [PATCH] qemu_agent: Remove agent reference only when disposing it
On 10.01.2013 01:59, Eric Blake wrote: > On 01/09/2013 06:30 AM, Michal Privoznik wrote: >> https://bugzilla.redhat.com/show_bug.cgi?id=892079 >> >> With current code, if user calls virDomainPMSuspendForDuration() >> followed by virDomainDestroy(), the former API checks for qemu agent >> presence, which will evaluate as true (if agent is configured). While >> talking to qemu agent, the qemu driver is unlocked, so the latter API >> starts executing. However, if machine dies meanwhile, libvirtd gets >> EOF on the agent socket and qemuProcessHandleAgentEOF() is called. The >> handler clears reference to qemu agent while the destroy API already >> holding a reference to it. This leads to NULL dereferencing later in >> the code. Therefore, the agent pointer should be set to NULL only if >> we are the exclusive owner of it. >> --- >> >> There's a reproducer in the BZ. It doesn't have to be a windows guest, >> I was able to reproduce with F17 guest as well. >> >> src/qemu/qemu_process.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) > > ACK. > Thanks, pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu_agent: Remove agent reference only when disposing it
On 10.01.2013 01:59, Eric Blake wrote: > On 01/09/2013 06:30 AM, Michal Privoznik wrote: >> https://bugzilla.redhat.com/show_bug.cgi?id=892079 >> >> With current code, if user calls virDomainPMSuspendForDuration() >> followed by virDomainDestroy(), the former API checks for qemu agent >> presence, which will evaluate as true (if agent is configured). While >> talking to qemu agent, the qemu driver is unlocked, so the latter API >> starts executing. However, if machine dies meanwhile, libvirtd gets >> EOF on the agent socket and qemuProcessHandleAgentEOF() is called. The >> handler clears reference to qemu agent while the destroy API already >> holding a reference to it. This leads to NULL dereferencing later in >> the code. Therefore, the agent pointer should be set to NULL only if >> we are the exclusive owner of it. >> --- >> >> There's a reproducer in the BZ. It doesn't have to be a windows guest, >> I was able to reproduce with F17 guest as well. >> >> src/qemu/qemu_process.c | 3 ++- >> 1 file changed, 2 insertions(+), 1 deletion(-) > > ACK. > Thanks, pushed. Michal -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCH] qemu_agent: Remove agent reference only when disposing it
On 01/09/2013 06:30 AM, Michal Privoznik wrote: > https://bugzilla.redhat.com/show_bug.cgi?id=892079 > > With current code, if user calls virDomainPMSuspendForDuration() > followed by virDomainDestroy(), the former API checks for qemu agent > presence, which will evaluate as true (if agent is configured). While > talking to qemu agent, the qemu driver is unlocked, so the latter API > starts executing. However, if machine dies meanwhile, libvirtd gets > EOF on the agent socket and qemuProcessHandleAgentEOF() is called. The > handler clears reference to qemu agent while the destroy API already > holding a reference to it. This leads to NULL dereferencing later in > the code. Therefore, the agent pointer should be set to NULL only if > we are the exclusive owner of it. > --- > > There's a reproducer in the BZ. It doesn't have to be a windows guest, > I was able to reproduce with F17 guest as well. > > src/qemu/qemu_process.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) ACK. -- Eric Blake eblake redhat com+1-919-301-3266 Libvirt virtualization library http://libvirt.org signature.asc Description: OpenPGP digital signature -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
[libvirt] [PATCH] qemu_agent: Remove agent reference only when disposing it
https://bugzilla.redhat.com/show_bug.cgi?id=892079 With current code, if user calls virDomainPMSuspendForDuration() followed by virDomainDestroy(), the former API checks for qemu agent presence, which will evaluate as true (if agent is configured). While talking to qemu agent, the qemu driver is unlocked, so the latter API starts executing. However, if machine dies meanwhile, libvirtd gets EOF on the agent socket and qemuProcessHandleAgentEOF() is called. The handler clears reference to qemu agent while the destroy API already holding a reference to it. This leads to NULL dereferencing later in the code. Therefore, the agent pointer should be set to NULL only if we are the exclusive owner of it. --- There's a reproducer in the BZ. It doesn't have to be a windows guest, I was able to reproduce with F17 guest as well. src/qemu/qemu_process.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index 938c17e..320c0c6 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -133,7 +133,8 @@ qemuProcessHandleAgentEOF(qemuAgentPtr agent, virDomainObjLock(vm); priv = vm->privateData; -if (priv->agent == agent) +if (priv->agent == agent && +!virObjectUnref(priv->agent)) priv->agent = NULL; virDomainObjUnlock(vm); -- 1.8.0.2 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list