On 02/08/2017 09:32 PM, Wang King wrote:

If there is a process with a client which registers event callbacks, and it calls libvirt's API which uses the same virConnectPtr in that callback function. When this process exit abnormally lead to client disconnect, there is a possibility that the main thread is refer to virServerClient just after the virServerClient been freed by job thread of libvirtd.

Following is the backtrace: #0 0x00007fda223d66d8 in virClassIsDerivedFrom (klass=0xdeadbeef,parent=0x7fda24c81b40) #1 0x00007fda223d6a1e in virObjectIsClass (anyobj=anyobj@entry=0x7fd9e575b400,klass=<optimized out>) #2 0x00007fda223d6a44 in virObjectLock (anyobj=anyobj@entry=0x7fd9e575b400) #3 0x00007fda22507f71 in virNetServerClientSendMessage (client=client@entry=0x7fd9e575b400, msg=msg@entry=0x7fd9ec30de90) #4 0x00007fda230d714d in remoteDispatchObjectEventSend (client=0x7fd9e575b400, program=0x7fda24c844e0, procnr=procnr@entry=348, proc=0x7fda2310e5e0 <xdr_remote_domain_event_callback_tunable_msg>, data=data@entry=0x7ffc3857fdb0) #5 0x00007fda230dd71b in remoteRelayDomainEventTunable (conn=<optimized out>, dom=0x7fda27cd7660, params=0x7fda27f3aae0, nparams=1, opaque=0x7fd9e6c99e00) #6 0x00007fda224484cb in virDomainEventDispatchDefaultFunc (conn=0x7fda27cd0120, event=0x7fda2736ea00, cb=0x7fda230dd610 <remoteRelayDomainEventTunable>, cbopaque=0x7fd9e6c99e00) #7 0x00007fda22446871 in virObjectEventStateDispatchCallbacks (callbacks=<optimized out>, callbacks=<optimized out>, event=0x7fda2736ea00, state=0x7fda24ca3960) #8 virObjectEventStateQueueDispatch (callbacks=0x7fda24c65800, queue=0x7ffc3857fe90, state=0x7fda24ca3960) #9 virObjectEventStateFlush (state=0x7fda24ca3960) #10 virObjectEventTimer (timer=<optimized out>, opaque=0x7fda24ca3960) #11 0x00007fda223ae8b9 in virEventPollDispatchTimeouts () #12 virEventPollRunOnce () #13 0x00007fda223ad1d2 in virEventRunDefaultImpl () #14 0x00007fda225046cd in virNetDaemonRun (dmn=dmn@entry=0x7fda24c775c0) #15 0x00007fda230d6351 in main (argc=<optimized out>, argv=<optimized out>)

(gdb) p *(virNetServerClientPtr)0x7fd9e575b400 $2 = {parent = {parent = {u = {dummy_align1 = 140573849338048, dummy_align2 = 0x7fd9e65ac0c0, s = {magic = 3864707264, refs = 32729}}, klass = 0x7fda00000078}, lock = {lock = {__data = {__lock = 0, __count = 0, __owner = 0, __nusers = 0, __kind = 0, __spins = 0, __list = {__prev = 0x0, __next = 0x0}}, __size = '\000' <repeats 39 times>, __align = 0}}}, wantClose = false, delayedClose = false, sock = 0x0, auth = 0, readonly = false, tlsCtxt = 0x0, tls = 0x0, sasl = 0x0, sockTimer = 0, identity = 0x0, nrequests = 0, nrequests_max = 0, rx = 0x0, tx = 0x0, filters = 0x0, nextFilterID = 0, dispatchFunc = 0x0, dispatchOpaque = 0x0, privateData = 0x0, privateDataFreeFunc = 0x0, privateDataPreExecRestart = 0x0, privateDataCloseFunc = 0x0, keepalive = 0x0} --- src/rpc/virnetserverclient.c | 6 ++++++ 1 file changed, 6 insertions(+)

When you decided to post the same patch using a different header, could you please also put a self NACK on the original patch. When I have time to look through unreviewed patches I don't want to wade through duplicates or ones that are replaced already. This is a duplicate of : http://www.redhat.com/archives/libvir-list/2017-February/msg00452.html Tks - John

diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 81da82c..562516f 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -1021,6 +1021,12 @@ void virNetServerClientClose(virNetServerClientPtr client) client->sock = NULL; }

+ if (client->privateData && + client->privateDataFreeFunc) { + client->privateDataFreeFunc(client->privateData); + client->privateData = NULL; + } + virObjectUnlock(client); }