From nobody Wed Apr 24 01:06:47 2024 Delivered-To: importer@patchew.org Received-SPF: pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) client-ip=66.175.222.108; envelope-from=bounce+27952+69672+1787277+3901457@groups.io; helo=mail02.groups.io; Authentication-Results: mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+69672+1787277+3901457@groups.io; dmarc=fail(p=none dis=none) header.from=intel.com ARC-Seal: i=1; a=rsa-sha256; t=1609823145; cv=none; d=zohomail.com; s=zohoarc; b=f9kylM7sdduXlTQOMXiQnVofIqSk95lLenVxoi3tdZncq4Ljb+JXaeo9l078ihp4iq5Xif793rpv6qtjJLWD2I1QSeSNMLFNrpEPXzvum0kTXTLmiLiklZVaA8Xbcow8by0leiDH1Umu+Q68u16J9v/3Jn0ZDk32CXMFIIZhR2s= ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=zohomail.com; s=zohoarc; t=1609823145; h=Content-Transfer-Encoding:Cc:Date:From:In-Reply-To:List-Id:List-Unsubscribe:MIME-Version:Message-ID:Reply-To:References:Sender:Subject:To; bh=jtl7cEZ+1Gw0b4QzuAXp6QSZdKfOp+joZEZwH9j1aVE=; b=cMIEBgpRZ2lIRejjr4BqJ9K73F4LvCE8AAe35FHCh9OW3qJRBoY6V9yW4mWlgyNtqhT/eWrDyKaQBHOkLNGDlYVh24juuMhk9hpKYAz3muvH1q6X3cia/UCObXgUkFyPtCtk3dbard4EIfe3Mjqk94m9thh1vm0mZoXcQs6jUgs= ARC-Authentication-Results: i=1; mx.zohomail.com; dkim=pass; spf=pass (zohomail.com: domain of groups.io designates 66.175.222.108 as permitted sender) smtp.mailfrom=bounce+27952+69672+1787277+3901457@groups.io; dmarc=fail header.from= (p=none dis=none) header.from= Received: from mail02.groups.io (mail02.groups.io [66.175.222.108]) by mx.zohomail.com with SMTPS id 160982314514444.39784614796952; Mon, 4 Jan 2021 21:05:45 -0800 (PST) Return-Path: X-Received: by 127.0.0.2 with SMTP id oLRoYY1788612x9VA8n1snHT; Mon, 04 Jan 2021 21:05:44 -0800 X-Received: from mga05.intel.com (mga05.intel.com [192.55.52.43]) by mx.groups.io with SMTP id smtpd.web08.1450.1609823138568417608 for ; Mon, 04 Jan 2021 21:05:39 -0800 IronPort-SDR: JfZQ6io2iTtPAWZFYAWjQzhh7TFc2TRzctpw7YxqmJJeta2N4B6T72STpKdVcgjWd8zgSQrBSY fHqlqxxi0ULw== X-IronPort-AV: E=McAfee;i="6000,8403,9854"; a="261816971" X-IronPort-AV: E=Sophos;i="5.78,476,1599548400"; d="scan'208";a="261816971" X-Received: from orsmga008.jf.intel.com ([10.7.209.65]) by fmsmga105.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Jan 2021 21:05:31 -0800 IronPort-SDR: K7lrXjmBQzhWgNucQoasDZWI2prWj9dqNBAOuSUtJEeLWNX0vfq4BRs+C4+Jq+WGtfpkrAjC57 kRXG4UARzEeA== X-IronPort-AV: E=Sophos;i="5.78,476,1599548400"; d="scan'208";a="378720574" X-Received: from mdkinney-mobl2.amr.corp.intel.com ([10.254.77.44]) by orsmga008-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 04 Jan 2021 21:05:31 -0800 From: "Michael D Kinney" To: devel@edk2.groups.io Cc: Bret Barkelew , Hao A Wu , Liming Gao , Bret Barkelew Subject: [edk2-devel] [stable/202011][Patch 1/2] MdeModulePkg/Variable/RuntimeDxe: Restore Variable Lock Protocol behavior Date: Mon, 4 Jan 2021 21:05:21 -0800 Message-Id: <20210105050522.1178-2-michael.d.kinney@intel.com> In-Reply-To: <20210105050522.1178-1-michael.d.kinney@intel.com> References: <20210105050522.1178-1-michael.d.kinney@intel.com> MIME-Version: 1.0 Precedence: Bulk List-Unsubscribe: Sender: devel@edk2.groups.io List-Id: Mailing-List: list devel@edk2.groups.io; contact devel+owner@edk2.groups.io Reply-To: devel@edk2.groups.io,michael.d.kinney@intel.com X-Gm-Message-State: 3YMruKnOB3UiXUoSbxXmgW5Yx1787277AA= Content-Transfer-Encoding: quoted-printable DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=groups.io; q=dns/txt; s=20140610; t=1609823144; bh=JLZqxk+aoeFSkLhD8bQOCYh/+nZCXConwJt0ZevfO3k=; h=Cc:Date:From:Reply-To:Subject:To; b=a+zJGHDPpy/mdkqNe7JohQ9xbQEevM19UhZcaYxgIzBSXUOF4rvHLkyEbocqy1iLgFi rF+g14C8SOYDSDUr2dND3kQI3iN1Pi9tRt4gXFgtDU+hIYbEuC6JIlG+NKf5xnwkQQNvY gTaiGjaazsraHFB0uMIlbryGRr8/TjedBbQ= X-ZohoMail-DKIM: pass (identity @groups.io) Content-Type: text/plain; charset="utf-8" From: Bret Barkelew https://bugzilla.tianocore.org/show_bug.cgi?id=3D3111 The VariableLock shim currently fails if called twice because the underlying Variable Policy engine returns an error if a policy is set on an existing variable. This breaks existing code which expect it to silently pass if a variable is locked multiple times (because it should "be locked"). Refactor the shim to confirm that the variable is indeed locked and then change the error to EFI_SUCCESS and generate a DEBUG_ERROR message so the duplicate lock can be reported in a debug log and removed. Cc: Michael D Kinney Cc: Hao A Wu Cc: Liming Gao Signed-off-by: Bret Barkelew Reviewed-by: Hao A Wu Reviewed-by: Liming Gao Reviewed-by: Michael D Kinney (cherry picked from commit a18a9bde36d2ffc12df29cdced1efa1f8f9f2021) --- .../RuntimeDxe/VariableLockRequestToLock.c | 95 ++++++++++++------- 1 file changed, 59 insertions(+), 36 deletions(-) diff --git a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequest= ToLock.c b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestTo= Lock.c index 4aa854aaf260..7d87e50efdcd 100644 --- a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c +++ b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLock.c @@ -1,67 +1,90 @@ -/** @file -- VariableLockRequestToLock.c -Temporary location of the RequestToLock shim code while -projects are moved to VariablePolicy. Should be removed when deprecated. +/** @file + Temporary location of the RequestToLock shim code while projects + are moved to VariablePolicy. Should be removed when deprecated. =20 -Copyright (c) Microsoft Corporation. -SPDX-License-Identifier: BSD-2-Clause-Patent + Copyright (c) Microsoft Corporation. + SPDX-License-Identifier: BSD-2-Clause-Patent =20 **/ =20 #include - #include #include - -#include - -#include #include #include - +#include =20 /** DEPRECATED. THIS IS ONLY HERE AS A CONVENIENCE WHILE PORTING. - Mark a variable that will become read-only after leaving the DXE phase o= f execution. - Write request coming from SMM environment through EFI_SMM_VARIABLE_PROTO= COL is allowed. + Mark a variable that will become read-only after leaving the DXE phase of + execution. Write request coming from SMM environment through + EFI_SMM_VARIABLE_PROTOCOL is allowed. =20 @param[in] This The VARIABLE_LOCK_PROTOCOL instance. - @param[in] VariableName A pointer to the variable name that will be mad= e read-only subsequently. - @param[in] VendorGuid A pointer to the vendor GUID that will be made = read-only subsequently. + @param[in] VariableName A pointer to the variable name that will be made + read-only subsequently. + @param[in] VendorGuid A pointer to the vendor GUID that will be made + read-only subsequently. =20 - @retval EFI_SUCCESS The variable specified by the VariableName= and the VendorGuid was marked - as pending to be read-only. + @retval EFI_SUCCESS The variable specified by the VariableName= and + the VendorGuid was marked as pending to be + read-only. @retval EFI_INVALID_PARAMETER VariableName or VendorGuid is NULL. Or VariableName is an empty string. - @retval EFI_ACCESS_DENIED EFI_END_OF_DXE_EVENT_GROUP_GUID or EFI_EVE= NT_GROUP_READY_TO_BOOT has - already been signaled. - @retval EFI_OUT_OF_RESOURCES There is not enough resource to hold the l= ock request. + @retval EFI_ACCESS_DENIED EFI_END_OF_DXE_EVENT_GROUP_GUID or + EFI_EVENT_GROUP_READY_TO_BOOT has already = been + signaled. + @retval EFI_OUT_OF_RESOURCES There is not enough resource to hold the l= ock + request. **/ EFI_STATUS EFIAPI VariableLockRequestToLock ( - IN CONST EDKII_VARIABLE_LOCK_PROTOCOL *This, - IN CHAR16 *VariableName, - IN EFI_GUID *VendorGuid + IN CONST EDKII_VARIABLE_LOCK_PROTOCOL *This, + IN CHAR16 *VariableName, + IN EFI_GUID *VendorGuid ) { - EFI_STATUS Status; - VARIABLE_POLICY_ENTRY *NewPolicy; + EFI_STATUS Status; + VARIABLE_POLICY_ENTRY *NewPolicy; + + DEBUG ((DEBUG_ERROR, "!!! DEPRECATED INTERFACE !!! %a() will go away soo= n!\n", __FUNCTION__)); + DEBUG ((DEBUG_ERROR, "!!! DEPRECATED INTERFACE !!! Please move to use Va= riable Policy!\n")); + DEBUG ((DEBUG_ERROR, "!!! DEPRECATED INTERFACE !!! Variable: %g %s\n", V= endorGuid, VariableName)); =20 NewPolicy =3D NULL; - Status =3D CreateBasicVariablePolicy( VendorGuid, - VariableName, - VARIABLE_POLICY_NO_MIN_SIZE, - VARIABLE_POLICY_NO_MAX_SIZE, - VARIABLE_POLICY_NO_MUST_ATTR, - VARIABLE_POLICY_NO_CANT_ATTR, - VARIABLE_POLICY_TYPE_LOCK_NOW, - &NewPolicy ); + Status =3D CreateBasicVariablePolicy( + VendorGuid, + VariableName, + VARIABLE_POLICY_NO_MIN_SIZE, + VARIABLE_POLICY_NO_MAX_SIZE, + VARIABLE_POLICY_NO_MUST_ATTR, + VARIABLE_POLICY_NO_CANT_ATTR, + VARIABLE_POLICY_TYPE_LOCK_NOW, + &NewPolicy + ); if (!EFI_ERROR( Status )) { - Status =3D RegisterVariablePolicy( NewPolicy ); + Status =3D RegisterVariablePolicy (NewPolicy); + + // + // If the error returned is EFI_ALREADY_STARTED, we need to check the + // current database for the variable and see whether it's locked. If i= t's + // locked, we're still fine, but also generate a DEBUG_ERROR message s= o the + // duplicate lock can be removed. + // + if (Status =3D=3D EFI_ALREADY_STARTED) { + Status =3D ValidateSetVariable (VariableName, VendorGuid, 0, 0, NULL= ); + if (Status =3D=3D EFI_WRITE_PROTECTED) { + DEBUG ((DEBUG_ERROR, " Variable: %g %s is already locked!\n", Ven= dorGuid, VariableName)); + Status =3D EFI_SUCCESS; + } else { + DEBUG ((DEBUG_ERROR, " Variable: %g %s can not be locked!\n", Ven= dorGuid, VariableName)); + Status =3D EFI_ACCESS_DENIED; + } + } } - if (EFI_ERROR( Status )) { + if (EFI_ERROR (Status)) { DEBUG(( DEBUG_ERROR, "%a - Failed to lock variable %s! %r\n", __FUNCTI= ON__, VariableName, Status )); - ASSERT_EFI_ERROR( Status ); } if (NewPolicy !=3D NULL) { FreePool( NewPolicy ); --=20 2.29.2.windows.2 -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D- Groups.io Links: You receive all messages sent to this group. View/Reply Online (#69672): https://edk2.groups.io/g/devel/message/69672 Mute This Topic: https://groups.io/mt/79444819/1787277 Group Owner: devel+owner@edk2.groups.io Unsubscribe: https://edk2.groups.io/g/devel/unsub [importer@patchew.org] -=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-=3D-