From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail.byosoft.com.cn (mail.byosoft.com.cn [58.240.74.242]) by mx.groups.io with SMTP id smtpd.web12.28577.1607909963352765611 for ; Sun, 13 Dec 2020 17:39:24 -0800 Authentication-Results: mx.groups.io; dkim=missing; spf=none, err=permanent DNS error (domain: byosoft.com.cn, ip: 58.240.74.242, mailfrom: gaoliming@byosoft.com.cn) Received: from DESKTOPS6D0PVI ([58.246.60.130]) (envelope-sender ) by 192.168.6.13 with ESMTP for ; Mon, 14 Dec 2020 09:39:21 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: , Cc: "'Bret Barkelew'" , "'Hao A Wu'" References: <20201211080118.1885-1-michael.d.kinney@intel.com> <20201211080118.1885-2-michael.d.kinney@intel.com> In-Reply-To: <20201211080118.1885-2-michael.d.kinney@intel.com> Subject: =?UTF-8?B?5Zue5aSNOiBbZWRrMi1kZXZlbF0gW1BhdGNoIHY0IDEvMl0gTWRlTW9kdWxlUGtnL1ZhcmlhYmxlL1J1bnRpbWVEeGU6IFJlc3RvcmUgVmFyaWFibGUgTG9jayBQcm90b2NvbCBiZWhhdmlvcg==?= Date: Mon, 14 Dec 2020 09:39:22 +0800 Message-ID: <004801d6d1b9$f2c72260$d8556720$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQIl4wcJKLWfmmDjkbh9mjGnD2xDxAJciI7aqUTJbkA= Content-Type: text/plain; charset="gb2312" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Reviewed-by: Liming Gao > -----=D3=CA=BC=FE=D4=AD=BC=FE----- > =B7=A2=BC=FE=C8=CB: bounce+27952+68702+4905953+8761045@groups.io > =B4=FA=B1=ED Michael D > Kinney > =B7=A2=CB=CD=CA=B1=BC=E4: 2020=C4=EA12=D4=C211=C8=D5 16:01 > =CA=D5=BC=FE=C8=CB: devel@edk2.groups.io > =B3=AD=CB=CD: Bret Barkelew ; Hao A Wu > ; Liming Gao ; Bret > Barkelew > =D6=F7=CC=E2: [edk2-devel] [Patch v4 1/2] MdeModulePkg/Variable/RuntimeD= xe: > Restore Variable Lock Protocol behavior >=20 > From: Bret Barkelew >=20 > https://bugzilla.tianocore.org/show_bug.cgi?id=3D3111 >=20 > 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. >=20 > This breaks existing code which expect it to silently pass if a variable > is locked multiple times (because it should "be locked"). >=20 > 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. >=20 > Cc: Michael D Kinney > Cc: Hao A Wu > Cc: Liming Gao > Signed-off-by: Bret Barkelew > --- > .../RuntimeDxe/VariableLockRequestToLock.c | 95 ++++++++++++------- > 1 file changed, 59 insertions(+), 36 deletions(-) >=20 > diff --git > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLo > ck.c > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLo > ck.c > index 4aa854aaf260..7d87e50efdcd 100644 > --- > a/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLo > ck.c > +++ > b/MdeModulePkg/Universal/Variable/RuntimeDxe/VariableLockRequestToLo > ck.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 phas= e of > execution. > - Write request coming from SMM environment through > EFI_SMM_VARIABLE_PROTOCOL is allowed. > + Mark a variable that will become read-only after leaving the DXE phas= e 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 > made 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_EVENT_GROUP_READY_TO_BOOT has > - already been signaled. > - @retval EFI_OUT_OF_RESOURCES There is not enough resource to hold > the lock 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 lock > + 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 soon!\n", __FUNCTION__)); > + DEBUG ((DEBUG_ERROR, "!!! DEPRECATED INTERFACE !!! Please move to > use Variable Policy!\n")); > + DEBUG ((DEBUG_ERROR, "!!! DEPRECATED INTERFACE !!! > Variable: %g %s\n", VendorGuid, 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 t= he > + // current database for the variable and see whether it's locked. I= f it's > + // locked, we're still fine, but also generate a DEBUG_ERROR messag= e > so 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", > VendorGuid, VariableName)); > + Status =3D EFI_SUCCESS; > + } else { > + DEBUG ((DEBUG_ERROR, " Variable: %g %s can not be > locked!\n", VendorGuid, 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", > __FUNCTION__, VariableName, Status )); > - ASSERT_EFI_ERROR( Status ); > } > if (NewPolicy !=3D NULL) { > FreePool( NewPolicy ); > -- > 2.29.2.windows.2 >=20 >=20 >=20 >=20 >=20