From nobody Sun Feb 8 07:07:57 2026 Delivered-To: importer@patchew.org Received-SPF: pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) client-ip=209.132.183.28; envelope-from=libvir-list-bounces@redhat.com; helo=mx1.redhat.com; Authentication-Results: mx.zohomail.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass(p=none dis=none) header.from=redhat.com ARC-Seal: i=1; a=rsa-sha256; t=1565277240; cv=none; d=zoho.com; s=zohoarc; b=X2hpMWunxky2FUcIEkPg89KDnNiftLbbN+pftQwwdS64UMl6URim/zT07HxGjCfjCTRH5ue09olA+23FljR65wd4/iT/+Bmu5hJZGeZHwS9mTxsaQpcq7rYtigfSO5BCZppgLnkDaDfgM+JL5pBPucTcU72moj+l4gyWwJJSV6o= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zoho.com; s=zohoarc; t=1565277240; h=Content-Type:Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:References:Sender:Subject:To:ARC-Authentication-Results; bh=OZnLnu2GXVpBOvmFL6dpg1E5W0mavrRCVeNA2R7yyqo=; b=R6ojJIBkt5gCuwmaAB5xryOlMuG7Iq03DJiVzIb+V+lS+iV4JO0WzBDYVYxzvqWhfmJIvVetNcHFiBcAp2dFyUcRCu5WTnsUhIRn5vMkfr4yj5E1Vhu2chMbZ+w+4PgeF8mVsaGirHSVRBRiFtaJprXtgNEIc3X/yOg6ef16L5E= ARC-Authentication-Results: i=1; mx.zoho.com; spf=pass (zoho.com: domain of redhat.com designates 209.132.183.28 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) header.from= Return-Path: Received: from mx1.redhat.com (mx1.redhat.com [209.132.183.28]) by mx.zohomail.com with SMTPS id 1565277240087665.8700902007563; Thu, 8 Aug 2019 08:14:00 -0700 (PDT) Received: from smtp.corp.redhat.com (int-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.12]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id F17F5C001F03; Thu, 8 Aug 2019 15:13:58 +0000 (UTC) Received: from colo-mx.corp.redhat.com (colo-mx02.intmail.prod.int.phx2.redhat.com [10.5.11.21]) by smtp.corp.redhat.com (Postfix) with ESMTPS id C7BAD60BE1; Thu, 8 Aug 2019 15:13:58 +0000 (UTC) Received: from lists01.pubmisc.prod.ext.phx2.redhat.com (lists01.pubmisc.prod.ext.phx2.redhat.com [10.5.19.33]) by colo-mx.corp.redhat.com (Postfix) with ESMTP id 89C05246A3; Thu, 8 Aug 2019 15:13:58 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.phx2.redhat.com [10.5.11.22]) by lists01.pubmisc.prod.ext.phx2.redhat.com (8.13.8/8.13.8) with ESMTP id x78FC837027996 for ; Thu, 8 Aug 2019 11:12:08 -0400 Received: by smtp.corp.redhat.com (Postfix) id 174D51001958; Thu, 8 Aug 2019 15:12:08 +0000 (UTC) Received: from localhost.localdomain.com (ovpn-112-28.ams2.redhat.com [10.36.112.28]) by smtp.corp.redhat.com (Postfix) with ESMTP id E4AD9100194E; Thu, 8 Aug 2019 15:12:06 +0000 (UTC) From: =?UTF-8?q?Daniel=20P=2E=20Berrang=C3=A9?= To: libvir-list@redhat.com Date: Thu, 8 Aug 2019 16:10:38 +0100 Message-Id: <20190808151044.11385-37-berrange@redhat.com> In-Reply-To: <20190808151044.11385-1-berrange@redhat.com> References: <20190808151044.11385-1-berrange@redhat.com> MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.84 on 10.5.11.22 X-loop: libvir-list@redhat.com Cc: =?UTF-8?q?J=C3=A1n=20Tomko?= , Andrea Bolognani Subject: [libvirt] [PATCH v4 36/42] remote: fix lock ordering mistake in event registration X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.12 Precedence: junk List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable Sender: libvir-list-bounces@redhat.com Errors-To: libvir-list-bounces@redhat.com X-Scanned-By: MIMEDefang 2.79 on 10.5.11.12 X-Greylist: Sender IP whitelisted, not delayed by milter-greylist-4.5.16 (mx1.redhat.com [10.5.110.31]); Thu, 08 Aug 2019 15:13:59 +0000 (UTC) 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=C3=A1n Tomko Reviewed-by: Andrea Bolognani Signed-off-by: Daniel P. Berrang=C3=A9 --- 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(virNetServ= erPtr server ATTRIBUTE_UNUSED struct daemonClientPrivate *priv =3D virNetServerClientGetPrivateData(client); =20 + virMutexLock(&priv->lock); + if (!priv->conn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not ope= n")); goto cleanup; } =20 - 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(virNetSe= rverPtr server ATTRIBUTE_UNUS struct daemonClientPrivate *priv =3D virNetServerClientGetPrivateData(client); =20 + virMutexLock(&priv->lock); + if (!priv->conn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not ope= n")); goto cleanup; } =20 - virMutexLock(&priv->lock); - for (i =3D 0; i < priv->ndomainEventCallbacks; i++) { if (priv->domainEventCallbacks[i]->eventID =3D=3D VIR_DOMAIN_EVENT= _ID_LIFECYCLE) { callbackID =3D priv->domainEventCallbacks[i]->callbackID; @@ -4440,13 +4440,13 @@ remoteDispatchConnectDomainEventRegisterAny(virNetS= erverPtr server ATTRIBUTE_UNU struct daemonClientPrivate *priv =3D virNetServerClientGetPrivateData(client); =20 + virMutexLock(&priv->lock); + if (!priv->conn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not ope= n")); goto cleanup; } =20 - 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 =3D NULL; =20 + virMutexLock(&priv->lock); + if (!priv->conn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not ope= n")); goto cleanup; } =20 - virMutexLock(&priv->lock); - if (args->dom && !(dom =3D get_nonnull_domain(priv->conn, *args->dom))) goto cleanup; @@ -4590,13 +4590,13 @@ remoteDispatchConnectDomainEventDeregisterAny(virNe= tServerPtr server ATTRIBUTE_U struct daemonClientPrivate *priv =3D virNetServerClientGetPrivateData(client); =20 + virMutexLock(&priv->lock); + if (!priv->conn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not ope= n")); goto cleanup; } =20 - 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 @@ remoteDispatchConnectDomainEventCallbackDeregisterA= ny(virNetServerPtr server ATT struct daemonClientPrivate *priv =3D virNetServerClientGetPrivateData(client); =20 + virMutexLock(&priv->lock); + if (!priv->conn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not ope= n")); goto cleanup; } =20 - virMutexLock(&priv->lock); - for (i =3D 0; i < priv->ndomainEventCallbacks; i++) { if (priv->domainEventCallbacks[i]->callbackID =3D=3D args->callbac= kID) break; @@ -6089,13 +6089,13 @@ remoteDispatchConnectNetworkEventRegisterAny(virNet= ServerPtr server ATTRIBUTE_UN virNetServerClientGetPrivateData(client); virNetworkPtr net =3D NULL; =20 + virMutexLock(&priv->lock); + if (!priv->networkConn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not ope= n")); goto cleanup; } =20 - virMutexLock(&priv->lock); - if (args->net && !(net =3D get_nonnull_network(priv->networkConn, *args->net))) goto cleanup; @@ -6162,13 +6162,13 @@ remoteDispatchConnectNetworkEventDeregisterAny(virN= etServerPtr server ATTRIBUTE_ struct daemonClientPrivate *priv =3D virNetServerClientGetPrivateData(client); =20 + virMutexLock(&priv->lock); + if (!priv->networkConn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not ope= n")); goto cleanup; } =20 - virMutexLock(&priv->lock); - for (i =3D 0; i < priv->nnetworkEventCallbacks; i++) { if (priv->networkEventCallbacks[i]->callbackID =3D=3D args->callba= ckID) break; @@ -6211,13 +6211,13 @@ remoteDispatchConnectStoragePoolEventRegisterAny(vi= rNetServerPtr server ATTRIBUT virNetServerClientGetPrivateData(client); virStoragePoolPtr pool =3D NULL; =20 + virMutexLock(&priv->lock); + if (!priv->storageConn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not ope= n")); goto cleanup; } =20 - virMutexLock(&priv->lock); - if (args->pool && !(pool =3D get_nonnull_storage_pool(priv->storageConn, *args->pool= ))) goto cleanup; @@ -6283,13 +6283,13 @@ remoteDispatchConnectStoragePoolEventDeregisterAny(= virNetServerPtr server ATTRIB struct daemonClientPrivate *priv =3D virNetServerClientGetPrivateData(client); =20 + virMutexLock(&priv->lock); + if (!priv->storageConn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not ope= n")); goto cleanup; } =20 - virMutexLock(&priv->lock); - for (i =3D 0; i < priv->nstorageEventCallbacks; i++) { if (priv->storageEventCallbacks[i]->callbackID =3D=3D args->callba= ckID) break; @@ -6332,13 +6332,13 @@ remoteDispatchConnectNodeDeviceEventRegisterAny(vir= NetServerPtr server ATTRIBUTE virNetServerClientGetPrivateData(client); virNodeDevicePtr dev =3D NULL; =20 + virMutexLock(&priv->lock); + if (!priv->nodedevConn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not ope= n")); goto cleanup; } =20 - virMutexLock(&priv->lock); - if (args->dev && !(dev =3D get_nonnull_node_device(priv->nodedevConn, *args->dev))) goto cleanup; @@ -6404,13 +6404,13 @@ remoteDispatchConnectNodeDeviceEventDeregisterAny(v= irNetServerPtr server ATTRIBU struct daemonClientPrivate *priv =3D virNetServerClientGetPrivateData(client); =20 + virMutexLock(&priv->lock); + if (!priv->nodedevConn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not ope= n")); goto cleanup; } =20 - virMutexLock(&priv->lock); - for (i =3D 0; i < priv->nnodeDeviceEventCallbacks; i++) { if (priv->nodeDeviceEventCallbacks[i]->callbackID =3D=3D args->cal= lbackID) break; @@ -6453,13 +6453,13 @@ remoteDispatchConnectSecretEventRegisterAny(virNetS= erverPtr server ATTRIBUTE_UNU virNetServerClientGetPrivateData(client); virSecretPtr secret =3D NULL; =20 + virMutexLock(&priv->lock); + if (!priv->secretConn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not ope= n")); goto cleanup; } =20 - virMutexLock(&priv->lock); - if (args->secret && !(secret =3D get_nonnull_secret(priv->secretConn, *args->secret))) goto cleanup; @@ -6525,13 +6525,13 @@ remoteDispatchConnectSecretEventDeregisterAny(virNe= tServerPtr server ATTRIBUTE_U struct daemonClientPrivate *priv =3D virNetServerClientGetPrivateData(client); =20 + virMutexLock(&priv->lock); + if (!priv->secretConn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not ope= n")); goto cleanup; } =20 - virMutexLock(&priv->lock); - for (i =3D 0; i < priv->nsecretEventCallbacks; i++) { if (priv->secretEventCallbacks[i]->callbackID =3D=3D args->callbac= kID) break; @@ -6575,13 +6575,13 @@ qemuDispatchConnectDomainMonitorEventRegister(virNe= tServerPtr server ATTRIBUTE_U virDomainPtr dom =3D NULL; const char *event =3D args->event ? *args->event : NULL; =20 + virMutexLock(&priv->lock); + if (!priv->conn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not ope= n")); goto cleanup; } =20 - virMutexLock(&priv->lock); - if (args->dom && !(dom =3D get_nonnull_domain(priv->conn, *args->dom))) goto cleanup; @@ -6643,13 +6643,13 @@ qemuDispatchConnectDomainMonitorEventDeregister(vir= NetServerPtr server ATTRIBUTE struct daemonClientPrivate *priv =3D virNetServerClientGetPrivateData(client); =20 + virMutexLock(&priv->lock); + if (!priv->conn) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("connection not ope= n")); goto cleanup; } =20 - virMutexLock(&priv->lock); - for (i =3D 0; i < priv->nqemuEventCallbacks; i++) { if (priv->qemuEventCallbacks[i]->callbackID =3D=3D args->callbackI= D) break; --=20 2.21.0 -- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list