External/libvirt
1
0
Fork 0
mirror of https://gitlab.com/libvirt/libvirt.git synced 2024-11-08 14:29:56 +00:00
Code Issues Packages Projects Releases Wiki Activity

remote: fix lock ordering mistake in event registration

Browse Source 
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.

Reviewed-by: Ján Tomko <jtomko@redhat.com>
Reviewed-by: Andrea Bolognani <abologna@redhat.com>
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
This commit is contained in:
Daniel P. Berrangé 2019-07-29 15:09:14 +01:00
parent 9cc8ecc809
commit 7ea3f0d7ba
1 changed files with 32 additions and 32 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

64
src/remote/remote_daemon_dispatch.c
View File

@ -4212,13 +4212,13 @@ remoteDispatchConnectDomainEventRegister(virNetServerPtr server ATTRIBUTE_UNUSED
struct daemonClientPrivate *priv = struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client); virNetServerClientGetPrivateData(client);
virMutexLock(&priv->lock);
if (!priv->conn) { if (!priv->conn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
goto cleanup; goto cleanup;
} }
virMutexLock(&priv->lock);
/* If we call register first, we could append a complete callback /* 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 * 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 * deregister works to undo our register. So instead we append an
@ -4276,13 +4276,13 @@ remoteDispatchConnectDomainEventDeregister(virNetServerPtr server ATTRIBUTE_UNUS
struct daemonClientPrivate *priv = struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client); virNetServerClientGetPrivateData(client);
virMutexLock(&priv->lock);
if (!priv->conn) { if (!priv->conn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
goto cleanup; goto cleanup;
} }
virMutexLock(&priv->lock);
for (i = 0; i < priv->ndomainEventCallbacks; i++) { for (i = 0; i < priv->ndomainEventCallbacks; i++) {
if (priv->domainEventCallbacks[i]->eventID == VIR_DOMAIN_EVENT_ID_LIFECYCLE) { if (priv->domainEventCallbacks[i]->eventID == VIR_DOMAIN_EVENT_ID_LIFECYCLE) {
callbackID = priv->domainEventCallbacks[i]->callbackID; callbackID = priv->domainEventCallbacks[i]->callbackID;
@ -4440,13 +4440,13 @@ remoteDispatchConnectDomainEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNU
struct daemonClientPrivate *priv = struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client); virNetServerClientGetPrivateData(client);
virMutexLock(&priv->lock);
if (!priv->conn) { if (!priv->conn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
goto cleanup; goto cleanup;
} }
virMutexLock(&priv->lock);
/* We intentionally do not use VIR_DOMAIN_EVENT_ID_LAST here; any /* We intentionally do not use VIR_DOMAIN_EVENT_ID_LAST here; any
* new domain events added after this point should only use the * new domain events added after this point should only use the
* modern callback style of RPC. */ * modern callback style of RPC. */
@ -4516,13 +4516,13 @@ remoteDispatchConnectDomainEventCallbackRegisterAny(virNetServerPtr server ATTRI
virNetServerClientGetPrivateData(client); virNetServerClientGetPrivateData(client);
virDomainPtr dom = NULL; virDomainPtr dom = NULL;
virMutexLock(&priv->lock);
if (!priv->conn) { if (!priv->conn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
goto cleanup; goto cleanup;
} }
virMutexLock(&priv->lock);
if (args->dom && if (args->dom &&
!(dom = get_nonnull_domain(priv->conn, *args->dom))) !(dom = get_nonnull_domain(priv->conn, *args->dom)))
goto cleanup; goto cleanup;
@ -4590,13 +4590,13 @@ remoteDispatchConnectDomainEventDeregisterAny(virNetServerPtr server ATTRIBUTE_U
struct daemonClientPrivate *priv = struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client); virNetServerClientGetPrivateData(client);
virMutexLock(&priv->lock);
if (!priv->conn) { if (!priv->conn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
goto cleanup; goto cleanup;
} }
virMutexLock(&priv->lock);
/* We intentionally do not use VIR_DOMAIN_EVENT_ID_LAST here; any /* We intentionally do not use VIR_DOMAIN_EVENT_ID_LAST here; any
* new domain events added after this point should only use the * new domain events added after this point should only use the
* modern callback style of RPC. */ * modern callback style of RPC. */
@ -4647,13 +4647,13 @@ remoteDispatchConnectDomainEventCallbackDeregisterAny(virNetServerPtr server ATT
struct daemonClientPrivate *priv = struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client); virNetServerClientGetPrivateData(client);
virMutexLock(&priv->lock);
if (!priv->conn) { if (!priv->conn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
goto cleanup; goto cleanup;
} }
virMutexLock(&priv->lock);
for (i = 0; i < priv->ndomainEventCallbacks; i++) { for (i = 0; i < priv->ndomainEventCallbacks; i++) {
if (priv->domainEventCallbacks[i]->callbackID == args->callbackID) if (priv->domainEventCallbacks[i]->callbackID == args->callbackID)
break; break;
@ -6089,13 +6089,13 @@ remoteDispatchConnectNetworkEventRegisterAny(virNetServerPtr server ATTRIBUTE_UN
virNetServerClientGetPrivateData(client); virNetServerClientGetPrivateData(client);
virNetworkPtr net = NULL; virNetworkPtr net = NULL;
virMutexLock(&priv->lock);
if (!priv->networkConn) { if (!priv->networkConn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
goto cleanup; goto cleanup;
} }
virMutexLock(&priv->lock);
if (args->net && if (args->net &&
!(net = get_nonnull_network(priv->networkConn, *args->net))) !(net = get_nonnull_network(priv->networkConn, *args->net)))
goto cleanup; goto cleanup;
@ -6162,13 +6162,13 @@ remoteDispatchConnectNetworkEventDeregisterAny(virNetServerPtr server ATTRIBUTE_
struct daemonClientPrivate *priv = struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client); virNetServerClientGetPrivateData(client);
virMutexLock(&priv->lock);
if (!priv->networkConn) { if (!priv->networkConn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
goto cleanup; goto cleanup;
} }
virMutexLock(&priv->lock);
for (i = 0; i < priv->nnetworkEventCallbacks; i++) { for (i = 0; i < priv->nnetworkEventCallbacks; i++) {
if (priv->networkEventCallbacks[i]->callbackID == args->callbackID) if (priv->networkEventCallbacks[i]->callbackID == args->callbackID)
break; break;
@ -6211,13 +6211,13 @@ remoteDispatchConnectStoragePoolEventRegisterAny(virNetServerPtr server ATTRIBUT
virNetServerClientGetPrivateData(client); virNetServerClientGetPrivateData(client);
virStoragePoolPtr pool = NULL; virStoragePoolPtr pool = NULL;
virMutexLock(&priv->lock);
if (!priv->storageConn) { if (!priv->storageConn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
goto cleanup; goto cleanup;
} }
virMutexLock(&priv->lock);
if (args->pool && if (args->pool &&
!(pool = get_nonnull_storage_pool(priv->storageConn, *args->pool))) !(pool = get_nonnull_storage_pool(priv->storageConn, *args->pool)))
goto cleanup; goto cleanup;
@ -6283,13 +6283,13 @@ remoteDispatchConnectStoragePoolEventDeregisterAny(virNetServerPtr server ATTRIB
struct daemonClientPrivate *priv = struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client); virNetServerClientGetPrivateData(client);
virMutexLock(&priv->lock);
if (!priv->storageConn) { if (!priv->storageConn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
goto cleanup; goto cleanup;
} }
virMutexLock(&priv->lock);
for (i = 0; i < priv->nstorageEventCallbacks; i++) { for (i = 0; i < priv->nstorageEventCallbacks; i++) {
if (priv->storageEventCallbacks[i]->callbackID == args->callbackID) if (priv->storageEventCallbacks[i]->callbackID == args->callbackID)
break; break;
@ -6332,13 +6332,13 @@ remoteDispatchConnectNodeDeviceEventRegisterAny(virNetServerPtr server ATTRIBUTE
virNetServerClientGetPrivateData(client); virNetServerClientGetPrivateData(client);
virNodeDevicePtr dev = NULL; virNodeDevicePtr dev = NULL;
virMutexLock(&priv->lock);
if (!priv->nodedevConn) { if (!priv->nodedevConn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
goto cleanup; goto cleanup;
} }
virMutexLock(&priv->lock);
if (args->dev && if (args->dev &&
!(dev = get_nonnull_node_device(priv->nodedevConn, *args->dev))) !(dev = get_nonnull_node_device(priv->nodedevConn, *args->dev)))
goto cleanup; goto cleanup;
@ -6404,13 +6404,13 @@ remoteDispatchConnectNodeDeviceEventDeregisterAny(virNetServerPtr server ATTRIBU
struct daemonClientPrivate *priv = struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client); virNetServerClientGetPrivateData(client);
virMutexLock(&priv->lock);
if (!priv->nodedevConn) { if (!priv->nodedevConn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
goto cleanup; goto cleanup;
} }
virMutexLock(&priv->lock);
for (i = 0; i < priv->nnodeDeviceEventCallbacks; i++) { for (i = 0; i < priv->nnodeDeviceEventCallbacks; i++) {
if (priv->nodeDeviceEventCallbacks[i]->callbackID == args->callbackID) if (priv->nodeDeviceEventCallbacks[i]->callbackID == args->callbackID)
break; break;
@ -6453,13 +6453,13 @@ remoteDispatchConnectSecretEventRegisterAny(virNetServerPtr server ATTRIBUTE_UNU
virNetServerClientGetPrivateData(client); virNetServerClientGetPrivateData(client);
virSecretPtr secret = NULL; virSecretPtr secret = NULL;
virMutexLock(&priv->lock);
if (!priv->secretConn) { if (!priv->secretConn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
goto cleanup; goto cleanup;
} }
virMutexLock(&priv->lock);
if (args->secret && if (args->secret &&
!(secret = get_nonnull_secret(priv->secretConn, *args->secret))) !(secret = get_nonnull_secret(priv->secretConn, *args->secret)))
goto cleanup; goto cleanup;
@ -6525,13 +6525,13 @@ remoteDispatchConnectSecretEventDeregisterAny(virNetServerPtr server ATTRIBUTE_U
struct daemonClientPrivate *priv = struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client); virNetServerClientGetPrivateData(client);
virMutexLock(&priv->lock);
if (!priv->secretConn) { if (!priv->secretConn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
goto cleanup; goto cleanup;
} }
virMutexLock(&priv->lock);
for (i = 0; i < priv->nsecretEventCallbacks; i++) { for (i = 0; i < priv->nsecretEventCallbacks; i++) {
if (priv->secretEventCallbacks[i]->callbackID == args->callbackID) if (priv->secretEventCallbacks[i]->callbackID == args->callbackID)
break; break;
@ -6575,13 +6575,13 @@ qemuDispatchConnectDomainMonitorEventRegister(virNetServerPtr server ATTRIBUTE_U
virDomainPtr dom = NULL; virDomainPtr dom = NULL;
const char *event = args->event ? *args->event : NULL; const char *event = args->event ? *args->event : NULL;
virMutexLock(&priv->lock);
if (!priv->conn) { if (!priv->conn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
goto cleanup; goto cleanup;
} }
virMutexLock(&priv->lock);
if (args->dom && if (args->dom &&
!(dom = get_nonnull_domain(priv->conn, *args->dom))) !(dom = get_nonnull_domain(priv->conn, *args->dom)))
goto cleanup; goto cleanup;
@ -6643,13 +6643,13 @@ qemuDispatchConnectDomainMonitorEventDeregister(virNetServerPtr server ATTRIBUTE
struct daemonClientPrivate *priv = struct daemonClientPrivate *priv =
virNetServerClientGetPrivateData(client); virNetServerClientGetPrivateData(client);
virMutexLock(&priv->lock);
if (!priv->conn) { if (!priv->conn) {
virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open")); virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not open"));
goto cleanup; goto cleanup;
} }
virMutexLock(&priv->lock);
for (i = 0; i < priv->nqemuEventCallbacks; i++) { for (i = 0; i < priv->nqemuEventCallbacks; i++) {
if (priv->qemuEventCallbacks[i]->callbackID == args->callbackID) if (priv->qemuEventCallbacks[i]->callbackID == args->callbackID)
break; break;