[v4] Implement heap guard feature

[edk2] [PATCH v4 0/7] Implement heap guard feature

Posted by Jian J Wang 6 years, 6 months ago

> Path V4 changes:
> a. Change names of gEdkiiSmmMemoryAttributeProtocolGuid related
>    definitions from EFI_ to EDKII_
> b. Coding style cleanup
> c. Split patches in a more reasonable order and groups

> Patch V3 changes:
> a. Add new protocol gEdkiiSmmMemoryAttributeProtocolGuid to do
>    memory attributes update instead of doing it directly in SmmCore
> b. Fix GCC build error

> Patch V2 changes:
> a. Remove local variable initializer with memory copy from globals
> b. Change map table dump code to use DEBUG_PAGE|DEBUG_POOL level
>    message 
> c. Fix malfunction in 32-bit boot mode
> d. Add comment for the use of mOnGuarding
> e. Change name of function InitializePageTableLib to 
>    InitializePageTableGlobals
> f. Add code in 32-bit code to bypass setting page table to read-only
> g. Coding style clean-up
>

This feature makes use of paging mechanism to add a hidden (not present)
page just before and after the allocated memory block. If the code tries
to access memory outside of the allocated part, page fault exception will
be triggered.

This feature is disabled by default and is not recommended to enable it
in production build of BIOS.

This patch has passed following validations:

  a. Boot to shell (OVMF, Intel real platform)(32/64)
  b. Boot to Fedora 25 (64)

NT32 emulation platform was not validated with this feature enabled
due to the fact that it doesn't support paging which is needed for
this feature to work. But all are validated with feature is disabled.

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>

Jian J Wang (7):
  MdeModulePkg/MdeModulePkg.dec,.uni: Add Protocol, PCDs and string
    tokens
  MdeModulePkg/SmmMemoryAttribute.h: Add new protocol definitions
  UefiCpuPkg/CpuDxe: Reduce debug message
  MdeModulePkg/DxeIpl: Enable paging for heap guard
  MdeModulePkg/DxeCore: Implement heap guard feature for UEFI
  UefiCpuPkg/PiSmmCpuDxeSmm: Add SmmMemoryAttribute protocol
  MdeModulePkg/PiSmmCore: Implement heap guard feature for SMM mode

 MdeModulePkg/Core/Dxe/DxeMain.inf                  |    4 +
 MdeModulePkg/Core/Dxe/Mem/HeapGuard.c              | 1182 ++++++++++++++++
 MdeModulePkg/Core/Dxe/Mem/HeapGuard.h              |  394 ++++++
 MdeModulePkg/Core/Dxe/Mem/Imem.h                   |   38 +-
 MdeModulePkg/Core/Dxe/Mem/Page.c                   |  130 +-
 MdeModulePkg/Core/Dxe/Mem/Pool.c                   |  154 +-
 MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf            |    1 +
 MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c    |   36 +-
 MdeModulePkg/Core/PiSmmCore/HeapGuard.c            | 1467 ++++++++++++++++++++
 MdeModulePkg/Core/PiSmmCore/HeapGuard.h            |  398 ++++++
 MdeModulePkg/Core/PiSmmCore/Page.c                 |   52 +-
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.c            |    7 +-
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h            |   81 +-
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf          |    8 +
 MdeModulePkg/Core/PiSmmCore/Pool.c                 |   81 +-
 MdeModulePkg/Include/Protocol/SmmMemoryAttribute.h |  136 ++
 MdeModulePkg/MdeModulePkg.dec                      |   60 +
 MdeModulePkg/MdeModulePkg.uni                      |   58 +
 UefiCpuPkg/CpuDxe/CpuPageTable.c                   |    5 +-
 UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c           |   10 +
 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            |   10 +-
 25 files changed, 4496 insertions(+), 99 deletions(-)
 create mode 100644 MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
 create mode 100644 MdeModulePkg/Core/Dxe/Mem/HeapGuard.h
 create mode 100644 MdeModulePkg/Core/PiSmmCore/HeapGuard.c
 create mode 100644 MdeModulePkg/Core/PiSmmCore/HeapGuard.h
 create mode 100644 MdeModulePkg/Include/Protocol/SmmMemoryAttribute.h

-- 
2.14.1.windows.1

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v4 0/7] Implement heap guard feature

Posted by Laszlo Ersek 6 years, 6 months ago

Hi Jian,

On 10/27/17 08:11, Jian J Wang wrote:
>> Path V4 changes:
>> a. Change names of gEdkiiSmmMemoryAttributeProtocolGuid related
>>    definitions from EFI_ to EDKII_
>> b. Coding style cleanup
>> c. Split patches in a more reasonable order and groups
> 
>> Patch V3 changes:
>> a. Add new protocol gEdkiiSmmMemoryAttributeProtocolGuid to do
>>    memory attributes update instead of doing it directly in SmmCore
>> b. Fix GCC build error
> 
>> Patch V2 changes:
>> a. Remove local variable initializer with memory copy from globals
>> b. Change map table dump code to use DEBUG_PAGE|DEBUG_POOL level
>>    message 
>> c. Fix malfunction in 32-bit boot mode
>> d. Add comment for the use of mOnGuarding
>> e. Change name of function InitializePageTableLib to 
>>    InitializePageTableGlobals
>> f. Add code in 32-bit code to bypass setting page table to read-only
>> g. Coding style clean-up
>>
> 
> This feature makes use of paging mechanism to add a hidden (not present)
> page just before and after the allocated memory block. If the code tries
> to access memory outside of the allocated part, page fault exception will
> be triggered.
> 
> This feature is disabled by default and is not recommended to enable it
> in production build of BIOS.
> 
> This patch has passed following validations:
> 
>   a. Boot to shell (OVMF, Intel real platform)(32/64)
>   b. Boot to Fedora 25 (64)
> 
> NT32 emulation platform was not validated with this feature enabled
> due to the fact that it doesn't support paging which is needed for
> this feature to work. But all are validated with feature is disabled.
> 
> 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>
> 
> Jian J Wang (7):
>   MdeModulePkg/MdeModulePkg.dec,.uni: Add Protocol, PCDs and string
>     tokens
>   MdeModulePkg/SmmMemoryAttribute.h: Add new protocol definitions
>   UefiCpuPkg/CpuDxe: Reduce debug message
>   MdeModulePkg/DxeIpl: Enable paging for heap guard
>   MdeModulePkg/DxeCore: Implement heap guard feature for UEFI
>   UefiCpuPkg/PiSmmCpuDxeSmm: Add SmmMemoryAttribute protocol
>   MdeModulePkg/PiSmmCore: Implement heap guard feature for SMM mode
> 
>  MdeModulePkg/Core/Dxe/DxeMain.inf                  |    4 +
>  MdeModulePkg/Core/Dxe/Mem/HeapGuard.c              | 1182 ++++++++++++++++
>  MdeModulePkg/Core/Dxe/Mem/HeapGuard.h              |  394 ++++++
>  MdeModulePkg/Core/Dxe/Mem/Imem.h                   |   38 +-
>  MdeModulePkg/Core/Dxe/Mem/Page.c                   |  130 +-
>  MdeModulePkg/Core/Dxe/Mem/Pool.c                   |  154 +-
>  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf            |    1 +
>  MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c    |   36 +-
>  MdeModulePkg/Core/PiSmmCore/HeapGuard.c            | 1467 ++++++++++++++++++++
>  MdeModulePkg/Core/PiSmmCore/HeapGuard.h            |  398 ++++++
>  MdeModulePkg/Core/PiSmmCore/Page.c                 |   52 +-
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.c            |    7 +-
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.h            |   81 +-
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf          |    8 +
>  MdeModulePkg/Core/PiSmmCore/Pool.c                 |   81 +-
>  MdeModulePkg/Include/Protocol/SmmMemoryAttribute.h |  136 ++
>  MdeModulePkg/MdeModulePkg.dec                      |   60 +
>  MdeModulePkg/MdeModulePkg.uni                      |   58 +
>  UefiCpuPkg/CpuDxe/CpuPageTable.c                   |    5 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c           |   10 +
>  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            |   10 +-
>  25 files changed, 4496 insertions(+), 99 deletions(-)
>  create mode 100644 MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
>  create mode 100644 MdeModulePkg/Core/Dxe/Mem/HeapGuard.h
>  create mode 100644 MdeModulePkg/Core/PiSmmCore/HeapGuard.c
>  create mode 100644 MdeModulePkg/Core/PiSmmCore/HeapGuard.h
>  create mode 100644 MdeModulePkg/Include/Protocol/SmmMemoryAttribute.h
> 

I applied your patches on top of edk2 master (76fd5a660d70,
"MdeModulePkg/AtaAtapiPassThru: disable only BM-DMA at
ExitBootServices()", 2017-10-26), and regression-tested the following
platforms:

* ArmVirtQemu, AARCH64, QEMU TCG (x86_64) host, Fedora 25 Server guest,
normal boot. (I figured I'd test this because the DXE_CORE modifications
affect aarch64 too.)

* OVMF, IA32, QEMU/KVM, Fedora 25 guest, Q35 machine type, SMM. Normal
boot and S3. UEFI variable access test.

* OVMF, IA32X64, QEMU/KVM, Fedora 26 guest, Q35 machine type, SMM.
Normal boot and S3. UEFI variable access test. "multiprocessing" test.

* OVMF, IA32X64, QEMU/KVM, Windows 10 guest, Q35 machine type, SMM.
Normal boot and S3.

* OVMF, X64, QEMU/KVM, Fedora 26 guest, i440fx machine type, no SMM.
Normal boot and S3. "multiprocessing" test.

The test case references are from
<https://github.com/tianocore/tianocore.github.io/wiki/Testing-SMM-with-QEMU,-KVM-and-libvirt>.

I didn't encounter any regressions.

For the series:

Regression-tested-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v4 0/7] Implement heap guard feature

Posted by Wang, Jian J 6 years, 5 months ago

Just a friendly reminder.

The recent commits have fixed two blocking issues for checking in heap guard feature.

      6fe575d052e36b243657a5885b5457decac41f03 (BaseCryptLib memory overflow)
      cf8197a39d07179027455421a182598bd6989999
      5df73e2cc8e39da97d56da058667607f1c43acac (AllocateCopyPool memory overflow)
      2a6ede28fd8efd3051794e1f2727a692d2725fe9
      469293f8ee406f2b0bad2cf3bbbc510b2a1364eb

Please give your r-b if no more comments. I'd be happy to check in this patch soon.

Thanks,
Jian

> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jian J
> Wang
> Sent: Friday, October 27, 2017 2:12 PM
> To: edk2-devel@lists.01.org
> Subject: [edk2] [PATCH v4 0/7] Implement heap guard feature
> 
> > Path V4 changes:
> > a. Change names of gEdkiiSmmMemoryAttributeProtocolGuid related
> >    definitions from EFI_ to EDKII_
> > b. Coding style cleanup
> > c. Split patches in a more reasonable order and groups
> 
> > Patch V3 changes:
> > a. Add new protocol gEdkiiSmmMemoryAttributeProtocolGuid to do
> >    memory attributes update instead of doing it directly in SmmCore
> > b. Fix GCC build error
> 
> > Patch V2 changes:
> > a. Remove local variable initializer with memory copy from globals
> > b. Change map table dump code to use DEBUG_PAGE|DEBUG_POOL level
> >    message
> > c. Fix malfunction in 32-bit boot mode
> > d. Add comment for the use of mOnGuarding
> > e. Change name of function InitializePageTableLib to
> >    InitializePageTableGlobals
> > f. Add code in 32-bit code to bypass setting page table to read-only
> > g. Coding style clean-up
> >
> 
> This feature makes use of paging mechanism to add a hidden (not present)
> page just before and after the allocated memory block. If the code tries
> to access memory outside of the allocated part, page fault exception will
> be triggered.
> 
> This feature is disabled by default and is not recommended to enable it
> in production build of BIOS.
> 
> This patch has passed following validations:
> 
>   a. Boot to shell (OVMF, Intel real platform)(32/64)
>   b. Boot to Fedora 25 (64)
> 
> NT32 emulation platform was not validated with this feature enabled
> due to the fact that it doesn't support paging which is needed for
> this feature to work. But all are validated with feature is disabled.
> 
> 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>
> 
> Jian J Wang (7):
>   MdeModulePkg/MdeModulePkg.dec,.uni: Add Protocol, PCDs and string
>     tokens
>   MdeModulePkg/SmmMemoryAttribute.h: Add new protocol definitions
>   UefiCpuPkg/CpuDxe: Reduce debug message
>   MdeModulePkg/DxeIpl: Enable paging for heap guard
>   MdeModulePkg/DxeCore: Implement heap guard feature for UEFI
>   UefiCpuPkg/PiSmmCpuDxeSmm: Add SmmMemoryAttribute protocol
>   MdeModulePkg/PiSmmCore: Implement heap guard feature for SMM mode
> 
>  MdeModulePkg/Core/Dxe/DxeMain.inf                  |    4 +
>  MdeModulePkg/Core/Dxe/Mem/HeapGuard.c              | 1182
> ++++++++++++++++
>  MdeModulePkg/Core/Dxe/Mem/HeapGuard.h              |  394 ++++++
>  MdeModulePkg/Core/Dxe/Mem/Imem.h                   |   38 +-
>  MdeModulePkg/Core/Dxe/Mem/Page.c                   |  130 +-
>  MdeModulePkg/Core/Dxe/Mem/Pool.c                   |  154 +-
>  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf            |    1 +
>  MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c    |   36 +-
>  MdeModulePkg/Core/PiSmmCore/HeapGuard.c            | 1467
> ++++++++++++++++++++
>  MdeModulePkg/Core/PiSmmCore/HeapGuard.h            |  398 ++++++
>  MdeModulePkg/Core/PiSmmCore/Page.c                 |   52 +-
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.c            |    7 +-
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.h            |   81 +-
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf          |    8 +
>  MdeModulePkg/Core/PiSmmCore/Pool.c                 |   81 +-
>  MdeModulePkg/Include/Protocol/SmmMemoryAttribute.h |  136 ++
>  MdeModulePkg/MdeModulePkg.dec                      |   60 +
>  MdeModulePkg/MdeModulePkg.uni                      |   58 +
>  UefiCpuPkg/CpuDxe/CpuPageTable.c                   |    5 +-
>  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c           |   10 +
>  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            |   10 +-
>  25 files changed, 4496 insertions(+), 99 deletions(-)
>  create mode 100644 MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
>  create mode 100644 MdeModulePkg/Core/Dxe/Mem/HeapGuard.h
>  create mode 100644 MdeModulePkg/Core/PiSmmCore/HeapGuard.c
>  create mode 100644 MdeModulePkg/Core/PiSmmCore/HeapGuard.h
>  create mode 100644 MdeModulePkg/Include/Protocol/SmmMemoryAttribute.h
> 
> --
> 2.14.1.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v4 0/7] Implement heap guard feature

Posted by Yao, Jiewen 6 years, 5 months ago

That is great.

I have minor suggestion below:
1) Since StaticPaging and HeapGuard are conflicted feature, I suggest we use ASSERT to tell the end user, instead of disable StaticPaging silently.

+  //
+  // Don't mark page table as read-only if heap guard is enabled.
+  //
+  //      BIT2: SMM page guard enabled
+  //      BIT3: SMM pool guard enabled
+  //
+  if ((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0) {
+    return ;
+  }
+

2) I do not think we need add below in MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf. Would you please double confirm?

+  CpuLib

+  gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask    ## CONSUMES


With ASSERT added and cleanup in PiSmmCore.inf, reviewed-by: Jiewen.yao@intel.com


Thank you
Yao Jiewen



> -----Original Message-----
> From: Wang, Jian J
> Sent: Thursday, November 9, 2017 10:00 AM
> To: Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric <eric.dong@intel.com>;
> Zeng, Star <star.zeng@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
> Subject: RE: [edk2] [PATCH v4 0/7] Implement heap guard feature
> 
> Just a friendly reminder.
> 
> The recent commits have fixed two blocking issues for checking in heap guard
> feature.
> 
>       6fe575d052e36b243657a5885b5457decac41f03 (BaseCryptLib memory
> overflow)
>       cf8197a39d07179027455421a182598bd6989999
>       5df73e2cc8e39da97d56da058667607f1c43acac (AllocateCopyPool memory
> overflow)
>       2a6ede28fd8efd3051794e1f2727a692d2725fe9
>       469293f8ee406f2b0bad2cf3bbbc510b2a1364eb
> 
> Please give your r-b if no more comments. I'd be happy to check in this patch
> soon.
> 
> Thanks,
> Jian
> 
> > -----Original Message-----
> > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Jian J
> > Wang
> > Sent: Friday, October 27, 2017 2:12 PM
> > To: edk2-devel@lists.01.org
> > Subject: [edk2] [PATCH v4 0/7] Implement heap guard feature
> >
> > > Path V4 changes:
> > > a. Change names of gEdkiiSmmMemoryAttributeProtocolGuid related
> > >    definitions from EFI_ to EDKII_
> > > b. Coding style cleanup
> > > c. Split patches in a more reasonable order and groups
> >
> > > Patch V3 changes:
> > > a. Add new protocol gEdkiiSmmMemoryAttributeProtocolGuid to do
> > >    memory attributes update instead of doing it directly in SmmCore
> > > b. Fix GCC build error
> >
> > > Patch V2 changes:
> > > a. Remove local variable initializer with memory copy from globals
> > > b. Change map table dump code to use DEBUG_PAGE|DEBUG_POOL level
> > >    message
> > > c. Fix malfunction in 32-bit boot mode
> > > d. Add comment for the use of mOnGuarding
> > > e. Change name of function InitializePageTableLib to
> > >    InitializePageTableGlobals
> > > f. Add code in 32-bit code to bypass setting page table to read-only
> > > g. Coding style clean-up
> > >
> >
> > This feature makes use of paging mechanism to add a hidden (not present)
> > page just before and after the allocated memory block. If the code tries
> > to access memory outside of the allocated part, page fault exception will
> > be triggered.
> >
> > This feature is disabled by default and is not recommended to enable it
> > in production build of BIOS.
> >
> > This patch has passed following validations:
> >
> >   a. Boot to shell (OVMF, Intel real platform)(32/64)
> >   b. Boot to Fedora 25 (64)
> >
> > NT32 emulation platform was not validated with this feature enabled
> > due to the fact that it doesn't support paging which is needed for
> > this feature to work. But all are validated with feature is disabled.
> >
> > 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>
> >
> > Jian J Wang (7):
> >   MdeModulePkg/MdeModulePkg.dec,.uni: Add Protocol, PCDs and string
> >     tokens
> >   MdeModulePkg/SmmMemoryAttribute.h: Add new protocol definitions
> >   UefiCpuPkg/CpuDxe: Reduce debug message
> >   MdeModulePkg/DxeIpl: Enable paging for heap guard
> >   MdeModulePkg/DxeCore: Implement heap guard feature for UEFI
> >   UefiCpuPkg/PiSmmCpuDxeSmm: Add SmmMemoryAttribute protocol
> >   MdeModulePkg/PiSmmCore: Implement heap guard feature for SMM
> mode
> >
> >  MdeModulePkg/Core/Dxe/DxeMain.inf                  |    4 +
> >  MdeModulePkg/Core/Dxe/Mem/HeapGuard.c              | 1182
> > ++++++++++++++++
> >  MdeModulePkg/Core/Dxe/Mem/HeapGuard.h              |  394
> ++++++
> >  MdeModulePkg/Core/Dxe/Mem/Imem.h                   |   38 +-
> >  MdeModulePkg/Core/Dxe/Mem/Page.c                   |  130 +-
> >  MdeModulePkg/Core/Dxe/Mem/Pool.c                   |  154 +-
> >  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf            |    1 +
> >  MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c    |   36 +-
> >  MdeModulePkg/Core/PiSmmCore/HeapGuard.c            | 1467
> > ++++++++++++++++++++
> >  MdeModulePkg/Core/PiSmmCore/HeapGuard.h            |  398
> ++++++
> >  MdeModulePkg/Core/PiSmmCore/Page.c                 |   52 +-
> >  MdeModulePkg/Core/PiSmmCore/PiSmmCore.c            |    7 +-
> >  MdeModulePkg/Core/PiSmmCore/PiSmmCore.h            |   81 +-
> >  MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf          |    8 +
> >  MdeModulePkg/Core/PiSmmCore/Pool.c                 |   81 +-
> >  MdeModulePkg/Include/Protocol/SmmMemoryAttribute.h |  136 ++
> >  MdeModulePkg/MdeModulePkg.dec                      |   60 +
> >  MdeModulePkg/MdeModulePkg.uni                      |   58 +
> >  UefiCpuPkg/CpuDxe/CpuPageTable.c                   |    5 +-
> >  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c           |   10 +
> >  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            |   10 +-
> >  25 files changed, 4496 insertions(+), 99 deletions(-)
> >  create mode 100644 MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> >  create mode 100644 MdeModulePkg/Core/Dxe/Mem/HeapGuard.h
> >  create mode 100644 MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> >  create mode 100644 MdeModulePkg/Core/PiSmmCore/HeapGuard.h
> >  create mode 100644
> MdeModulePkg/Include/Protocol/SmmMemoryAttribute.h
> >
> > --
> > 2.14.1.windows.1
> >
> > _______________________________________________
> > edk2-devel mailing list
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Re: [edk2] [PATCH v4 0/7] Implement heap guard feature

Posted by Wang, Jian J 6 years, 5 months ago

Jiewen,

Thanks for the comments.

1) I agree.
2) I forgot to remove them since we added new SMM protocol to do the same thing instead.

> -----Original Message-----
> From: Yao, Jiewen
> Sent: Thursday, November 09, 2017 10:53 AM
> To: Wang, Jian J <jian.j.wang@intel.com>; Dong, Eric <eric.dong@intel.com>;
> Zeng, Star <star.zeng@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Laszlo Ersek
> <lersek@redhat.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: edk2-devel@lists.01.org
> Subject: RE: [edk2] [PATCH v4 0/7] Implement heap guard feature
> 
> That is great.
> 
> I have minor suggestion below:
> 1) Since StaticPaging and HeapGuard are conflicted feature, I suggest we use
> ASSERT to tell the end user, instead of disable StaticPaging silently.
> 
> +  //
> +  // Don't mark page table as read-only if heap guard is enabled.
> +  //
> +  //      BIT2: SMM page guard enabled
> +  //      BIT3: SMM pool guard enabled
> +  //
> +  if ((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT3 | BIT2)) != 0) {
> +    return ;
> +  }
> +
> 
> 2) I do not think we need add below in
> MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf. Would you please double
> confirm?
> 
> +  CpuLib
> 
> +
> gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask
> ## CONSUMES
> 
> 
> With ASSERT added and cleanup in PiSmmCore.inf, reviewed-by:
> Jiewen.yao@intel.com
> 
> 
> Thank you
> Yao Jiewen
> 
> 
> 
> > -----Original Message-----
> > From: Wang, Jian J
> > Sent: Thursday, November 9, 2017 10:00 AM
> > To: Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric <eric.dong@intel.com>;
> > Zeng, Star <star.zeng@intel.com>; Ni, Ruiyu <ruiyu.ni@intel.com>; Laszlo
> Ersek
> > <lersek@redhat.com>; Kinney, Michael D <michael.d.kinney@intel.com>
> > Cc: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
> > Subject: RE: [edk2] [PATCH v4 0/7] Implement heap guard feature
> >
> > Just a friendly reminder.
> >
> > The recent commits have fixed two blocking issues for checking in heap guard
> > feature.
> >
> >       6fe575d052e36b243657a5885b5457decac41f03 (BaseCryptLib memory
> > overflow)
> >       cf8197a39d07179027455421a182598bd6989999
> >       5df73e2cc8e39da97d56da058667607f1c43acac (AllocateCopyPool
> memory
> > overflow)
> >       2a6ede28fd8efd3051794e1f2727a692d2725fe9
> >       469293f8ee406f2b0bad2cf3bbbc510b2a1364eb
> >
> > Please give your r-b if no more comments. I'd be happy to check in this patch
> > soon.
> >
> > Thanks,
> > Jian
> >
> > > -----Original Message-----
> > > From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of
> Jian J
> > > Wang
> > > Sent: Friday, October 27, 2017 2:12 PM
> > > To: edk2-devel@lists.01.org
> > > Subject: [edk2] [PATCH v4 0/7] Implement heap guard feature
> > >
> > > > Path V4 changes:
> > > > a. Change names of gEdkiiSmmMemoryAttributeProtocolGuid related
> > > >    definitions from EFI_ to EDKII_
> > > > b. Coding style cleanup
> > > > c. Split patches in a more reasonable order and groups
> > >
> > > > Patch V3 changes:
> > > > a. Add new protocol gEdkiiSmmMemoryAttributeProtocolGuid to do
> > > >    memory attributes update instead of doing it directly in SmmCore
> > > > b. Fix GCC build error
> > >
> > > > Patch V2 changes:
> > > > a. Remove local variable initializer with memory copy from globals
> > > > b. Change map table dump code to use DEBUG_PAGE|DEBUG_POOL level
> > > >    message
> > > > c. Fix malfunction in 32-bit boot mode
> > > > d. Add comment for the use of mOnGuarding
> > > > e. Change name of function InitializePageTableLib to
> > > >    InitializePageTableGlobals
> > > > f. Add code in 32-bit code to bypass setting page table to read-only
> > > > g. Coding style clean-up
> > > >
> > >
> > > This feature makes use of paging mechanism to add a hidden (not present)
> > > page just before and after the allocated memory block. If the code tries
> > > to access memory outside of the allocated part, page fault exception will
> > > be triggered.
> > >
> > > This feature is disabled by default and is not recommended to enable it
> > > in production build of BIOS.
> > >
> > > This patch has passed following validations:
> > >
> > >   a. Boot to shell (OVMF, Intel real platform)(32/64)
> > >   b. Boot to Fedora 25 (64)
> > >
> > > NT32 emulation platform was not validated with this feature enabled
> > > due to the fact that it doesn't support paging which is needed for
> > > this feature to work. But all are validated with feature is disabled.
> > >
> > > 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>
> > >
> > > Jian J Wang (7):
> > >   MdeModulePkg/MdeModulePkg.dec,.uni: Add Protocol, PCDs and string
> > >     tokens
> > >   MdeModulePkg/SmmMemoryAttribute.h: Add new protocol definitions
> > >   UefiCpuPkg/CpuDxe: Reduce debug message
> > >   MdeModulePkg/DxeIpl: Enable paging for heap guard
> > >   MdeModulePkg/DxeCore: Implement heap guard feature for UEFI
> > >   UefiCpuPkg/PiSmmCpuDxeSmm: Add SmmMemoryAttribute protocol
> > >   MdeModulePkg/PiSmmCore: Implement heap guard feature for SMM
> > mode
> > >
> > >  MdeModulePkg/Core/Dxe/DxeMain.inf                  |    4 +
> > >  MdeModulePkg/Core/Dxe/Mem/HeapGuard.c              | 1182
> > > ++++++++++++++++
> > >  MdeModulePkg/Core/Dxe/Mem/HeapGuard.h              |  394
> > ++++++
> > >  MdeModulePkg/Core/Dxe/Mem/Imem.h                   |   38 +-
> > >  MdeModulePkg/Core/Dxe/Mem/Page.c                   |  130 +-
> > >  MdeModulePkg/Core/Dxe/Mem/Pool.c                   |  154 +-
> > >  MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf            |    1 +
> > >  MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c    |   36 +-
> > >  MdeModulePkg/Core/PiSmmCore/HeapGuard.c            | 1467
> > > ++++++++++++++++++++
> > >  MdeModulePkg/Core/PiSmmCore/HeapGuard.h            |  398
> > ++++++
> > >  MdeModulePkg/Core/PiSmmCore/Page.c                 |   52 +-
> > >  MdeModulePkg/Core/PiSmmCore/PiSmmCore.c            |    7 +-
> > >  MdeModulePkg/Core/PiSmmCore/PiSmmCore.h            |   81 +-
> > >  MdeModulePkg/Core/PiSmmCore/PiSmmCore.inf          |    8 +
> > >  MdeModulePkg/Core/PiSmmCore/Pool.c                 |   81 +-
> > >  MdeModulePkg/Include/Protocol/SmmMemoryAttribute.h |  136 ++
> > >  MdeModulePkg/MdeModulePkg.dec                      |   60 +
> > >  MdeModulePkg/MdeModulePkg.uni                      |   58 +
> > >  UefiCpuPkg/CpuDxe/CpuPageTable.c                   |    5 +-
> > >  UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c           |   10 +
> > >  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            |   10 +-
> > >  25 files changed, 4496 insertions(+), 99 deletions(-)
> > >  create mode 100644 MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> > >  create mode 100644 MdeModulePkg/Core/Dxe/Mem/HeapGuard.h
> > >  create mode 100644 MdeModulePkg/Core/PiSmmCore/HeapGuard.c
> > >  create mode 100644 MdeModulePkg/Core/PiSmmCore/HeapGuard.h
> > >  create mode 100644
> > MdeModulePkg/Include/Protocol/SmmMemoryAttribute.h
> > >
> > > --
> > > 2.14.1.windows.1
> > >
> > > _______________________________________________
> > > edk2-devel mailing list
> > > edk2-devel@lists.01.org
> > > https://lists.01.org/mailman/listinfo/edk2-devel
_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel