[libvirt] [PATCH v3 36/48] remote: fix lock ordering mistake in event registration
Daniel P. Berrangé
berrange at redhat.com
Mon Jul 29 17:11:18 UTC 2019
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 at redhat.com>
---
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
More information about the libvir-list
mailing list