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.web11.1392.1608167686368996744 for ; Wed, 16 Dec 2020 17:14:47 -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 ; Thu, 17 Dec 2020 09:14:38 +0800 X-WM-Sender: gaoliming@byosoft.com.cn X-WM-AuthFlag: YES X-WM-AuthUser: gaoliming@byosoft.com.cn From: "gaoliming" To: "'Masahisa Kojima'" , , Cc: "'Kun Qin'" , "'Jian J Wang'" , "'Hao A Wu'" , "'Ard Biesheuvel'" , "'Sami Mujawar'" , "'Jiewen Yao'" , "'Supreeth Venkatesh'" , "'Bret Barkelew'" References: <20201216141919.23262-1-masahisa.kojima@linaro.org> <20201216141919.23262-2-masahisa.kojima@linaro.org> In-Reply-To: <20201216141919.23262-2-masahisa.kojima@linaro.org> Subject: =?UTF-8?B?5Zue5aSNOiBbUEFUQ0ggMS8xXSBNZGVNb2R1bGVQa2cvVmFyQ2hlY2tQb2xpY3lMaWI6IGltcGxlbWVudCBzdGFuZGFsb25lIE1NIHZlcnNpb24=?= Date: Thu, 17 Dec 2020 09:14:41 +0800 Message-ID: <002001d6d411$ff6a2fd0$fe3e8f70$@byosoft.com.cn> MIME-Version: 1.0 X-Mailer: Microsoft Outlook 16.0 Thread-Index: AQIsTiDj8DCaOFji41OQNzs0OfaOJAJMIgpRqT0loJA= Content-Type: text/plain; charset="gb2312" Content-Transfer-Encoding: quoted-printable Content-Language: zh-cn Masahisa: The patch is good. Reviewed-by: Liming Gao =20 Now, Mike proposes to create stable tag branch to include the critical = bug fix. I think this one is also the critical fix to be cherry-pick to the stable tag branch.=20 Thanks Liming > -----=D3=CA=BC=FE=D4=AD=BC=FE----- > =B7=A2=BC=FE=C8=CB: Masahisa Kojima > =B7=A2=CB=CD=CA=B1=BC=E4: 2020=C4=EA12=D4=C216=C8=D5 22:19 > =CA=D5=BC=FE=C8=CB: devel@edk2.groups.io > =B3=AD=CB=CD: Kun Qin ; Masahisa Kojima > ; Jian J Wang ; Hao = A > Wu ; Liming Gao ; Ard > Biesheuvel ; Sami Mujawar > ; Jiewen Yao ; Supreeth > Venkatesh ; Bret Barkelew > > =D6=F7=CC=E2: [PATCH 1/1] MdeModulePkg/VarCheckPolicyLib: implement = standalone > MM version >=20 > This commit adds the VarCheckPolicyLib that will be able to > execute in the context of standalone MM. >=20 > Signed-off-by: Masahisa Kojima > Co-authored-by: Kun Qin > Cc: Jian J Wang > Cc: Hao A Wu > Cc: Liming Gao > Cc: Ard Biesheuvel > Cc: Sami Mujawar > Cc: Jiewen Yao > Cc: Supreeth Venkatesh > Cc: Bret Barkelew > --- > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > | 5 +- > MdeModulePkg/Library/VarCheckPolicyLib/{VarCheckPolicyLib.inf =3D> > VarCheckPolicyLibStandaloneMm.inf} | 23 +++++---- > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.h > | 42 ++++++++++++++++ > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c > | 14 +++--- >=20 > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibStandaloneMm > .c | 50 ++++++++++++++++++++ > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibTraditional.c > | 50 ++++++++++++++++++++ > 6 files changed, 165 insertions(+), 19 deletions(-) >=20 > diff --git = a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > index 077bcc8990ca..9af436d25f81 100644 > --- a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > +++ b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > @@ -13,11 +13,13 @@ [Defines] > MODULE_TYPE =3D DXE_RUNTIME_DRIVER > VERSION_STRING =3D 1.0 > LIBRARY_CLASS =3D NULL|DXE_RUNTIME_DRIVER > DXE_SMM_DRIVER > - CONSTRUCTOR =3D VarCheckPolicyLibConstructor > + CONSTRUCTOR =3D > VarCheckPolicyLibTraditionalConstructor >=20 >=20 > [Sources] > VarCheckPolicyLib.c > + VarCheckPolicyLibTraditional.c > + VarCheckPolicyLib.h >=20 >=20 > [Packages] > @@ -29,7 +31,6 @@ [LibraryClasses] > BaseLib > DebugLib > BaseMemoryLib > - DxeServicesLib > MemoryAllocationLib > VarCheckLib > VariablePolicyLib > diff --git = a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibStandaloneM > m.inf > similarity index 51% > copy from MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > copy to > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibStandaloneMm > .inf > index 077bcc8990ca..ab427f189a3d 100644 > --- a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.inf > +++ > b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibStandaloneM > m.inf > @@ -1,35 +1,41 @@ > -## @file VarCheckPolicyLib.inf > +## @file VarCheckPolicyLibStandaloneMm.inf > # This is an instance of a VarCheck lib that leverages the business = logic > behind > # the VariablePolicy code to make its decisions. > # > -# Copyright (c) Microsoft Corporation. > +## > +# Copyright (c) Microsoft Corporation. All rights reserved. > # SPDX-License-Identifier: BSD-2-Clause-Patent > +# > ## >=20 > [Defines] > INF_VERSION =3D 0x00010005 > - BASE_NAME =3D VarCheckPolicyLib > - FILE_GUID =3D > 9C28A48F-C884-4B1F-8B95-DEF125448023 > - MODULE_TYPE =3D DXE_RUNTIME_DRIVER > + BASE_NAME =3D > VarCheckPolicyLibStandaloneMm > + FILE_GUID =3D > 44B09E3D-5EDA-4673-ABCF-C8AE4560C8EC > + MODULE_TYPE =3D MM_STANDALONE > + PI_SPECIFICATION_VERSION =3D 0x00010032 > VERSION_STRING =3D 1.0 > - LIBRARY_CLASS =3D NULL|DXE_RUNTIME_DRIVER > DXE_SMM_DRIVER > - CONSTRUCTOR =3D VarCheckPolicyLibConstructor > + LIBRARY_CLASS =3D NULL|MM_STANDALONE > + CONSTRUCTOR =3D > VarCheckPolicyLibStandaloneConstructor >=20 >=20 > [Sources] > VarCheckPolicyLib.c > + VarCheckPolicyLibStandaloneMm.c > + VarCheckPolicyLib.h >=20 >=20 > [Packages] > MdePkg/MdePkg.dec > MdeModulePkg/MdeModulePkg.dec > + StandaloneMmPkg/StandaloneMmPkg.dec >=20 >=20 > [LibraryClasses] > BaseLib > DebugLib > BaseMemoryLib > - DxeServicesLib > + MemLib > MemoryAllocationLib > VarCheckLib > VariablePolicyLib > @@ -37,6 +43,5 @@ [LibraryClasses] > SafeIntLib > MmServicesTableLib >=20 > - > [Guids] > gVarCheckPolicyLibMmiHandlerGuid ## CONSUME ## Used to > register for MM Communication events. > diff --git = a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.h > b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.h > new file mode 100644 > index 000000000000..2226c8a19fec > --- /dev/null > +++ b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.h > @@ -0,0 +1,42 @@ > +/** @file -- VarCheckPolicyLib.h > +This internal header file defines the common interface of constructor = for > +VarCheckPolicyLib. > + > +Copyright (c) Microsoft Corporation. All rights reserved. > +SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#ifndef _VAR_CHECK_POLICY_LIB_H_ > +#define _VAR_CHECK_POLICY_LIB_H_ > + > +/** > + Common constructor function of VarCheckPolicyLib to register = VarCheck > handler > + and SW MMI handlers. > + > + @retval EFI_SUCCESS The constructor executed correctly. > + > +**/ > +EFI_STATUS > +EFIAPI > +VarCheckPolicyLibCommonConstructor ( > + VOID > + ); > + > +/** > + This function is wrapper function to validate the buffer. > + > + @param Buffer The buffer start address to be checked. > + @param Length The buffer length to be checked. > + > + @retval TRUE This buffer is valid per processor architecture and = not > overlap with SMRAM/MMRAM. > + @retval FALSE This buffer is not valid per processor architecture = or > overlap with SMRAM/MMRAM. > +**/ > +BOOLEAN > +EFIAPI > +VarCheckPolicyIsBufferOutsideValid ( > + IN EFI_PHYSICAL_ADDRESS Buffer, > + IN UINT64 Length > + ); > + > +#endif // _VAR_CHECK_POLICY_LIB_H_ > diff --git = a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c > b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c > index 257aa9591303..14e1904e96d3 100644 > --- a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c > +++ b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLib.c > @@ -12,7 +12,6 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > #include > #include > #include > -#include > #include > #include >=20 > @@ -23,6 +22,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent >=20 > #include >=20 > +#include "VarCheckPolicyLib.h" > + > = //=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D > // As a VarCheck library, we're linked into the VariableServices > // and may not be able to call them indirectly. To get around this, > @@ -102,7 +103,8 @@ VarCheckPolicyLibMmiHandler ( > // Make sure that the buffer does not overlap SMM. > // This should be covered by the SmiManage infrastructure, but just = to be > safe... > InternalCommBufferSize =3D *CommBufferSize; > - if (InternalCommBufferSize > > VAR_CHECK_POLICY_MM_COMM_BUFFER_SIZE > || !SmmIsBufferOutsideSmmValid((UINTN)CommBuffer, > (UINT64)InternalCommBufferSize)) { > + if (InternalCommBufferSize > > VAR_CHECK_POLICY_MM_COMM_BUFFER_SIZE || > + !VarCheckPolicyIsBufferOutsideValid((UINTN)CommBuffer, > (UINT64)InternalCommBufferSize)) { > DEBUG ((DEBUG_ERROR, "%a - Invalid CommBuffer supplied! > 0x%016lX[0x%016lX]\n", __FUNCTION__, CommBuffer, > InternalCommBufferSize)); > return EFI_INVALID_PARAMETER; > } > @@ -305,17 +307,13 @@ VarCheckPolicyLibMmiHandler ( > Constructor function of VarCheckPolicyLib to register VarCheck = handler > and > SW MMI handlers. >=20 > - @param[in] ImageHandle The firmware allocated handle for the EFI > image. > - @param[in] SystemTable A pointer to the EFI System Table. > - > @retval EFI_SUCCESS The constructor executed correctly. >=20 > **/ > EFI_STATUS > EFIAPI > -VarCheckPolicyLibConstructor ( > - IN EFI_HANDLE ImageHandle, > - IN EFI_SYSTEM_TABLE *SystemTable > +VarCheckPolicyLibCommonConstructor ( > + VOID > ) > { > EFI_STATUS Status; > diff --git > a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibStandaloneM > m.c > b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibStandaloneM > m.c > new file mode 100644 > index 000000000000..b283ced9d4e3 > --- /dev/null > +++ > b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibStandaloneM > m.c > @@ -0,0 +1,50 @@ > +/** @file -- VarCheckPolicyLibStandaloneMm.c > +This is an instance of a VarCheck lib constructor for Standalone MM. > + > +Copyright (c) Microsoft Corporation. All rights reserved. > +SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include > + > +#include "VarCheckPolicyLib.h" > + > +/** > + Standalone MM constructor function of VarCheckPolicyLib to invoke > common > + constructor routine. > + > + @param[in] ImageHandle The firmware allocated handle for the EFI > image. > + @param[in] SystemTable A pointer to the EFI System Table. > + > + @retval EFI_SUCCESS The constructor executed correctly. > + > +**/ > +EFI_STATUS > +EFIAPI > +VarCheckPolicyLibStandaloneConstructor ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_MM_SYSTEM_TABLE *SystemTable > + ) > +{ > + return VarCheckPolicyLibCommonConstructor (); > +} > + > +/** > + This function is wrapper function to validate the buffer. > + > + @param Buffer The buffer start address to be checked. > + @param Length The buffer length to be checked. > + > + @retval TRUE This buffer is valid per processor architectureand = not > overlap with MMRAM. > + @retval FALSE This buffer is not valid per processor architecture = or > overlap with MMRAM. > +**/ > +BOOLEAN > +EFIAPI > +VarCheckPolicyIsBufferOutsideValid ( > + IN EFI_PHYSICAL_ADDRESS Buffer, > + IN UINT64 Length > + ) > +{ > + return MmIsBufferOutsideMmValid (Buffer, Length); > +} > diff --git > = a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibTraditional.c > = b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibTraditional.c > new file mode 100644 > index 000000000000..f404aaaa470c > --- /dev/null > +++ > = b/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibTraditional.c > @@ -0,0 +1,50 @@ > +/** @file -- VarCheckPolicyLibTraditional.c > +This is an instance of a VarCheck lib constructor for traditional = SMM. > + > +Copyright (c) Microsoft Corporation. All rights reserved. > +SPDX-License-Identifier: BSD-2-Clause-Patent > + > +**/ > + > +#include > + > +#include "VarCheckPolicyLib.h" > + > +/** > + Traditional constructor function of VarCheckPolicyLib to invoke = common > + constructor routine. > + > + @param[in] ImageHandle The firmware allocated handle for the EFI > image. > + @param[in] SystemTable A pointer to the EFI System Table. > + > + @retval EFI_SUCCESS The constructor executed correctly. > + > +**/ > +EFI_STATUS > +EFIAPI > +VarCheckPolicyLibTraditionalConstructor ( > + IN EFI_HANDLE ImageHandle, > + IN EFI_SYSTEM_TABLE *SystemTable > + ) > +{ > + return VarCheckPolicyLibCommonConstructor (); > +} > + > +/** > + This function is wrapper function to validate the buffer. > + > + @param Buffer The buffer start address to be checked. > + @param Length The buffer length to be checked. > + > + @retval TRUE This buffer is valid per processor architecture and = not > overlap with SMRAM. > + @retval FALSE This buffer is not valid per processor architecture = or > overlap with SMRAM. > +**/ > +BOOLEAN > +EFIAPI > +VarCheckPolicyIsBufferOutsideValid ( > + IN EFI_PHYSICAL_ADDRESS Buffer, > + IN UINT64 Length > + ) > +{ > + return SmmIsBufferOutsideSmmValid (Buffer, Length); > +} > -- > 2.17.1