[libvirt] [PATCH v2 0/2] Fix possible use-after-free when sending event message
v1: https://www.redhat.com/archives/libvir-list/2017-March/msg01228.html Change since v1, add the derefFcn as an argument to the renamed macro (not quite sure how I missed that originally. John Ferlan (2): daemon: Rework remoteClientFreeFunc cleanup loops into C macro remote: Fix possible use-after-free when sending event message daemon/remote.c | 164 ++++++++++++++++++++------------------------------------ 1 file changed, 58 insertions(+), 106 deletions(-) -- 2.9.3
Rather than 'n' repetitive code segments, let's create a single macro which will make the code easier to read. Signed-off-by: John Ferlan <jferlan@redhat.com> --- daemon/remote.c | 126 +++++++++++++++++--------------------------------------- 1 file changed, 37 insertions(+), 89 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 5696b43..5cdc53e 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -1668,6 +1668,24 @@ void remoteRelayConnectionClosedEvent(virConnectPtr conn ATTRIBUTE_UNUSED, int r &msg); } +#define DEREG_CB(conn, eventCallbacks, neventCallbacks, deregFcn, name) \ + do { \ + size_t i; \ + for (i = 0; i < neventCallbacks; i++) { \ + int callbackID = eventCallbacks[i]->callbackID; \ + if (callbackID < 0) { \ + VIR_WARN("unexpected incomplete %s callback %zu", name, i); \ + continue; \ + } \ + VIR_DEBUG("Deregistering remote %s event relay %d", \ + name, callbackID); \ + eventCallbacks[i]->callbackID = -1; \ + if (deregFcn(conn, callbackID) < 0) \ + VIR_WARN("unexpected %s event deregister failure", name); \ + } \ + VIR_FREE(eventCallbacks); \ + } while (0); + /* * You must hold lock for at least the client * We don't free stuff here, merely disconnect the client's @@ -1682,98 +1700,27 @@ void remoteClientFreeFunc(void *data) /* Deregister event delivery callback */ if (priv->conn) { virIdentityPtr sysident = virIdentityGetSystem(); - size_t i; virIdentitySetCurrent(sysident); - for (i = 0; i < priv->ndomainEventCallbacks; i++) { - int callbackID = priv->domainEventCallbacks[i]->callbackID; - if (callbackID < 0) { - VIR_WARN("unexpected incomplete domain callback %zu", i); - continue; - } - VIR_DEBUG("Deregistering remote domain event relay %d", - callbackID); - priv->domainEventCallbacks[i]->callbackID = -1; - if (virConnectDomainEventDeregisterAny(priv->conn, callbackID) < 0) - VIR_WARN("unexpected domain event deregister failure"); - } - VIR_FREE(priv->domainEventCallbacks); - - for (i = 0; i < priv->nnetworkEventCallbacks; i++) { - int callbackID = priv->networkEventCallbacks[i]->callbackID; - if (callbackID < 0) { - VIR_WARN("unexpected incomplete network callback %zu", i); - continue; - } - VIR_DEBUG("Deregistering remote network event relay %d", - callbackID); - priv->networkEventCallbacks[i]->callbackID = -1; - if (virConnectNetworkEventDeregisterAny(priv->conn, - callbackID) < 0) - VIR_WARN("unexpected network event deregister failure"); - } - VIR_FREE(priv->networkEventCallbacks); - - for (i = 0; i < priv->nstorageEventCallbacks; i++) { - int callbackID = priv->storageEventCallbacks[i]->callbackID; - if (callbackID < 0) { - VIR_WARN("unexpected incomplete storage pool callback %zu", i); - continue; - } - VIR_DEBUG("Deregistering remote storage pool event relay %d", - callbackID); - priv->storageEventCallbacks[i]->callbackID = -1; - if (virConnectStoragePoolEventDeregisterAny(priv->conn, - callbackID) < 0) - VIR_WARN("unexpected storage pool event deregister failure"); - } - VIR_FREE(priv->storageEventCallbacks); - - for (i = 0; i < priv->nnodeDeviceEventCallbacks; i++) { - int callbackID = priv->nodeDeviceEventCallbacks[i]->callbackID; - if (callbackID < 0) { - VIR_WARN("unexpected incomplete node device callback %zu", i); - continue; - } - VIR_DEBUG("Deregistering remote node device event relay %d", - callbackID); - priv->nodeDeviceEventCallbacks[i]->callbackID = -1; - if (virConnectNodeDeviceEventDeregisterAny(priv->conn, - callbackID) < 0) - VIR_WARN("unexpected node device event deregister failure"); - } - VIR_FREE(priv->nodeDeviceEventCallbacks); - - for (i = 0; i < priv->nsecretEventCallbacks; i++) { - int callbackID = priv->secretEventCallbacks[i]->callbackID; - if (callbackID < 0) { - VIR_WARN("unexpected incomplete secret callback %zu", i); - continue; - } - VIR_DEBUG("Deregistering remote secret event relay %d", - callbackID); - priv->secretEventCallbacks[i]->callbackID = -1; - if (virConnectSecretEventDeregisterAny(priv->conn, - callbackID) < 0) - VIR_WARN("unexpected secret event deregister failure"); - } - VIR_FREE(priv->secretEventCallbacks); - - for (i = 0; i < priv->nqemuEventCallbacks; i++) { - int callbackID = priv->qemuEventCallbacks[i]->callbackID; - if (callbackID < 0) { - VIR_WARN("unexpected incomplete qemu monitor callback %zu", i); - continue; - } - VIR_DEBUG("Deregistering remote qemu monitor event relay %d", - callbackID); - priv->qemuEventCallbacks[i]->callbackID = -1; - if (virConnectDomainQemuMonitorEventDeregister(priv->conn, - callbackID) < 0) - VIR_WARN("unexpected qemu monitor event deregister failure"); - } - VIR_FREE(priv->qemuEventCallbacks); + DEREG_CB(priv->conn, priv->domainEventCallbacks, + priv->ndomainEventCallbacks, + virConnectDomainEventDeregisterAny, "domain"); + DEREG_CB(priv->conn, priv->networkEventCallbacks, + priv->nnetworkEventCallbacks, + virConnectNetworkEventDeregisterAny, "network"); + DEREG_CB(priv->conn, priv->storageEventCallbacks, + priv->nstorageEventCallbacks, + virConnectStoragePoolEventDeregisterAny, "storage"); + DEREG_CB(priv->conn, priv->nodeDeviceEventCallbacks, + priv->nnodeDeviceEventCallbacks, + virConnectNodeDeviceEventDeregisterAny, "node device"); + DEREG_CB(priv->conn, priv->secretEventCallbacks, + priv->nsecretEventCallbacks, + virConnectSecretEventDeregisterAny, "secret"); + DEREG_CB(priv->conn, priv->qemuEventCallbacks, + priv->nqemuEventCallbacks, + virConnectDomainQemuMonitorEventDeregister, "qemu monitor"); if (priv->closeRegistered) { if (virConnectUnregisterCloseCallback(priv->conn, @@ -1789,6 +1736,7 @@ void remoteClientFreeFunc(void *data) VIR_FREE(priv); } +#undef DEREG_CB static void remoteClientCloseFunc(virNetServerClientPtr client) -- 2.9.3
On 03/27/2017 12:47 PM, John Ferlan wrote:
Rather than 'n' repetitive code segments, let's create a single macro which will make the code easier to read.
Signed-off-by: John Ferlan <jferlan@redhat.com> --- daemon/remote.c | 126 +++++++++++++++++--------------------------------------- 1 file changed, 37 insertions(+), 89 deletions(-)
ACK.
Based upon an idea and some research by Wang King <king.wang@huawei.com> and xinhua.Cao <caoxinhua@huawei.com>. Since we're assigning the 'client' to our callback event lookaside list, it's imperative that we grab a reference to the object; otherwise, when the object is unref'd during virNetServerProcessClients when it's determined that the virNetServerClientIsClosed and the memory is free'd before perhaps the object event state callbacks are run. When a virObjectLock() is run, before sending the message the following trace occurs; #0 0x00007fda223d66d8 in virClassIsDerivedFrom (klass=0xdeadbeef, parent=0x7fda24c81b40) at util/virobject.c:169 #1 0x00007fda223d6a1e in virObjectIsClass (anyobj=anyobj@entry=0x7fd9e575b400, klass=<optimized out>) at util/virobject.c:365 #2 0x00007fda223d6a44 in virObjectLock (anyobj=0x7fd9e575b400) at util/virobject.c:317 #3 0x00007fda22507f71 in virNetServerClientSendMessage (client=client@entry=0x7fd9e575b400, msg=msg@entry=0x7fd9ec30de90) at rpc/virnetserverclient.c:1422 #4 0x00007fda230d714d in remoteDispatchObjectEventSend (client=0x7fd9e575b400, program=0x7fda24c844e0, procnr=348, proc=0x7fda2310e5e0 <xdr_remote_domain_event_callback_tunable_msg>, data=0x7ffc3857fdb0) at remote.c:3803 #5 0x00007fda230dd71b in remoteRelayDomainEventTunable (conn=<optimized out>, dom=0x7fda27cd7660, params=0x7fda27f3aae0, nparams=1,opaque=0x7fd9e6c99e00) at remote.c:1033 #6 0x00007fda224484cb in virDomainEventDispatchDefaultFunc (conn=0x7fda27cd0120, event=0x7fda2736ea00, cb=0x7fda230dd610 <remoteRelayDomainEventTunable>, cbopaque=0x7fd9e6c99e00) at conf/domain_event.c:1910 #7 0x00007fda22446871 in virObjectEventStateDispatchCallbacks (callbacks=<optimized out>, callbacks=<optimized out>, event=0x7fda2736ea00,state=0x7fda24ca3960) at conf/object_event.c:722 #8 virObjectEventStateQueueDispatch (callbacks=0x7fda24c65800, queue=0x7ffc3857fe90, state=0x7fda24ca3960) at conf/object_event.c:736 #9 virObjectEventStateFlush (state=0x7fda24ca3960) at conf/object_event.c:814 #10 virObjectEventTimer (timer=<optimized out>, opaque=0x7fda24ca3960) at conf/object_event.c:560 #11 0x00007fda223ae8b9 in virEventPollDispatchTimeouts () at util/vireventpoll.c:458 #12 virEventPollRunOnce () at util/vireventpoll.c:654 #13 0x00007fda223ad1d2 in virEventRunDefaultImpl () at util/virevent.c:314 #14 0x00007fda225046cd in virNetDaemonRun (dmn=0x7fda24c775c0) at rpc/virnetdaemon.c:818 #15 0x00007fda230d6351 in main (argc=<optimized out>, argv=<optimized out>) at libvirtd.c:1623 Signed-off-by: John Ferlan <jferlan@redhat.com> --- daemon/remote.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-) diff --git a/daemon/remote.c b/daemon/remote.c index 5cdc53e..25a29cf 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -124,7 +124,11 @@ remoteDispatchObjectEventSend(virNetServerClientPtr client, static void remoteEventCallbackFree(void *opaque) { - VIR_FREE(opaque); + daemonClientEventCallbackPtr callback = opaque; + if (!callback) + return; + virObjectUnref(callback->client); + VIR_FREE(callback); } @@ -3896,7 +3900,7 @@ remoteDispatchConnectDomainEventRegister(virNetServerPtr server ATTRIBUTE_UNUSED */ if (VIR_ALLOC(callback) < 0) goto cleanup; - callback->client = client; + callback->client = virObjectRef(client); callback->eventID = VIR_DOMAIN_EVENT_ID_LIFECYCLE; callback->callbackID = -1; callback->legacy = true; @@ -3923,7 +3927,7 @@ remoteDispatchConnectDomainEventRegister(virNetServerPtr server ATTRIBUTE_UNUSED rv = 0; cleanup: - VIR_FREE(callback); + remoteEventCallbackFree(callback); if (rv < 0) virNetMessageSaveError(rerr); virMutexUnlock(&priv->lock); @@ -4131,7 +4135,7 @@ remoteDispatchConnectDomainEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNU * success, we use 'ref' to save a copy of the pointer. */ if (VIR_ALLOC(callback) < 0) goto cleanup; - callback->client = client; + callback->client = virObjectRef(client); callback->eventID = args->eventID; callback->callbackID = -1; callback->legacy = true; @@ -4158,7 +4162,7 @@ remoteDispatchConnectDomainEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNU rv = 0; cleanup: - VIR_FREE(callback); + remoteEventCallbackFree(callback); if (rv < 0) virNetMessageSaveError(rerr); virMutexUnlock(&priv->lock); @@ -4207,7 +4211,7 @@ remoteDispatchConnectDomainEventCallbackRegisterAny(virNetServerPtr server ATTRI * success, we use 'ref' to save a copy of the pointer. */ if (VIR_ALLOC(callback) < 0) goto cleanup; - callback->client = client; + callback->client = virObjectRef(client); callback->eventID = args->eventID; callback->callbackID = -1; ref = callback; @@ -4234,7 +4238,7 @@ remoteDispatchConnectDomainEventCallbackRegisterAny(virNetServerPtr server ATTRI rv = 0; cleanup: - VIR_FREE(callback); + remoteEventCallbackFree(callback); if (rv < 0) virNetMessageSaveError(rerr); virObjectUnref(dom); @@ -5717,7 +5721,7 @@ remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server ATTRIBUTE_UN * success, we use 'ref' to save a copy of the pointer. */ if (VIR_ALLOC(callback) < 0) goto cleanup; - callback->client = client; + callback->client = virObjectRef(client); callback->eventID = args->eventID; callback->callbackID = -1; ref = callback; @@ -5744,7 +5748,7 @@ remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server ATTRIBUTE_UN rv = 0; cleanup: - VIR_FREE(callback); + remoteEventCallbackFree(callback); if (rv < 0) virNetMessageSaveError(rerr); virObjectUnref(net); @@ -5839,7 +5843,7 @@ remoteDispatchConnectStoragePoolEventRegisterAny(virNetServerPtr server ATTRIBUT * success, we use 'ref' to save a copy of the pointer. */ if (VIR_ALLOC(callback) < 0) goto cleanup; - callback->client = client; + callback->client = virObjectRef(client); callback->eventID = args->eventID; callback->callbackID = -1; ref = callback; @@ -5866,7 +5870,7 @@ remoteDispatchConnectStoragePoolEventRegisterAny(virNetServerPtr server ATTRIBUT rv = 0; cleanup: - VIR_FREE(callback); + remoteEventCallbackFree(callback); if (rv < 0) virNetMessageSaveError(rerr); virObjectUnref(pool); @@ -5960,7 +5964,7 @@ remoteDispatchConnectNodeDeviceEventRegisterAny(virNetServerPtr server ATTRIBUTE * success, we use 'ref' to save a copy of the pointer. */ if (VIR_ALLOC(callback) < 0) goto cleanup; - callback->client = client; + callback->client = virObjectRef(client); callback->eventID = args->eventID; callback->callbackID = -1; ref = callback; @@ -5987,7 +5991,7 @@ remoteDispatchConnectNodeDeviceEventRegisterAny(virNetServerPtr server ATTRIBUTE rv = 0; cleanup: - VIR_FREE(callback); + remoteEventCallbackFree(callback); if (rv < 0) virNetMessageSaveError(rerr); virObjectUnref(dev); @@ -6081,7 +6085,7 @@ remoteDispatchConnectSecretEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNU * success, we use 'ref' to save a copy of the pointer. */ if (VIR_ALLOC(callback) < 0) goto cleanup; - callback->client = client; + callback->client = virObjectRef(client); callback->eventID = args->eventID; callback->callbackID = -1; ref = callback; @@ -6108,7 +6112,7 @@ remoteDispatchConnectSecretEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNU rv = 0; cleanup: - VIR_FREE(callback); + remoteEventCallbackFree(callback); if (rv < 0) virNetMessageSaveError(rerr); virObjectUnref(secret); @@ -6197,7 +6201,7 @@ qemuDispatchConnectDomainMonitorEventRegister(virNetServerPtr server ATTRIBUTE_U * success, we use 'ref' to save a copy of the pointer. */ if (VIR_ALLOC(callback) < 0) goto cleanup; - callback->client = client; + callback->client = virObjectRef(client); callback->callbackID = -1; ref = callback; if (VIR_APPEND_ELEMENT(priv->qemuEventCallbacks, @@ -6224,7 +6228,7 @@ qemuDispatchConnectDomainMonitorEventRegister(virNetServerPtr server ATTRIBUTE_U rv = 0; cleanup: - VIR_FREE(callback); + remoteEventCallbackFree(callback); if (rv < 0) virNetMessageSaveError(rerr); virObjectUnref(dom); -- 2.9.3
On 03/27/2017 06:47 PM, John Ferlan wrote:
Based upon an idea and some research by Wang King <king.wang@huawei.com> and xinhua.Cao <caoxinhua@huawei.com>.
Since we're assigning the 'client' to our callback event lookaside list, it's imperative that we grab a reference to the object; otherwise, when the object is unref'd during virNetServerProcessClients when it's determined that the virNetServerClientIsClosed and the memory is free'd before perhaps the object event state callbacks are run. When a virObjectLock() is run, before sending the message the following trace occurs;
#0 0x00007fda223d66d8 in virClassIsDerivedFrom (klass=0xdeadbeef, parent=0x7fda24c81b40) at util/virobject.c:169 #1 0x00007fda223d6a1e in virObjectIsClass (anyobj=anyobj@entry=0x7fd9e575b400, klass=<optimized out>) at util/virobject.c:365 #2 0x00007fda223d6a44 in virObjectLock (anyobj=0x7fd9e575b400) at util/virobject.c:317 #3 0x00007fda22507f71 in virNetServerClientSendMessage (client=client@entry=0x7fd9e575b400, msg=msg@entry=0x7fd9ec30de90) at rpc/virnetserverclient.c:1422 #4 0x00007fda230d714d in remoteDispatchObjectEventSend (client=0x7fd9e575b400, program=0x7fda24c844e0, procnr=348, proc=0x7fda2310e5e0 <xdr_remote_domain_event_callback_tunable_msg>, data=0x7ffc3857fdb0) at remote.c:3803 #5 0x00007fda230dd71b in remoteRelayDomainEventTunable (conn=<optimized out>, dom=0x7fda27cd7660, params=0x7fda27f3aae0, nparams=1,opaque=0x7fd9e6c99e00) at remote.c:1033 #6 0x00007fda224484cb in virDomainEventDispatchDefaultFunc (conn=0x7fda27cd0120, event=0x7fda2736ea00, cb=0x7fda230dd610 <remoteRelayDomainEventTunable>, cbopaque=0x7fd9e6c99e00) at conf/domain_event.c:1910 #7 0x00007fda22446871 in virObjectEventStateDispatchCallbacks (callbacks=<optimized out>, callbacks=<optimized out>, event=0x7fda2736ea00,state=0x7fda24ca3960) at conf/object_event.c:722 #8 virObjectEventStateQueueDispatch (callbacks=0x7fda24c65800, queue=0x7ffc3857fe90, state=0x7fda24ca3960) at conf/object_event.c:736 #9 virObjectEventStateFlush (state=0x7fda24ca3960) at conf/object_event.c:814 #10 virObjectEventTimer (timer=<optimized out>, opaque=0x7fda24ca3960) at conf/object_event.c:560 #11 0x00007fda223ae8b9 in virEventPollDispatchTimeouts () at util/vireventpoll.c:458 #12 virEventPollRunOnce () at util/vireventpoll.c:654 #13 0x00007fda223ad1d2 in virEventRunDefaultImpl () at util/virevent.c:314 #14 0x00007fda225046cd in virNetDaemonRun (dmn=0x7fda24c775c0) at rpc/virnetdaemon.c:818 #15 0x00007fda230d6351 in main (argc=<optimized out>, argv=<optimized out>) at libvirtd.c:1623
Signed-off-by: John Ferlan <jferlan@redhat.com> --- daemon/remote.c | 38 +++++++++++++++++++++----------------- 1 file changed, 21 insertions(+), 17 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c index 5cdc53e..25a29cf 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -124,7 +124,11 @@ remoteDispatchObjectEventSend(virNetServerClientPtr client, static void remoteEventCallbackFree(void *opaque) { - VIR_FREE(opaque); + daemonClientEventCallbackPtr callback = opaque; + if (!callback) + return; + virObjectUnref(callback->client); + VIR_FREE(callback); }
@@ -3896,7 +3900,7 @@ remoteDispatchConnectDomainEventRegister(virNetServerPtr server ATTRIBUTE_UNUSED */ if (VIR_ALLOC(callback) < 0) goto cleanup; - callback->client = client; + callback->client = virObjectRef(client); callback->eventID = VIR_DOMAIN_EVENT_ID_LIFECYCLE; callback->callbackID = -1; callback->legacy = true; @@ -3923,7 +3927,7 @@ remoteDispatchConnectDomainEventRegister(virNetServerPtr server ATTRIBUTE_UNUSED rv = 0;
cleanup: - VIR_FREE(callback); + remoteEventCallbackFree(callback); if (rv < 0) virNetMessageSaveError(rerr); virMutexUnlock(&priv->lock);
I don't really see how this could work in the first place. I mean, @callback is allocated in the chunk above. Cool. Then, in between these two chunks it's passed (under code name @ref) to virConnectDomainEventRegisterAny() ... Aaand I get it now. Just realized that VIR_APPEND_ELEMENT() sets @callback to NULL. So we are safe here. D'oh. ACK Michal
On 03/27/2017 12:47 PM, John Ferlan wrote:
v1: https://www.redhat.com/archives/libvir-list/2017-March/msg01228.html
Change since v1, add the derefFcn as an argument to the renamed macro (not quite sure how I missed that originally.
John Ferlan (2): daemon: Rework remoteClientFreeFunc cleanup loops into C macro remote: Fix possible use-after-free when sending event message
daemon/remote.c | 164 ++++++++++++++++++++------------------------------------ 1 file changed, 58 insertions(+), 106 deletions(-)
Laine took a look at patch 1/2 - anyone want to look at 2/2 which he didn't feel comfortable looking at? Essentially it follows similar logic to virObjectEventCallbackListAddID when processing virObjectRef(conn), except this time the virObjectRef is on virNetServerClientPtr client whenever the callback functions grab it's address. When the callback is free'd the reference is removed (in remoteEventCallbackFree) so that virNetServerProcessClients doesn't inadvertently free the client before the callback code is done with it (sending an event message). Tks - John
ping? Tks, - John On 04/03/2017 10:12 AM, John Ferlan wrote:
On 03/27/2017 12:47 PM, John Ferlan wrote:
v1: https://www.redhat.com/archives/libvir-list/2017-March/msg01228.html
Change since v1, add the derefFcn as an argument to the renamed macro (not quite sure how I missed that originally.
John Ferlan (2): daemon: Rework remoteClientFreeFunc cleanup loops into C macro remote: Fix possible use-after-free when sending event message
daemon/remote.c | 164 ++++++++++++++++++++------------------------------------ 1 file changed, 58 insertions(+), 106 deletions(-)
Laine took a look at patch 1/2 - anyone want to look at 2/2 which he didn't feel comfortable looking at?
Essentially it follows similar logic to virObjectEventCallbackListAddID when processing virObjectRef(conn), except this time the virObjectRef is on virNetServerClientPtr client whenever the callback functions grab it's address. When the callback is free'd the reference is removed (in remoteEventCallbackFree) so that virNetServerProcessClients doesn't inadvertently free the client before the callback code is done with it (sending an event message).
Tks -
John
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Ping? Tks John On 04/12/2017 07:58 PM, John Ferlan wrote:
ping?
Tks, -
John
On 04/03/2017 10:12 AM, John Ferlan wrote:
On 03/27/2017 12:47 PM, John Ferlan wrote:
v1: https://www.redhat.com/archives/libvir-list/2017-March/msg01228.html
Change since v1, add the derefFcn as an argument to the renamed macro (not quite sure how I missed that originally.
John Ferlan (2): daemon: Rework remoteClientFreeFunc cleanup loops into C macro remote: Fix possible use-after-free when sending event message
daemon/remote.c | 164 ++++++++++++++++++++------------------------------------ 1 file changed, 58 insertions(+), 106 deletions(-)
Laine took a look at patch 1/2 - anyone want to look at 2/2 which he didn't feel comfortable looking at?
Essentially it follows similar logic to virObjectEventCallbackListAddID when processing virObjectRef(conn), except this time the virObjectRef is on virNetServerClientPtr client whenever the callback functions grab it's address. When the callback is free'd the reference is removed (in remoteEventCallbackFree) so that virNetServerProcessClients doesn't inadvertently free the client before the callback code is done with it (sending an event message).
Tks -
John
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
participants (3)
-
John Ferlan -
Laine Stump -
Michal Privoznik