[libvirt] [PATCH v3 36/48] remote: fix lock ordering mistake in event registration

If the event (un)registration methods are invoked while no connection is open, they jump to a cleanup block which unlocks a mutex which is not currently locked. Signed-off-by: Daniel P. Berrangé <berrange(a)redhat.com&gt; --- src/remote/remote_daemon_dispatch.c | 64 ++++++++++++++--------------- 1 file changed, 32 insertions(+), 32 deletions(-) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon_dispatch.c index 90103f5093..4a3312a944 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -4212,13 +4212,13 @@ remoteDispatchConnectDomainEventRegister(virNetServerPtr server ATTRIBUTE_UNUSED struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + virMutexLock(&priv->lock); + if (!priv->conn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); goto cleanup; } - virMutexLock(&priv->lock); - /* If we call register first, we could append a complete callback * to our array, but on OOM append failure, we'd have to then hope * deregister works to undo our register. So instead we append an @@ -4276,13 +4276,13 @@ remoteDispatchConnectDomainEventDeregister(virNetServerPtr server ATTRIBUTE_UNUS struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + virMutexLock(&priv->lock); + if (!priv->conn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); goto cleanup; } - virMutexLock(&priv->lock); - for (i = 0; i < priv->ndomainEventCallbacks; i++) { if (priv->domainEventCallbacks[i]->eventID == VIR_DOMAIN_EVENT_ID_LIFECYCLE) { callbackID = priv->domainEventCallbacks[i]->callbackID; @@ -4440,13 +4440,13 @@ remoteDispatchConnectDomainEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNU struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + virMutexLock(&priv->lock); + if (!priv->conn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); goto cleanup; } - virMutexLock(&priv->lock); - /* We intentionally do not use VIR_DOMAIN_EVENT_ID_LAST here; any * new domain events added after this point should only use the * modern callback style of RPC. */ @@ -4516,13 +4516,13 @@ remoteDispatchConnectDomainEventCallbackRegisterAny(virNetServerPtr server ATTRI virNetServerClientGetPrivateData(client); virDomainPtr dom = NULL; + virMutexLock(&priv->lock); + if (!priv->conn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); goto cleanup; } - virMutexLock(&priv->lock); - if (args->dom && !(dom = get_nonnull_domain(priv->conn, *args->dom))) goto cleanup; @@ -4590,13 +4590,13 @@ remoteDispatchConnectDomainEventDeregisterAny(virNetServerPtr server ATTRIBUTE_U struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + virMutexLock(&priv->lock); + if (!priv->conn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); goto cleanup; } - virMutexLock(&priv->lock); - /* We intentionally do not use VIR_DOMAIN_EVENT_ID_LAST here; any * new domain events added after this point should only use the * modern callback style of RPC. */ @@ -4647,13 +4647,13 @@ remoteDispatchConnectDomainEventCallbackDeregisterAny(virNetServerPtr server ATT struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + virMutexLock(&priv->lock); + if (!priv->conn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); goto cleanup; } - virMutexLock(&priv->lock); - for (i = 0; i < priv->ndomainEventCallbacks; i++) { if (priv->domainEventCallbacks[i]->callbackID == args->callbackID) break; @@ -6089,13 +6089,13 @@ remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server ATTRIBUTE_UN virNetServerClientGetPrivateData(client); virNetworkPtr net = NULL; + virMutexLock(&priv->lock); + if (!priv->networkConn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); goto cleanup; } - virMutexLock(&priv->lock); - if (args->net && !(net = get_nonnull_network(priv->networkConn, *args->net))) goto cleanup; @@ -6162,13 +6162,13 @@ remoteDispatchConnectNetworkEventDeregisterAny(virNetServerPtr server ATTRIBUTE_ struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + virMutexLock(&priv->lock); + if (!priv->networkConn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); goto cleanup; } - virMutexLock(&priv->lock); - for (i = 0; i < priv->nnetworkEventCallbacks; i++) { if (priv->networkEventCallbacks[i]->callbackID == args->callbackID) break; @@ -6211,13 +6211,13 @@ remoteDispatchConnectStoragePoolEventRegisterAny(virNetServerPtr server ATTRIBUT virNetServerClientGetPrivateData(client); virStoragePoolPtr pool = NULL; + virMutexLock(&priv->lock); + if (!priv->storageConn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); goto cleanup; } - virMutexLock(&priv->lock); - if (args->pool && !(pool = get_nonnull_storage_pool(priv->storageConn, *args->pool))) goto cleanup; @@ -6283,13 +6283,13 @@ remoteDispatchConnectStoragePoolEventDeregisterAny(virNetServerPtr server ATTRIB struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + virMutexLock(&priv->lock); + if (!priv->storageConn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); goto cleanup; } - virMutexLock(&priv->lock); - for (i = 0; i < priv->nstorageEventCallbacks; i++) { if (priv->storageEventCallbacks[i]->callbackID == args->callbackID) break; @@ -6332,13 +6332,13 @@ remoteDispatchConnectNodeDeviceEventRegisterAny(virNetServerPtr server ATTRIBUTE virNetServerClientGetPrivateData(client); virNodeDevicePtr dev = NULL; + virMutexLock(&priv->lock); + if (!priv->nodedevConn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); goto cleanup; } - virMutexLock(&priv->lock); - if (args->dev && !(dev = get_nonnull_node_device(priv->nodedevConn, *args->dev))) goto cleanup; @@ -6404,13 +6404,13 @@ remoteDispatchConnectNodeDeviceEventDeregisterAny(virNetServerPtr server ATTRIBU struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + virMutexLock(&priv->lock); + if (!priv->nodedevConn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); goto cleanup; } - virMutexLock(&priv->lock); - for (i = 0; i < priv->nnodeDeviceEventCallbacks; i++) { if (priv->nodeDeviceEventCallbacks[i]->callbackID == args->callbackID) break; @@ -6453,13 +6453,13 @@ remoteDispatchConnectSecretEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNU virNetServerClientGetPrivateData(client); virSecretPtr secret = NULL; + virMutexLock(&priv->lock); + if (!priv->secretConn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); goto cleanup; } - virMutexLock(&priv->lock); - if (args->secret && !(secret = get_nonnull_secret(priv->secretConn, *args->secret))) goto cleanup; @@ -6525,13 +6525,13 @@ remoteDispatchConnectSecretEventDeregisterAny(virNetServerPtr server ATTRIBUTE_U struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + virMutexLock(&priv->lock); + if (!priv->secretConn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); goto cleanup; } - virMutexLock(&priv->lock); - for (i = 0; i < priv->nsecretEventCallbacks; i++) { if (priv->secretEventCallbacks[i]->callbackID == args->callbackID) break; @@ -6575,13 +6575,13 @@ qemuDispatchConnectDomainMonitorEventRegister(virNetServerPtr server ATTRIBUTE_U virDomainPtr dom = NULL; const char *event = args->event ? *args->event : NULL; + virMutexLock(&priv->lock); + if (!priv->conn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); goto cleanup; } - virMutexLock(&priv->lock); - if (args->dom && !(dom = get_nonnull_domain(priv->conn, *args->dom))) goto cleanup; @@ -6643,13 +6643,13 @@ qemuDispatchConnectDomainMonitorEventDeregister(virNetServerPtr server ATTRIBUTE struct daemonClientPrivate *priv = virNetServerClientGetPrivateData(client); + virMutexLock(&priv->lock); + if (!priv->conn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); goto cleanup; } - virMutexLock(&priv->lock); - for (i = 0; i < priv->nqemuEventCallbacks; i++) { if (priv->qemuEventCallbacks[i]->callbackID == args->callbackID) break; -- 2.21.0