From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-lf1-f49.google.com (mail-lf1-f49.google.com [209.85.167.49]) by mx.groups.io with SMTP id smtpd.web12.35658.1608531005665843540 for ; Sun, 20 Dec 2020 22:10:06 -0800 Authentication-Results: mx.groups.io; dkim=pass header.i=@linaro.org header.s=google header.b=c5XOdfYR; spf=pass (domain: linaro.org, ip: 209.85.167.49, mailfrom: masahisa.kojima@linaro.org) Received: by mail-lf1-f49.google.com with SMTP id o17so20971979lfg.4 for ; Sun, 20 Dec 2020 22:10:05 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc:content-transfer-encoding; bh=ely3sOZbBjJkcGqNrUKO264wLEQDw2HwEc/XmGnF5Sk=; b=c5XOdfYR9C2zaCE0UZ4AADZfeKI2Tga4F5MbzBE19nanER+a6UUDc6RYsx411TRmk1 ltLDcTl2b69eSzU5pEYpLYipkhJqsAXiZjZVUVLqUceKwQ8+28RJ3qxH/CXzCJwRz8UO inx4SmCw2zfybP8XQVSR4Xc4RznOSMEFZM/hvwOmjuONcCzRUNjV3n1mje4EYAOmUvUj AJBikEs70yT1KEn0r1ffw+vEp5lMOEZeJ7wj/gTIRL48EhZtlWIZWAtZe0V/wkamGdzN zC96OHUK5V989gPDIjD3OGXmlCSC8SUs2eOzhWjpQLWM7T3d9t7y2XL90gmbTxKNVcrv SuBw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc:content-transfer-encoding; bh=ely3sOZbBjJkcGqNrUKO264wLEQDw2HwEc/XmGnF5Sk=; b=fOVH5Ah4f63aM0+Pc59jvNYSlBH3bWdC8loeEPjxCnC3FL99G+dD/TjPgMUyB9fQ+h BAPRhA/fQ1shM1wfI7BCAtyui3p2KPW0OLdO74UWbOPxlK5W7boheLyCUk4JRTDIH21C FjIp1hQfrAP1MKoVjJWFosgFZsis7mG1upDxPBfwasbB3Rb5p6rYmIPiLp/21wA8zz6Z K88baH99Hor8QO/NKuc35Ja960W+0Qdi5K6qh5UJlm9GoorojMmfDFTHBEvYhDskAuiO umJbgFoIpcnJxaTMNTG+S1dptLlibb907wlopIdP9w908+wWMAlFTaR13PcTGruuF8AC CaHA== X-Gm-Message-State: AOAM533KExXNCKUTWaRXdqqEMCPA0TmaX6vFlsge602eCEQMCDHaPcWk 8Bs1nW/eEdAAlsp/wsJIBOEu5maOepZAFka8D35/gyEr8Wl7eC1b X-Google-Smtp-Source: ABdhPJzfiYNWPfJ5yLDtgGpXAlvtTUVopTDrqvvwToCC661ijNCF1SphxiPrzsqP4FqD60Wb4bUTBwf8+sUhNEsLYeA= X-Received: by 2002:a2e:8714:: with SMTP id m20mr6294133lji.320.1608531003450; Sun, 20 Dec 2020 22:10:03 -0800 (PST) MIME-Version: 1.0 References: <20201216141919.23262-1-masahisa.kojima@linaro.org> <20201216141919.23262-2-masahisa.kojima@linaro.org> <16515BFEBC173A6F.9537@groups.io> <012d01d6d738$6b5ac6e0$421054a0$@byosoft.com.cn> In-Reply-To: <012d01d6d738$6b5ac6e0$421054a0$@byosoft.com.cn> From: "Masahisa Kojima" Date: Mon, 21 Dec 2020 15:09:51 +0900 Message-ID: Subject: =?UTF-8?B?UmU6IFtlZGsyLWRldmVsXSDlm57lpI06IFtQQVRDSCAxLzFdIE1kZU1vZHVsZVBrZy9WYXJDaGVja1BvbGljeUxpYjogaW1wbGVtZW50IHN0YW5kYWxvbmUgTU0gdmVyc2lvbg==?= To: edk2-devel-groups-io , Liming Gao Cc: michael.d.kinney@intel.com, Kun Qin , Jian J Wang , Hao A Wu , Ard Biesheuvel , Sami Mujawar , Jiewen Yao , Supreeth Venkatesh , Bret Barkelew Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable Hi Liming, > One minor comment, new added VarCheckPolicyLibStandaloneMm.inf is requ= ired > to be listed in MdeModulePkg.dsc for build test. Thank you for your comment. I will send v2 patch soon. Thanks, Masahisa On Mon, 21 Dec 2020 at 10:27, gaoliming wrote: > > Masahisa: > One minor comment, new added VarCheckPolicyLibStandaloneMm.inf is requ= ired > to be listed in MdeModulePkg.dsc for build test. > > Thanks > Liming > > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > > =E5=8F=91=E4=BB=B6=E4=BA=BA: bounce+27952+69077+4905953+8761045@groups= .io > > =E4=BB=A3=E8=A1=A8 gaol= iming > > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2020=E5=B9=B412=E6=9C=8817=E6=97= = =A5 9:15 > > =E6=94=B6=E4=BB=B6=E4=BA=BA: 'Masahisa Kojima' ; > > devel@edk2.groups.io; michael.d.kinney@intel.com > > =E6=8A=84=E9=80=81: 'Kun Qin' ; 'Jian J Wang' > ; > > 'Hao A Wu' ; 'Ard Biesheuvel' > > ; 'Sami Mujawar' ; > > 'Jiewen Yao' ; 'Supreeth Venkatesh' > > ; 'Bret Barkelew' > > > > =E4=B8=BB=E9=A2=98: [edk2-devel] =E5=9B=9E=E5=A4=8D: [PATCH 1/1] MdeMo= dulePkg/VarCheckPolicyLib: > > implement standalone MM version > > > > Masahisa: > > The patch is good. Reviewed-by: Liming Gao > > > > Now, Mike proposes to create stable tag branch to include the critic= al > bug > > fix. I think this one is also the critical fix to be cherry-pick to th= e > > stable tag branch. > > > > Thanks > > Liming > > > -----=E9=82=AE=E4=BB=B6=E5=8E=9F=E4=BB=B6----- > > > =E5=8F=91=E4=BB=B6=E4=BA=BA: Masahisa Kojima > > > =E5=8F=91=E9=80=81=E6=97=B6=E9=97=B4: 2020=E5=B9=B412=E6=9C=8816=E6= =97=A5 22:19 > > > =E6=94=B6=E4=BB=B6=E4=BA=BA: devel@edk2.groups.io > > > =E6=8A=84=E9=80=81: Kun Qin ; Masahisa Kojima > > > ; Jian J Wang ; H= ao > > A > > > Wu ; Liming Gao ; Ard > > > Biesheuvel ; Sami Mujawar > > > ; Jiewen Yao ; Supreeth > > > Venkatesh ; Bret Barkelew > > > > > > =E4=B8=BB=E9=A2=98: [PATCH 1/1] MdeModulePkg/VarCheckPolicyLib: impl= ement > > standalone > > > MM version > > > > > > This commit adds the VarCheckPolicyLib that will be able to > > > execute in the context of standalone MM. > > > > > > 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 +++--- > > > > > > > > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibStandaloneMm > > > .c | 50 ++++++++++++++++++++ > > > > > MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLibTraditional.c > > > | 50 ++++++++++++++++++++ > > > 6 files changed, 165 insertions(+), 19 deletions(-) > > > > > > 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 > > > > > > > > > [Sources] > > > VarCheckPolicyLib.c > > > + VarCheckPolicyLibTraditional.c > > > + VarCheckPolicyLib.h > > > > > > > > > [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.i= nf > > > 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 > > > +# > > > ## > > > > > > [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 > > > > > > > > > [Sources] > > > VarCheckPolicyLib.c > > > + VarCheckPolicyLibStandaloneMm.c > > > + VarCheckPolicyLib.h > > > > > > > > > [Packages] > > > MdePkg/MdePkg.dec > > > MdeModulePkg/MdeModulePkg.dec > > > + StandaloneMmPkg/StandaloneMmPkg.dec > > > > > > > > > [LibraryClasses] > > > BaseLib > > > DebugLib > > > BaseMemoryLib > > > - DxeServicesLib > > > + MemLib > > > MemoryAllocationLib > > > VarCheckLib > > > VariablePolicyLib > > > @@ -37,6 +43,5 @@ [LibraryClasses] > > > SafeIntLib > > > MmServicesTableLib > > > > > > - > > > [Guids] > > > gVarCheckPolicyLibMmiHandlerGuid ## CONSUME ## Used to > > > register for MM Communication events. > > > diff --git a/MdeModulePkg/Library/VarCheckPolicyLib/VarCheckPolicyLi= b.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 construct= or > 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 VarC= heck > > > 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/VarCheckPolicyLi= b.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 > > > > > > @@ -23,6 +22,8 @@ SPDX-License-Identifier: BSD-2-Clause-Patent > > > > > > #include > > > > > > +#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 ju= st > 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. > > > > > > - @param[in] ImageHandle The firmware allocated handle for the E= FI > > > image. > > > - @param[in] SystemTable A pointer to the EFI System Table. > > > - > > > @retval EFI_SUCCESS The constructor executed correctly. > > > > > > **/ > > > 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 E= FI > > > 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 S= MM. > > > + > > > +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 E= FI > > > 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 > > > > > > > > > > > > > > > > > > > >=20 > >