From nobody Tue May 14 13:00:01 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of redhat.com designates 170.10.129.124 as permitted sender) client-ip=170.10.129.124; envelope-from=libvir-list-bounces@redhat.com; helo=us-smtp-delivery-124.mimecast.com; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.129.124 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=1647621211; cv=none; d=zohomail.com; s=zohoarc; b=YzYtYiO/DtAoUOpB8YJxo1XQIdqC7HqkuAOgYr4NQ4MrF0wvI6a0aDmYp/I4GdoI0aYwBbTM9rHrbqS5MU61idbxVG5EAToG8I8nJa6ZCzEgnELdH17A5WPmFXKT/IxCoGb6p4vLMnrZJ7Dnk/d5sCiHIjjMVCrVzw65Y0vYaGA= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1647621211; h=Content-Type:Content-Transfer-Encoding:Date:From:List-Subscribe:List-Post:List-Id:List-Archive:List-Help:List-Unsubscribe:MIME-Version:Message-ID:Sender:Subject:To; bh=dscY40utMCZDU0gFROo1mMA8choFsGoKsUhbCQ6yYag=; b=BL/m1/cb9bw+7bUdCefEUZ6lGqYAwyqzi4YjAyyYVJ4u0ET77cu1OFQUfyRAA9xUFrMta9Zv+s3NHX4qZlskszNwivbHAR28oaGFVJDK0D7zMDyRnlmYTRMN3mALjdbBSHJo6JYSGnsVaCO4ENaKHgL6yIY9zQb/sjhTFWV+6jo= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of redhat.com designates 170.10.129.124 as permitted sender) smtp.mailfrom=libvir-list-bounces@redhat.com; dmarc=pass header.from= (p=none dis=none) Return-Path: Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.129.124]) by mx.zohomail.com with SMTPS id 1647621211814995.4171889323569; Fri, 18 Mar 2022 09:33:31 -0700 (PDT) Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-347-qvqG6KTRO2S142hne0IWug-1; Fri, 18 Mar 2022 12:33:29 -0400 Received: from smtp.corp.redhat.com (int-mx07.intmail.prod.int.rdu2.redhat.com [10.11.54.7]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 48867296A620; Fri, 18 Mar 2022 16:33:27 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com [10.30.29.100]) by smtp.corp.redhat.com (Postfix) with ESMTP id 0C8FB1463EC9; Fri, 18 Mar 2022 16:33:24 +0000 (UTC) Received: from mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (localhost [IPv6:::1]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id 8C972194035B; Fri, 18 Mar 2022 16:33:24 +0000 (UTC) Received: from smtp.corp.redhat.com (int-mx09.intmail.prod.int.rdu2.redhat.com [10.11.54.9]) by mm-prod-listman-01.mail-001.prod.us-east-1.aws.redhat.com (Postfix) with ESMTP id D4DA7194034E for ; Fri, 18 Mar 2022 16:33:22 +0000 (UTC) Received: by smtp.corp.redhat.com (Postfix) id AEA034B6133; Fri, 18 Mar 2022 16:33:22 +0000 (UTC) Received: from maggie.redhat.com (unknown [10.43.2.152]) by smtp.corp.redhat.com (Postfix) with ESMTP id 568114B6128 for ; Fri, 18 Mar 2022 16:33:22 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1647621210; h=from:from:sender:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding:list-id:list-help: list-unsubscribe:list-subscribe:list-post; bh=dscY40utMCZDU0gFROo1mMA8choFsGoKsUhbCQ6yYag=; b=GUeG6NLKa0YRtpw81HJ6xNmqg9lTaD/eO5zX5+cUtlX479P71zvNjhhbdNp/BwVT9ITPcG 1vQoVOrDa5UP7H62CUPZ4ienwhpBkhnOb+fX4rWoI5jAoUqj7xkP1SW0glbMkLIuuoTGRC 6VLwwsud+tPHvgRS4VED5h8iL1RCJ7I= X-MC-Unique: qvqG6KTRO2S142hne0IWug-1 X-Original-To: libvir-list@listman.corp.redhat.com From: Michal Privoznik To: libvir-list@redhat.com Subject: [PATCH] virNWFilterLockIface: Preserve correct lock ordering Date: Fri, 18 Mar 2022 17:33:20 +0100 Message-Id: MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.85 on 10.11.54.9 X-BeenThere: libvir-list@redhat.com X-Mailman-Version: 2.1.29 Precedence: list List-Id: Development discussions about the libvirt library & tools List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: libvir-list-bounces@redhat.com Sender: "libvir-list" X-Scanned-By: MIMEDefang 2.85 on 10.11.54.7 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=libvir-list-bounces@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Transfer-Encoding: quoted-printable X-ZohoMail-DKIM: pass (identity @redhat.com) X-ZM-MESSAGEID: 1647621213060100001 Content-Type: text/plain; charset="utf-8" In the not so distant past, the lock ordering in virNWFilterLockIface() was as follows: global mutex ifaceMapLock was acquired, then internal representation of given interface was looked up in a hash table (or created brand new if none was found), the global lock was released and the lock of the interface was acquired. But this was mistakenly changed as the function was rewritten to use automatic mutexes, because now the global lock is held throughout the whole run of the function and thus the interface specific lock is acquired with the global lock held. This results in a deadlock. Fixes: dd8150c48dcf94e8d3b0481be08eeef822b98b02 Signed-off-by: Michal Privoznik Reviewed-by: Erik Skultety Tested-by: Erik Skultety --- src/nwfilter/nwfilter_learnipaddr.c | 49 +++++++++++++++-------------- 1 file changed, 26 insertions(+), 23 deletions(-) diff --git a/src/nwfilter/nwfilter_learnipaddr.c b/src/nwfilter/nwfilter_le= arnipaddr.c index 2c85972012..ec2d337188 100644 --- a/src/nwfilter/nwfilter_learnipaddr.c +++ b/src/nwfilter/nwfilter_learnipaddr.c @@ -143,37 +143,40 @@ static bool threadsTerminate; int virNWFilterLockIface(const char *ifname) { - VIR_LOCK_GUARD lock =3D virLockGuardLock(&ifaceMapLock); - virNWFilterIfaceLock *ifaceLock =3D virHashLookup(ifaceLockMap, ifname= ); + virNWFilterIfaceLock *ifaceLock =3D NULL; =20 - if (!ifaceLock) { - ifaceLock =3D g_new0(virNWFilterIfaceLock, 1); + VIR_WITH_MUTEX_LOCK_GUARD(&ifaceMapLock) { + ifaceLock =3D virHashLookup(ifaceLockMap, ifname); =20 - if (virMutexInitRecursive(&ifaceLock->lock) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, "%s", - _("mutex initialization failed")); - g_free(ifaceLock); - return -1; - } + if (!ifaceLock) { + ifaceLock =3D g_new0(virNWFilterIfaceLock, 1); =20 - if (virStrcpyStatic(ifaceLock->ifname, ifname) < 0) { - virReportError(VIR_ERR_INTERNAL_ERROR, - _("interface name %s does not fit into buffer"), - ifaceLock->ifname); - g_free(ifaceLock); - return -1; - } + if (virMutexInitRecursive(&ifaceLock->lock) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, "%s", + _("mutex initialization failed")); + g_free(ifaceLock); + return -1; + } + + if (virStrcpyStatic(ifaceLock->ifname, ifname) < 0) { + virReportError(VIR_ERR_INTERNAL_ERROR, + _("interface name %s does not fit into buff= er"), + ifaceLock->ifname); + g_free(ifaceLock); + return -1; + } + + while (virHashAddEntry(ifaceLockMap, ifname, ifaceLock)) { + g_free(ifaceLock); + return -1; + } =20 - while (virHashAddEntry(ifaceLockMap, ifname, ifaceLock)) { - g_free(ifaceLock); - return -1; + ifaceLock->refctr =3D 0; } =20 - ifaceLock->refctr =3D 0; + ifaceLock->refctr++; } =20 - ifaceLock->refctr++; - virMutexLock(&ifaceLock->lock); =20 return 0; --=20 2.34.1