<html xmlns:v="urn:schemas-microsoft-com:vml" xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:x="urn:schemas-microsoft-com:office:excel" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40"> <head> <meta http-equiv="Content-Type" content="text/html; charset=us-ascii"> <meta name="Generator" content="Microsoft Word 12 (filtered medium)"> <style></style> </head> <body lang="ZH-CN" link="blue" vlink="purple" style="text-justify-trim:punctuation"> <div class="WordSection1"> From 6b42792cfee124a742999e698e348e99b382ba16 Mon Sep 17 00:00:00 2001<o:p></o:p> From: c00207022 <caoxinhua@huawei.com><o:p></o:p> Date: Thu, 2 Mar 2017 12:07:27 +0800<o:p></o:p> Subject: [PATCH] libvirt-remote:fix use-after-free when sending event message<o:p></o:p> <o:p> </o:p> [Changelog]:If there is a process with a client which registers event callbacks,<o:p></o:p> and it calls libvirt's API which uses the same virConnectPtr in that<o:p></o:p> callback function. When this process exit abnormally lead to client<o:p></o:p> disconnect, there is a possibility that the libvirtd's main thread is use<o:p></o:p> virServerClient to send event just after the virServerClient been freed by job<o:p></o:p> thread.<o:p></o:p> Following is backtrace:<o:p></o:p> <o:p> </o:p> #0 0x00007fda223d66d8 in virClassIsDerivedFrom (klass=0xdeadbeef, parent=0x7fda24c81b40) at util/virobject.c:169<o:p></o:p> #1 0x00007fda223d6a1e in virObjectIsClass (anyobj=anyobj@entry=0x7fd9e575b400, klass=<optimized out>) at util/virobject.c:365<o:p></o:p> #2 0x00007fda223d6a44 in virObjectLock (anyobj=0x7fd9e575b400) at util/virobject.c:317<o:p></o:p> #3 0x00007fda22507f71 in virNetServerClientSendMessage (client=client@entry=0x7fd9e575b400, msg=msg@entry=0x7fd9ec30de90)<o:p></o:p> at rpc/virnetserverclient.c:1422<o:p></o:p> #4 0x00007fda230d714d in remoteDispatchObjectEventSend (client=0x7fd9e575b400, program=0x7fda24c844e0, procnr=348,<o:p></o:p> proc=0x7fda2310e5e0 <xdr_remote_domain_event_callback_tunable_msg>, data=0x7ffc3857fdb0) at remote.c:3803<o:p></o:p> #5 0x00007fda230dd71b in remoteRelayDomainEventTunable (conn=<optimized out>, dom=0x7fda27cd7660, params=0x7fda27f3aae0, nparams=1,<o:p></o:p> opaque=0x7fd9e6c99e00) at remote.c:1033<o:p></o:p> #6 0x00007fda224484cb in virDomainEventDispatchDefaultFunc (conn=0x7fda27cd0120, event=0x7fda2736ea00,<o:p></o:p> cb=0x7fda230dd610 <remoteRelayDomainEventTunable>, cbopaque=0x7fd9e6c99e00) at conf/domain_event.c:1910<o:p></o:p> #7 0x00007fda22446871 in virObjectEventStateDispatchCallbacks (callbacks=<optimized out>, callbacks=<optimized out>, event=0x7fda2736ea00,<o:p></o:p> state=0x7fda24ca3960) at conf/object_event.c:722<o:p></o:p> #8 virObjectEventStateQueueDispatch (callbacks=0x7fda24c65800, queue=0x7ffc3857fe90, state=0x7fda24ca3960) at conf/object_event.c:736<o:p></o:p> #9 virObjectEventStateFlush (state=0x7fda24ca3960) at conf/object_event.c:814<o:p></o:p> #10 virObjectEventTimer (timer=<optimized out>, opaque=0x7fda24ca3960) at conf/object_event.c:560<o:p></o:p> #11 0x00007fda223ae8b9 in virEventPollDispatchTimeouts () at util/vireventpoll.c:458<o:p></o:p> #12 virEventPollRunOnce () at util/vireventpoll.c:654<o:p></o:p> #13 0x00007fda223ad1d2 in virEventRunDefaultImpl () at util/virevent.c:314<o:p></o:p> #14 0x00007fda225046cd in virNetDaemonRun (dmn=0x7fda24c775c0) at rpc/virnetdaemon.c:818<o:p></o:p> #15 0x00007fda230d6351 in main (argc=<optimized out>, argv=<optimized out>) at libvirtd.c:1623<o:p></o:p> <o:p> </o:p> Let's clean all event callback when client close.<o:p></o:p> <o:p> </o:p> Signed-off-by: Caoxinhua <caoxinhua@huawei.com><o:p></o:p> ---<o:p></o:p> daemon/remote.c | 76 +++++++++++++++++++++------------------------------------<o:p></o:p> 1 file changed, 28 insertions(+), 48 deletions(-)<o:p></o:p> <o:p> </o:p> diff --git a/daemon/remote.c b/daemon/remote.c<o:p></o:p> index 23c9de4..fb7cd50 100644<o:p></o:p> --- a/daemon/remote.c<o:p></o:p> +++ b/daemon/remote.c<o:p></o:p> @@ -101,6 +101,7 @@ static void make_nonnull_node_device(remote_nonnull_node_device *dev_dst, virNod<o:p></o:p> static void make_nonnull_secret(remote_nonnull_secret *secret_dst, virSecretPtr secret_src);<o:p></o:p> static void make_nonnull_nwfilter(remote_nonnull_nwfilter *net_dst, virNWFilterPtr nwfilter_src);<o:p></o:p> static void make_nonnull_domain_snapshot(remote_nonnull_domain_snapshot *snapshot_dst, virDomainSnapshotPtr snapshot_src);<o:p></o:p> +static void remoteClientCleanEventCallbacks(struct daemonClientPrivate *priv);<o:p></o:p> <o:p></o:p> static int<o:p></o:p> remoteSerializeDomainDiskErrors(virDomainDiskErrorPtr errors,<o:p></o:p> @@ -1483,24 +1484,15 @@ void remoteRelayConnectionClosedEvent(virConnectPtr conn ATTRIBUTE_UNUSED, int r<o:p></o:p> &msg);<o:p></o:p> }<o:p></o:p> <o:p></o:p> -/*<o:p></o:p> - * You must hold lock for at least the client<o:p></o:p> - * We don't free stuff here, merely disconnect the client's<o:p></o:p> - * network socket & resources.<o:p></o:p> - * We keep the libvirt connection open until any async<o:p></o:p> - * jobs have finished, then clean it up elsewhere<o:p></o:p> - */<o:p></o:p> -void remoteClientFreeFunc(void *data)<o:p></o:p> +static void remoteClientCleanEventCallbacks(struct daemonClientPrivate *priv)<o:p></o:p> {<o:p></o:p> - struct daemonClientPrivate *priv = data;<o:p></o:p> -<o:p></o:p> /* Deregister event delivery callback */<o:p></o:p> - if (priv->conn) {<o:p></o:p> - virIdentityPtr sysident = virIdentityGetSystem();<o:p></o:p> + if (priv && priv->conn) {<o:p></o:p> size_t i;<o:p></o:p> -<o:p></o:p> + virIdentityPtr sysident = virIdentityGetSystem();<o:p></o:p> virIdentitySetCurrent(sysident);<o:p></o:p> <o:p></o:p> + virMutexLock(&priv->lock);<o:p></o:p> for (i = 0; i < priv->ndomainEventCallbacks; i++) {<o:p></o:p> int callbackID = priv->domainEventCallbacks[i]->callbackID;<o:p></o:p> if (callbackID < 0) {<o:p></o:p> @@ -1514,6 +1506,7 @@ void remoteClientFreeFunc(void *data)<o:p></o:p> VIR_WARN("unexpected domain event deregister failure");<o:p></o:p> }<o:p></o:p> VIR_FREE(priv->domainEventCallbacks);<o:p></o:p> + priv->ndomainEventCallbacks = 0;<o:p></o:p> <o:p></o:p> for (i = 0; i < priv->nnetworkEventCallbacks; i++) {<o:p></o:p> int callbackID = priv->networkEventCallbacks[i]->callbackID;<o:p></o:p> @@ -1529,36 +1522,7 @@ void remoteClientFreeFunc(void *data)<o:p></o:p> VIR_WARN("unexpected network event deregister failure");<o:p></o:p> }<o:p></o:p> VIR_FREE(priv->networkEventCallbacks);<o:p></o:p> -<o:p></o:p> - for (i = 0; i < priv->nstorageEventCallbacks; i++) {<o:p></o:p> - int callbackID = priv->storageEventCallbacks[i]->callbackID;<o:p></o:p> - if (callbackID < 0) {<o:p></o:p> - VIR_WARN("unexpected incomplete storage pool callback %zu", i);<o:p></o:p> - continue;<o:p></o:p> - }<o:p></o:p> - VIR_DEBUG("Deregistering remote storage pool event relay %d",<o:p></o:p> - callbackID);<o:p></o:p> - priv->storageEventCallbacks[i]->callbackID = -1;<o:p></o:p> - if (virConnectStoragePoolEventDeregisterAny(priv->conn,<o:p></o:p> - callbackID) < 0)<o:p></o:p> - VIR_WARN("unexpected storage pool event deregister failure");<o:p></o:p> - }<o:p></o:p> - VIR_FREE(priv->storageEventCallbacks);<o:p></o:p> -<o:p></o:p> - for (i = 0; i < priv->nnodeDeviceEventCallbacks; i++) {<o:p></o:p> - int callbackID = priv->nodeDeviceEventCallbacks[i]->callbackID;<o:p></o:p> - if (callbackID < 0) {<o:p></o:p> - VIR_WARN("unexpected incomplete node device callback %zu", i);<o:p></o:p> - continue;<o:p></o:p> - }<o:p></o:p> - VIR_DEBUG("Deregistering remote node device event relay %d",<o:p></o:p> - callbackID);<o:p></o:p> - priv->nodeDeviceEventCallbacks[i]->callbackID = -1;<o:p></o:p> - if (virConnectNodeDeviceEventDeregisterAny(priv->conn,<o:p></o:p> - callbackID) < 0)<o:p></o:p> - VIR_WARN("unexpected node device event deregister failure");<o:p></o:p> - }<o:p></o:p> - VIR_FREE(priv->nodeDeviceEventCallbacks);<o:p></o:p> + priv->nnetworkEventCallbacks = 0;<o:p></o:p> <o:p></o:p> for (i = 0; i < priv->nqemuEventCallbacks; i++) {<o:p></o:p> int callbackID = priv->qemuEventCallbacks[i]->callbackID;<o:p></o:p> @@ -1574,31 +1538,47 @@ void remoteClientFreeFunc(void *data)<o:p></o:p> VIR_WARN("unexpected qemu monitor event deregister failure");<o:p></o:p> }<o:p></o:p> VIR_FREE(priv->qemuEventCallbacks);<o:p></o:p> + priv->nqemuEventCallbacks = 0;<o:p></o:p> <o:p></o:p> if (priv->closeRegistered) {<o:p></o:p> if (virConnectUnregisterCloseCallback(priv->conn,<o:p></o:p> remoteRelayConnectionClosedEvent) < 0)<o:p></o:p> VIR_WARN("unexpected close callback event deregister failure");<o:p></o:p> }<o:p></o:p> -<o:p></o:p> - virConnectClose(priv->conn);<o:p></o:p> -<o:p></o:p> + virMutexUnlock(&priv->lock);<o:p></o:p> virIdentitySetCurrent(NULL);<o:p></o:p> virObjectUnref(sysident);<o:p></o:p> }<o:p></o:p> +}<o:p></o:p> +<o:p></o:p> +<o:p></o:p> +/*<o:p></o:p> + * You must hold lock for at least the client<o:p></o:p> + * We don't free stuff here, merely disconnect the client's<o:p></o:p> + * network socket & resources.<o:p></o:p> + * We keep the libvirt connection open until any async<o:p></o:p> + * jobs have finished, then clean it up elsewhere<o:p></o:p> + */<o:p></o:p> +void remoteClientFreeFunc(void *data)<o:p></o:p> +{<o:p></o:p> + struct daemonClientPrivate *priv = data;<o:p></o:p> + if (!priv || !priv->conn)<o:p></o:p> + return;<o:p></o:p> +<o:p></o:p> + remoteClientCleanEventCallbacks(priv);<o:p></o:p> + virConnectClose(priv->conn);<o:p></o:p> <o:p></o:p> VIR_FREE(priv);<o:p></o:p> }<o:p></o:p> <o:p></o:p> -<o:p></o:p> static void remoteClientCloseFunc(virNetServerClientPtr client)<o:p></o:p> {<o:p></o:p> struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client);<o:p></o:p> <o:p></o:p> daemonRemoveAllClientStreams(priv->streams);<o:p></o:p> + remoteClientCleanEventCallbacks(priv);<o:p></o:p> }<o:p></o:p> <o:p></o:p> -<o:p></o:p> void *remoteClientInitHook(virNetServerClientPtr client,<o:p></o:p> void *opaque ATTRIBUTE_UNUSED)<o:p></o:p> {<o:p></o:p> -- <o:p></o:p> 1.8.3.1<o:p></o:p> <o:p> </o:p> <o:p> </o:p> <o:p> </o:p> </div> </body> </html>