From: "Wang, Jian J" <jian.j.wang@intel.com>
To: "edk2-devel@lists.01.org" <edk2-devel@lists.01.org>
Cc: "Yao, Jiewen" <jiewen.yao@intel.com>,
"Dong, Eric" <eric.dong@intel.com>,
Laszlo Ersek <lersek@redhat.com>,
"Ni, Ruiyu" <ruiyu.ni@intel.com>
Subject: Re: [PATCH v3 5/6] UefiCpuPkg/PiSmmCpuDxeSmm: Disable page table protection
Date: Thu, 26 Oct 2017 06:20:14 +0000 [thread overview]
Message-ID: <D827630B58408649ACB04F44C510003624CA54E4@SHSMSX103.ccr.corp.intel.com> (raw)
In-Reply-To: <20171023005054.7528-6-jian.j.wang@intel.com>
Laszlo and Ruiyu,
Could you please take a look at this part? There's a new protocol introduced.
Thanks,
Jian
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jian J
> Wang
> Sent: Monday, October 23, 2017 8:51 AM
> To: edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric <eric.dong@intel.com>
> Subject: [edk2] [PATCH v3 5/6] UefiCpuPkg/PiSmmCpuDxeSmm: Disable page
> table protection
>
> > v3
> > According to Jiewen's feedback, implement new protocol
> > gEdkiiSmmMemoryAttributeProtocolGuid
> > to change memory attributes.
>
> > v2
> > According to Eric's feedback:
> > a. Enclose bit-or with parentheses
> > b. Add code in 32-bit code to bypass setting page table to read-only
>
> Heap guard feature will update page attributes frequently. The page table
> should not set to be read-only if heap guard feature is enabled for SMM
> mode. Otherwise this feature cannot work.
>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Suggested-by: Ayellet Wolman <ayellet.wolman@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> ---
> UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 7 +
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c | 20 +++
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h | 98
> +++++++++++++
> UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf | 2 +
> UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 163
> +++++++++++++++++++++
> UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 3 +-
> 6 files changed, 292 insertions(+), 1 deletion(-)
>
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> index f295c2ebf2..27c11f1b8d 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c
> @@ -184,6 +184,13 @@ SetPageTableAttributes (
> BOOLEAN IsSplitted;
> BOOLEAN PageTableSplitted;
>
> + //
> + // Don't mark page table as read-only if heap guard is enabled.
> + //
> + if ((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0) {
> + return ;
> + }
> +
> DEBUG ((DEBUG_INFO, "SetPageTableAttributes\n"));
>
> //
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> index 282d2e6981..8635f2d2c8 100755
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.c
> @@ -76,6 +76,15 @@ EFI_SMM_CPU_PROTOCOL mSmmCpu = {
> SmmWriteSaveState
> };
>
> +///
> +/// SMM Memory Attribute Protocol instance
> +///
> +EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL mSmmMemoryAttribute = {
> + EdkiiSmmGetMemoryAttributes,
> + EdkiiSmmSetMemoryAttributes,
> + EdkiiSmmClearMemoryAttributes
> +};
> +
> EFI_CPU_INTERRUPT_HANDLER
> mExternalVectorTable[EXCEPTION_VECTOR_NUMBER];
>
> //
> @@ -893,6 +902,17 @@ PiCpuSmmEntry (
> );
> ASSERT_EFI_ERROR (Status);
>
> + //
> + // Install the SMM Memory Attribute Protocol into SMM protocol database
> + //
> + Status = gSmst->SmmInstallProtocolInterface (
> + &mSmmCpuHandle,
> + &gEdkiiSmmMemoryAttributeProtocolGuid,
> + EFI_NATIVE_INTERFACE,
> + &mSmmMemoryAttribute
> + );
> + ASSERT_EFI_ERROR (Status);
> +
> //
> // Expose address of CPU Hot Plug Data structure if CPU hot plug is supported.
> //
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> index 1cf85c1481..e1c231e10b 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.h
> @@ -25,6 +25,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY
> KIND, EITHER EXPRESS OR IMPLIED.
> #include <Protocol/SmmAccess2.h>
> #include <Protocol/SmmReadyToLock.h>
> #include <Protocol/SmmCpuService.h>
> +#include <Protocol/SmmMemoryAttribute.h>
>
> #include <Guid/AcpiS3Context.h>
> #include <Guid/PiSmmMemoryAttributesTable.h>
> @@ -1068,4 +1069,101 @@ TransferApToSafeState (
> IN UINTN NumberToFinishAddress
> );
>
> +/**
> + This function set given attributes of the memory region specified by
> + BaseAddress and Length.
> +
> + @param This The EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL
> instance.
> + @param BaseAddress The physical address that is the start address of
> + a memory region.
> + @param Length The size in bytes of the memory region.
> + @param Attributes The bit mask of attributes to set for the memory
> + region.
> +
> + @retval EFI_SUCCESS The attributes were set for the memory region.
> + @retval EFI_INVALID_PARAMETER Length is zero.
> + Attributes specified an illegal combination of
> + attributes that cannot be set together.
> + @retval EFI_UNSUPPORTED The processor does not support one or more
> + bytes of the memory resource range specified
> + by BaseAddress and Length.
> + The bit mask of attributes is not support for
> + the memory resource range specified by
> + BaseAddress and Length.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +EdkiiSmmSetMemoryAttributes (
> + IN EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL *This,
> + IN EFI_PHYSICAL_ADDRESS BaseAddress,
> + IN UINT64 Length,
> + IN UINT64 Attributes
> + );
> +
> +/**
> + This function clears given attributes of the memory region specified by
> + BaseAddress and Length.
> +
> + @param This The EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL
> instance.
> + @param BaseAddress The physical address that is the start address of
> + a memory region.
> + @param Length The size in bytes of the memory region.
> + @param Attributes The bit mask of attributes to set for the memory
> + region.
> +
> + @retval EFI_SUCCESS The attributes were set for the memory region.
> + @retval EFI_INVALID_PARAMETER Length is zero.
> + Attributes specified an illegal combination of
> + attributes that cannot be set together.
> + @retval EFI_UNSUPPORTED The processor does not support one or more
> + bytes of the memory resource range specified
> + by BaseAddress and Length.
> + The bit mask of attributes is not support for
> + the memory resource range specified by
> + BaseAddress and Length.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +EdkiiSmmClearMemoryAttributes (
> + IN EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL *This,
> + IN EFI_PHYSICAL_ADDRESS BaseAddress,
> + IN UINT64 Length,
> + IN UINT64 Attributes
> + );
> +
> +/**
> + This function retrieve the attributes of the memory region specified by
> + BaseAddress and Length. If different attributes are got from different part
> + of the memory region, EFI_NO_MAPPING will be returned.
> +
> + @param This The EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL
> instance.
> + @param BaseAddress The physical address that is the start address of
> + a memory region.
> + @param Length The size in bytes of the memory region.
> + @param Attributes Pointer to attributes returned.
> +
> + @retval EFI_SUCCESS The attributes got for the memory region.
> + @retval EFI_INVALID_PARAMETER Length is zero.
> + Attributes is NULL.
> + @retval EFI_NO_MAPPING Attributes are not consistent cross the
> memory
> + region.
> + @retval EFI_UNSUPPORTED The processor does not support one or more
> + bytes of the memory resource range specified
> + by BaseAddress and Length.
> + The bit mask of attributes is not support for
> + the memory resource range specified by
> + BaseAddress and Length.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +EdkiiSmmGetMemoryAttributes (
> + IN EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL *This,
> + IN EFI_PHYSICAL_ADDRESS BaseAddress,
> + IN UINT64 Length,
> + IN UINT64 *Attributes
> + );
> +
> #endif
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> index 099792e6ce..43d9edd0d5 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/PiSmmCpuDxeSmm.inf
> @@ -129,6 +129,7 @@
> gEfiSmmCpuProtocolGuid ## PRODUCES
> gEfiSmmReadyToLockProtocolGuid ## NOTIFY
> gEfiSmmCpuServiceProtocolGuid ## PRODUCES
> + gEdkiiSmmMemoryAttributeProtocolGuid ## PRODUCES
>
> [Guids]
> gEfiAcpiVariableGuid ## SOMETIMES_CONSUMES ## HOB # it is
> used for S3 boot.
> @@ -159,6 +160,7 @@
> gUefiCpuPkgTokenSpaceGuid.PcdCpuSmmStaticPageTable ##
> CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable ## CONSUMES
> gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask
> ## CONSUMES
> + gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask ##
> CONSUMES
>
> [Depex]
> gEfiMpServiceProtocolGuid
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> index 3ad5256f1e..eb5e639e4b 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c
> @@ -1120,3 +1120,166 @@ IsSmmCommBufferForbiddenAddress (
> }
> return FALSE;
> }
> +
> +/**
> + This function set given attributes of the memory region specified by
> + BaseAddress and Length.
> +
> + @param This The EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL
> instance.
> + @param BaseAddress The physical address that is the start address of
> + a memory region.
> + @param Length The size in bytes of the memory region.
> + @param Attributes The bit mask of attributes to set for the memory
> + region.
> +
> + @retval EFI_SUCCESS The attributes were set for the memory region.
> + @retval EFI_INVALID_PARAMETER Length is zero.
> + Attributes specified an illegal combination of
> + attributes that cannot be set together.
> + @retval EFI_UNSUPPORTED The processor does not support one or more
> + bytes of the memory resource range specified
> + by BaseAddress and Length.
> + The bit mask of attributes is not support for
> + the memory resource range specified by
> + BaseAddress and Length.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +EdkiiSmmSetMemoryAttributes (
> + IN EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL *This,
> + IN EFI_PHYSICAL_ADDRESS BaseAddress,
> + IN UINT64 Length,
> + IN UINT64 Attributes
> + )
> +{
> + return SmmSetMemoryAttributes (BaseAddress, Length, Attributes);
> +}
> +
> +/**
> + This function clears given attributes of the memory region specified by
> + BaseAddress and Length.
> +
> + @param This The EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL
> instance.
> + @param BaseAddress The physical address that is the start address of
> + a memory region.
> + @param Length The size in bytes of the memory region.
> + @param Attributes The bit mask of attributes to set for the memory
> + region.
> +
> + @retval EFI_SUCCESS The attributes were set for the memory region.
> + @retval EFI_INVALID_PARAMETER Length is zero.
> + Attributes specified an illegal combination of
> + attributes that cannot be set together.
> + @retval EFI_UNSUPPORTED The processor does not support one or more
> + bytes of the memory resource range specified
> + by BaseAddress and Length.
> + The bit mask of attributes is not support for
> + the memory resource range specified by
> + BaseAddress and Length.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +EdkiiSmmClearMemoryAttributes (
> + IN EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL *This,
> + IN EFI_PHYSICAL_ADDRESS BaseAddress,
> + IN UINT64 Length,
> + IN UINT64 Attributes
> + )
> +{
> + return SmmClearMemoryAttributes (BaseAddress, Length, Attributes);
> +}
> +
> +/**
> + This function retrieve the attributes of the memory region specified by
> + BaseAddress and Length. If different attributes are got from different part
> + of the memory region, EFI_NO_MAPPING will be returned.
> +
> + @param This The EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL
> instance.
> + @param BaseAddress The physical address that is the start address of
> + a memory region.
> + @param Length The size in bytes of the memory region.
> + @param Attributes Pointer to attributes returned.
> +
> + @retval EFI_SUCCESS The attributes got for the memory region.
> + @retval EFI_INVALID_PARAMETER Length is zero.
> + Attributes is NULL.
> + @retval EFI_NO_MAPPING Attributes are not consistent cross the
> memory
> + region.
> + @retval EFI_UNSUPPORTED The processor does not support one or more
> + bytes of the memory resource range specified
> + by BaseAddress and Length.
> + The bit mask of attributes is not support for
> + the memory resource range specified by
> + BaseAddress and Length.
> +
> +**/
> +EFI_STATUS
> +EFIAPI
> +EdkiiSmmGetMemoryAttributes (
> + IN EFI_SMM_MEMORY_ATTRIBUTE_PROTOCOL *This,
> + IN EFI_PHYSICAL_ADDRESS BaseAddress,
> + IN UINT64 Length,
> + IN UINT64 *Attributes
> + )
> +{
> + EFI_PHYSICAL_ADDRESS Address;
> + UINT64 *PageEntry;
> + UINT64 MemAttr;
> + PAGE_ATTRIBUTE PageAttr;
> + INT64 Size;
> +
> + if (Length < SIZE_4KB || Attributes == NULL) {
> + return EFI_INVALID_PARAMETER;
> + }
> +
> + Size = (INT64)Length;
> + MemAttr = (UINT64)-1;
> +
> + do {
> +
> + PageEntry = GetPageTableEntry (BaseAddress, &PageAttr);
> + if (PageEntry == NULL || PageAttr == PageNone) {
> + return EFI_UNSUPPORTED;
> + }
> +
> + //
> + // If the memory range is cross page table boundary, make sure they
> + // share the same attribute. Return EFI_NO_MAPPING if not.
> + //
> + *Attributes = GetAttributesFromPageEntry (PageEntry);
> + if (MemAttr != (UINT64)-1 && *Attributes != MemAttr) {
> + return EFI_NO_MAPPING;
> + }
> +
> + switch (PageAttr) {
> + case Page4K:
> + Address = *PageEntry & ~mAddressEncMask &
> PAGING_4K_ADDRESS_MASK_64;
> + Size -= (SIZE_4KB - (BaseAddress - Address));
> + BaseAddress += (SIZE_4KB - (BaseAddress - Address));
> + break;
> +
> + case Page2M:
> + Address = *PageEntry & ~mAddressEncMask &
> PAGING_2M_ADDRESS_MASK_64;
> + Size -= SIZE_2MB - (BaseAddress - Address);
> + BaseAddress += SIZE_2MB - (BaseAddress - Address);
> + break;
> +
> + case Page1G:
> + Address = *PageEntry & ~mAddressEncMask &
> PAGING_1G_ADDRESS_MASK_64;
> + Size -= SIZE_1GB - (BaseAddress - Address);
> + BaseAddress += SIZE_1GB - (BaseAddress - Address);
> + break;
> +
> + default:
> + return EFI_UNSUPPORTED;
> + }
> +
> + MemAttr = *Attributes;
> +
> + } while (Size > 0);
> +
> + return EFI_SUCCESS;
> +}
> +
> diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> index 3dde80f9ba..4d4668a0c6 100644
> --- a/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c
> @@ -902,7 +902,8 @@ SetPageTableAttributes (
> BOOLEAN IsSplitted;
> BOOLEAN PageTableSplitted;
>
> - if (!mCpuSmmStaticPageTable) {
> + if (!mCpuSmmStaticPageTable
> + || (PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0) {
> return ;
> }
>
> --
> 2.14.1.windows.1
>
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
next prev parent reply other threads:[~2017-10-26 6:16 UTC|newest]
Thread overview: 18+ messages / expand[flat|nested] mbox.gz Atom feed top
2017-10-23 0:50 [PATCH v3 0/6] Implement heap guard feature Jian J Wang
2017-10-23 0:50 ` [PATCH v3 1/6] MdeModulePkg/DxeCore: Implement heap guard feature for UEFI Jian J Wang
2017-10-23 0:50 ` [PATCH v3 2/6] MdeModulePkg/PiSmmCore: Implement heap guard feature for SMM mode Jian J Wang
2017-10-23 0:50 ` [PATCH v3 3/6] MdeModulePkg/MdeModulePkg.dec, .uni: Add Protocol, PCDs and string tokens Jian J Wang
2017-10-23 0:50 ` [PATCH v3 4/6] UefiCpuPkg/CpuDxe: Reduce debug message Jian J Wang
2017-10-23 0:50 ` [PATCH v3 5/6] UefiCpuPkg/PiSmmCpuDxeSmm: Disable page table protection Jian J Wang
2017-10-26 6:20 ` Wang, Jian J [this message]
2017-10-26 7:17 ` Ni, Ruiyu
2017-10-26 7:40 ` Wang, Jian J
2017-10-23 0:50 ` [PATCH v3 6/6] MdeModulePkg/DxeIpl: Enable paging for heap guard Jian J Wang
2017-10-25 1:48 ` [PATCH v3 0/6] Implement heap guard feature Wang, Jian J
2017-10-26 6:48 ` Yao, Jiewen
2017-10-26 6:52 ` Zeng, Star
2017-10-26 7:39 ` Wang, Jian J
2017-10-26 7:38 ` Wang, Jian J
2017-10-26 13:26 ` Laszlo Ersek
2017-10-27 1:39 ` Wang, Jian J
2017-10-27 12:32 ` Laszlo Ersek
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-list from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=D827630B58408649ACB04F44C510003624CA54E4@SHSMSX103.ccr.corp.intel.com \
--to=devel@edk2.groups.io \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox