From nobody Wed May 15 13:29:16 2024 Delivered-To: importer@patchew.org Received-SPF: none (zohomail.com: 8.43.85.245 is neither permitted nor denied by domain of lists.libvirt.org) client-ip=8.43.85.245; envelope-from=devel-bounces@lists.libvirt.org; helo=lists.libvirt.org; Authentication-Results: mx.zohomail.com; spf=none (zohomail.com: 8.43.85.245 is neither permitted nor denied by domain of lists.libvirt.org) smtp.mailfrom=devel-bounces@lists.libvirt.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.libvirt.org (lists.libvirt.org [8.43.85.245]) by mx.zohomail.com with SMTPS id 1705504575118425.41118823455383; Wed, 17 Jan 2024 07:16:15 -0800 (PST) Received: by lists.libvirt.org (Postfix, from userid 996) id 212B91EEE; Wed, 17 Jan 2024 10:16:14 -0500 (EST) Received: from lists.libvirt.org.85.43.8.in-addr.arpa (localhost [IPv6:::1]) by lists.libvirt.org (Postfix) with ESMTP id 562B81EDF; Wed, 17 Jan 2024 10:13:47 -0500 (EST) Received: by lists.libvirt.org (Postfix, from userid 996) id 6E62C1D58; Wed, 17 Jan 2024 10:13:35 -0500 (EST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.libvirt.org (Postfix) with ESMTPS id DD2071D0A for ; Wed, 17 Jan 2024 10:13:34 -0500 (EST) Received: from mimecast-mx02.redhat.com (mimecast-mx02.redhat.com [66.187.233.88]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-570-s-AYZ_AQPYuOa9FSLOwG1A-1; Wed, 17 Jan 2024 10:13:33 -0500 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 120F2837189 for ; Wed, 17 Jan 2024 15:13:33 +0000 (UTC) Received: from speedmetal.lan (unknown [10.45.242.25]) by smtp.corp.redhat.com (Postfix) with ESMTP id 795D31C066AB for ; Wed, 17 Jan 2024 15:13:32 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on lists.libvirt.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H3, RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.4 X-MC-Unique: s-AYZ_AQPYuOa9FSLOwG1A-1 From: Peter Krempa To: devel@lists.libvirt.org Subject: [PATCH 1/2] reproducer Date: Wed, 17 Jan 2024 16:13:29 +0100 Message-ID: In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.7 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Message-ID-Hash: LLAX6GDNVK7YOW6XRI56LAZUNJQKW276 X-Message-ID-Hash: LLAX6GDNVK7YOW6XRI56LAZUNJQKW276 X-MailFrom: pkrempa@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-config-1; header-match-config-2; header-match-config-3; header-match-devel.lists.libvirt.org-0; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; suspicious-header X-Mailman-Version: 3.2.2 Precedence: list List-Id: Development discussions about the libvirt library & tools Archived-At: List-Archive: List-Help: List-Post: List-Subscribe: List-Unsubscribe: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-ZM-MESSAGEID: 1705504577010100001 To reproduce the issue, start virtqemud with polkit auth enabled while looking at the logs. Run a virsh command which will ask for authentication. After providing the password CTRL+C the virsh instance. Any subsequent connection will get stuck --- src/rpc/virnetserver.c | 26 ++++++++++++++++++++++++-- src/rpc/virnetserverclient.c | 2 ++ 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/src/rpc/virnetserver.c b/src/rpc/virnetserver.c index a6c6443c55..f5f50464a2 100644 --- a/src/rpc/virnetserver.c +++ b/src/rpc/virnetserver.c @@ -821,8 +821,30 @@ void virNetServerSetClientAuthenticated(virNetServer *srv, virNetServerClient *client) { - VIR_LOCK_GUARD server_lock =3D virObjectLockGuard(srv); - VIR_LOCK_GUARD client_lock =3D virObjectLockGuard(client); + VIR_LOCK_GUARD server_lock; + VIR_LOCK_GUARD client_lock; + + printf("\n\n\n!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!\n"); + printf(" kill virsh now; waiting 10 s\n"); + printf("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!\n"); + + sleep(10); + + printf("\n\n\n!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!\n"); + printf(" continuing\n"); + printf("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!\n"); + + server_lock =3D virObjectLockGuard(srv); + + printf("\n\n\n!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!\n"); + printf(" srv lokced\n"); + printf("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!\n"); + + client_lock =3D virObjectLockGuard(client); + + printf("\n\n\n!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!\n"); + printf(" client lokced\n"); + printf("!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!\n"); virNetServerClientSetAuthLocked(client, VIR_NET_SERVER_SERVICE_AUTH_NO= NE); virNetServerSetClientAuthCompletedLocked(srv, client); diff --git a/src/rpc/virnetserverclient.c b/src/rpc/virnetserverclient.c index 355aab4b04..bfdf121243 100644 --- a/src/rpc/virnetserverclient.c +++ b/src/rpc/virnetserverclient.c @@ -1013,6 +1013,8 @@ virNetServerClientCloseLocked(virNetServerClient *cli= ent) if (client->sock) { g_clear_pointer(&client->sock, virObjectUnref); } + + VIR_DEBUG("client closed"); } --=20 2.43.0 _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org From nobody Wed May 15 13:29:16 2024 Delivered-To: importer@patchew.org Received-SPF: none (zohomail.com: 8.43.85.245 is neither permitted nor denied by domain of lists.libvirt.org) client-ip=8.43.85.245; envelope-from=devel-bounces@lists.libvirt.org; helo=lists.libvirt.org; Authentication-Results: mx.zohomail.com; spf=none (zohomail.com: 8.43.85.245 is neither permitted nor denied by domain of lists.libvirt.org) smtp.mailfrom=devel-bounces@lists.libvirt.org; dmarc=fail(p=none dis=none) header.from=redhat.com Return-Path: Received: from lists.libvirt.org (lists.libvirt.org [8.43.85.245]) by mx.zohomail.com with SMTPS id 1705505323616658.7348437301456; Wed, 17 Jan 2024 07:28:43 -0800 (PST) Received: by lists.libvirt.org (Postfix, from userid 996) id 57B211F0F; Wed, 17 Jan 2024 10:28:42 -0500 (EST) Received: from lists.libvirt.org.85.43.8.in-addr.arpa (localhost [IPv6:::1]) by lists.libvirt.org (Postfix) with ESMTP id C69F21DB2; Wed, 17 Jan 2024 10:23:46 -0500 (EST) Received: by lists.libvirt.org (Postfix, from userid 996) id 2EDAB1DBB; Wed, 17 Jan 2024 10:23:34 -0500 (EST) Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by lists.libvirt.org (Postfix) with ESMTPS id 6E00C1F5A for ; Wed, 17 Jan 2024 10:21:52 -0500 (EST) Received: from mimecast-mx02.redhat.com (mx-ext.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-175-61lWiJwRM6evhjJoelNb4w-1; Wed, 17 Jan 2024 10:13:34 -0500 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 21A7038212E5 for ; Wed, 17 Jan 2024 15:13:34 +0000 (UTC) Received: from speedmetal.lan (unknown [10.45.242.25]) by smtp.corp.redhat.com (Postfix) with ESMTP id 6ABDA1C066AB for ; Wed, 17 Jan 2024 15:13:33 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.4.4 (2020-01-24) on lists.libvirt.org X-Spam-Level: X-Spam-Status: No, score=-0.8 required=5.0 tests=HEADER_FROM_DIFFERENT_DOMAINS, MAILING_LIST_MULTI,RCVD_IN_DNSWL_NONE,RCVD_IN_MSPIKE_H4, RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no version=3.4.4 X-MC-Unique: 61lWiJwRM6evhjJoelNb4w-1 From: Peter Krempa To: devel@lists.libvirt.org Subject: [PATCH 2/2] remoteDispatchAuthPolkit: Fix lock ordering deadlock if client closes connection during auth Date: Wed, 17 Jan 2024 16:13:30 +0100 Message-ID: In-Reply-To: References: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 3.4.1 on 10.11.54.7 X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Message-ID-Hash: USPYRTZGRWXRF22U7PWECCA6Y6XTVHTD X-Message-ID-Hash: USPYRTZGRWXRF22U7PWECCA6Y6XTVHTD X-MailFrom: pkrempa@redhat.com X-Mailman-Rule-Misses: dmarc-mitigation; no-senders; approved; emergency; loop; banned-address; member-moderation; header-match-config-1; header-match-config-2; header-match-config-3; header-match-devel.lists.libvirt.org-0; nonmember-moderation; administrivia; implicit-dest; max-recipients; max-size; news-moderation; no-subject; suspicious-header X-Mailman-Version: 3.2.2 Precedence: list List-Id: Development discussions about the libvirt library & tools Archived-At: List-Archive: List-Help: List-Post: List-Subscribe: List-Unsubscribe: Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: quoted-printable X-ZM-MESSAGEID: 1705505324718100001 Locks in following text: A: virNetServer B: virNetServerClient C: daemonClientPrivate 'virNetServerSetClientAuthenticated' locks A then B 'remoteDispatchAuthPolkit' calls 'virNetServerSetClientAuthenticated' while holding C. If a client closes its connection 'virNetServerProcessClients' with the lock A and B locked will call 'virNetServerClientCloseLocked' which will try to dispose of the 'client' private data by: ref(b); unlock(b); remoteClientFreePrivateCallbacks(); lock(b); unref(b); Unfortunately remoteClientFreePrivateCallbacks() tries lock C. Thus the locks are held in the following order: polkit auth: C -> A connection close: A -> C causing a textbook-example deadlock. To resolve it we can simply drop lock 'C' before calling 'virNetServerSetClientAuthenticated' as the lock is not needed any more. Resolves: https://issues.redhat.com/browse/RHEL-20337 Signed-off-by: Peter Krempa Reviewed-by: Martin Kletzander --- src/remote/remote_daemon_dispatch.c | 76 +++++++++++++++-------------- 1 file changed, 39 insertions(+), 37 deletions(-) diff --git a/src/remote/remote_daemon_dispatch.c b/src/remote/remote_daemon= _dispatch.c index 7daf503b51..aaabd1e56c 100644 --- a/src/remote/remote_daemon_dispatch.c +++ b/src/remote/remote_daemon_dispatch.c @@ -3979,50 +3979,52 @@ remoteDispatchAuthPolkit(virNetServer *server, struct daemonClientPrivate *priv =3D virNetServerClientGetPrivateData(client); int rv; - VIR_LOCK_GUARD lock =3D virLockGuardLock(&priv->lock); - - action =3D virNetServerClientGetReadonly(client) ? - "org.libvirt.unix.monitor" : - "org.libvirt.unix.manage"; - VIR_DEBUG("Start PolicyKit auth %d", virNetServerClientGetFD(client)); - if (virNetServerClientGetAuth(client) !=3D VIR_NET_SERVER_SERVICE_AUTH= _POLKIT) { - VIR_ERROR(_("client tried invalid PolicyKit init request")); - goto authfail; - } + VIR_WITH_MUTEX_LOCK_GUARD(&priv->lock) { + action =3D virNetServerClientGetReadonly(client) ? + "org.libvirt.unix.monitor" : + "org.libvirt.unix.manage"; - if (virNetServerClientGetUNIXIdentity(client, &callerUid, &callerGid, - &callerPid, ×tamp) < 0) { - goto authfail; - } + VIR_DEBUG("Start PolicyKit auth %d", virNetServerClientGetFD(clien= t)); + if (virNetServerClientGetAuth(client) !=3D VIR_NET_SERVER_SERVICE_= AUTH_POLKIT) { + VIR_ERROR(_("client tried invalid PolicyKit init request")); + goto authfail; + } - if (timestamp =3D=3D 0) { - VIR_WARN("Failing polkit auth due to missing client (pid=3D%lld) s= tart time", - (long long)callerPid); - goto authfail; - } + if (virNetServerClientGetUNIXIdentity(client, &callerUid, &callerG= id, + &callerPid, ×tamp) < 0)= { + goto authfail; + } - VIR_INFO("Checking PID %lld running as %d", - (long long) callerPid, callerUid); + if (timestamp =3D=3D 0) { + VIR_WARN("Failing polkit auth due to missing client (pid=3D%ll= d) start time", + (long long)callerPid); + goto authfail; + } - rv =3D virPolkitCheckAuth(action, - callerPid, - timestamp, - callerUid, - NULL, - true); - if (rv =3D=3D -1) - goto authfail; - else if (rv =3D=3D -2) - goto authdeny; + VIR_INFO("Checking PID %lld running as %d", + (long long) callerPid, callerUid); - PROBE(RPC_SERVER_CLIENT_AUTH_ALLOW, - "client=3D%p auth=3D%d identity=3D%s", - client, REMOTE_AUTH_POLKIT, ident); - VIR_INFO("Policy allowed action %s from pid %lld, uid %d", - action, (long long) callerPid, callerUid); - ret->complete =3D 1; + rv =3D virPolkitCheckAuth(action, + callerPid, + timestamp, + callerUid, + NULL, + true); + if (rv =3D=3D -1) + goto authfail; + else if (rv =3D=3D -2) + goto authdeny; + + PROBE(RPC_SERVER_CLIENT_AUTH_ALLOW, + "client=3D%p auth=3D%d identity=3D%s", + client, REMOTE_AUTH_POLKIT, ident); + VIR_INFO("Policy allowed action %s from pid %lld, uid %d", + action, (long long) callerPid, callerUid); + ret->complete =3D 1; + } + /* this must be called with the private data mutex unlocked */ virNetServerSetClientAuthenticated(server, client); return 0; --=20 2.43.0 _______________________________________________ Devel mailing list -- devel@lists.libvirt.org To unsubscribe send an email to devel-leave@lists.libvirt.org