[v3] Implement heap guard feature

[edk2] [PATCH v3 0/6] Implement heap guard feature

Posted by Jian J Wang 6 years, 5 months ago

> 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.

Cc: Star Zeng <star.zeng@intel.com>
Cc: Eric Dong <eric.dong@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Michael Kinney <michael.d.kinney@intel.com>
Cc: Ayellet Wolman <ayellet.wolman@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>

Jian J Wang (6):
  MdeModulePkg/DxeCore: Implement heap guard feature for UEFI
  MdeModulePkg/PiSmmCore: Implement heap guard feature for SMM mode
  MdeModulePkg/MdeModulePkg.dec,.uni: Add Protocol, PCDs and string
    tokens
  UefiCpuPkg/CpuDxe: Reduce debug message
  UefiCpuPkg/PiSmmCpuDxeSmm: Disable page table protection
  MdeModulePkg/DxeIpl: Enable paging for heap guard

 MdeModulePkg/Core/Dxe/DxeMain.inf                  |    4 +
 MdeModulePkg/Core/Dxe/Mem/HeapGuard.c              | 1184 ++++++++++++++++
 MdeModulePkg/Core/Dxe/Mem/HeapGuard.h              |  380 +++++
 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    |   29 +-
 MdeModulePkg/Core/PiSmmCore/HeapGuard.c            | 1469 ++++++++++++++++++++
 MdeModulePkg/Core/PiSmmCore/HeapGuard.h            |  399 ++++++
 MdeModulePkg/Core/PiSmmCore/Page.c                 |   51 +-
 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           |    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 +-
 25 files changed, 4472 insertions(+), 96 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 v3 0/6] Implement heap guard feature

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

Hi,

Just a warm reminding. I didn't see any feedbacks on the v3 patch.
If no more comments, I'll check in the patch soon.

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: Kinney, Michael D <michael.d.kinney@intel.com>; Wolman, Ayellet
> <ayellet.wolman@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: [edk2] [PATCH v3 0/6] Implement heap guard feature
> 
> > 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.
> 
> Cc: Star Zeng <star.zeng@intel.com>
> Cc: Eric Dong <eric.dong@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Michael Kinney <michael.d.kinney@intel.com>
> Cc: Ayellet Wolman <ayellet.wolman@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>
> 
> Jian J Wang (6):
>   MdeModulePkg/DxeCore: Implement heap guard feature for UEFI
>   MdeModulePkg/PiSmmCore: Implement heap guard feature for SMM mode
>   MdeModulePkg/MdeModulePkg.dec,.uni: Add Protocol, PCDs and string
>     tokens
>   UefiCpuPkg/CpuDxe: Reduce debug message
>   UefiCpuPkg/PiSmmCpuDxeSmm: Disable page table protection
>   MdeModulePkg/DxeIpl: Enable paging for heap guard
> 
>  MdeModulePkg/Core/Dxe/DxeMain.inf                  |    4 +
>  MdeModulePkg/Core/Dxe/Mem/HeapGuard.c              | 1184
> ++++++++++++++++
>  MdeModulePkg/Core/Dxe/Mem/HeapGuard.h              |  380 +++++
>  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    |   29 +-
>  MdeModulePkg/Core/PiSmmCore/HeapGuard.c            | 1469
> ++++++++++++++++++++
>  MdeModulePkg/Core/PiSmmCore/HeapGuard.h            |  399 ++++++
>  MdeModulePkg/Core/PiSmmCore/Page.c                 |   51 +-
>  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           |    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 +-
>  25 files changed, 4472 insertions(+), 96 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 v3 0/6] Implement heap guard feature

Posted by Yao, Jiewen 6 years, 5 months ago

That is great work. Jian.

Some suggestion for your consideration:

0) I suggest add Laszlo to review SMM part, and add Ruiyu to review SMM_MEMORY_ATTRIBUTE_PROTOCOL.

1) Would you please mention what test we have done for this feature?
Such as OVMF/realPlatform? IA32/X64?

Have you validated NT32? Or try to enable protection? :-)

2) Is that any dependency of this patch?
I think there is OPENSSL wrapper reallocate() issue not resolved yet.

I suggest we check in all dependent patch at first, then check in this one.

3) If you need submit V4, please separate MdeModulePkg/Include/Protocol/SmmMemoryAttribute.h from 2/6 to be a standalone patch. In general, an interface and an implementation are separated.


Thank you
Yao Jiewen



> -----Original Message-----
> From: Wang, Jian J
> Sent: Wednesday, October 25, 2017 9:48 AM
> To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Wolman, Ayellet
> <ayellet.wolman@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric
> <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH v3 0/6] Implement heap guard feature
> 
> Hi,
> 
> Just a warm reminding. I didn't see any feedbacks on the v3 patch.
> If no more comments, I'll check in the patch soon.
> 
> 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: Kinney, Michael D <michael.d.kinney@intel.com>; Wolman, Ayellet
> > <ayellet.wolman@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Dong, Eric
> > <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> > Subject: [edk2] [PATCH v3 0/6] Implement heap guard feature
> >
> > > 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.
> >
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Michael Kinney <michael.d.kinney@intel.com>
> > Cc: Ayellet Wolman <ayellet.wolman@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>
> >
> > Jian J Wang (6):
> >   MdeModulePkg/DxeCore: Implement heap guard feature for UEFI
> >   MdeModulePkg/PiSmmCore: Implement heap guard feature for SMM
> mode
> >   MdeModulePkg/MdeModulePkg.dec,.uni: Add Protocol, PCDs and string
> >     tokens
> >   UefiCpuPkg/CpuDxe: Reduce debug message
> >   UefiCpuPkg/PiSmmCpuDxeSmm: Disable page table protection
> >   MdeModulePkg/DxeIpl: Enable paging for heap guard
> >
> >  MdeModulePkg/Core/Dxe/DxeMain.inf                  |    4 +
> >  MdeModulePkg/Core/Dxe/Mem/HeapGuard.c              | 1184
> > ++++++++++++++++
> >  MdeModulePkg/Core/Dxe/Mem/HeapGuard.h              |  380
> +++++
> >  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    |   29 +-
> >  MdeModulePkg/Core/PiSmmCore/HeapGuard.c            | 1469
> > ++++++++++++++++++++
> >  MdeModulePkg/Core/PiSmmCore/HeapGuard.h            |  399
> ++++++
> >  MdeModulePkg/Core/PiSmmCore/Page.c                 |   51 +-
> >  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           |    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 +-
> >  25 files changed, 4472 insertions(+), 96 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 v3 0/6] Implement heap guard feature

Posted by Zeng, Star 6 years, 5 months ago

I suggest putting [3/6] at the first patch as it adds definitions that are used by other patches.

Thanks,
Star
-----Original Message-----
From: Yao, Jiewen 
Sent: Thursday, October 26, 2017 2:49 PM
To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Wolman, Ayellet <ayellet.wolman@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
Subject: RE: [edk2] [PATCH v3 0/6] Implement heap guard feature

That is great work. Jian.

Some suggestion for your consideration:

0) I suggest add Laszlo to review SMM part, and add Ruiyu to review SMM_MEMORY_ATTRIBUTE_PROTOCOL.

1) Would you please mention what test we have done for this feature?
Such as OVMF/realPlatform? IA32/X64?

Have you validated NT32? Or try to enable protection? :-)

2) Is that any dependency of this patch?
I think there is OPENSSL wrapper reallocate() issue not resolved yet.

I suggest we check in all dependent patch at first, then check in this one.

3) If you need submit V4, please separate MdeModulePkg/Include/Protocol/SmmMemoryAttribute.h from 2/6 to be a standalone patch. In general, an interface and an implementation are separated.


Thank you
Yao Jiewen



> -----Original Message-----
> From: Wang, Jian J
> Sent: Wednesday, October 25, 2017 9:48 AM
> To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Wolman, Ayellet 
> <ayellet.wolman@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Dong, 
> Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH v3 0/6] Implement heap guard feature
> 
> Hi,
> 
> Just a warm reminding. I didn't see any feedbacks on the v3 patch.
> If no more comments, I'll check in the patch soon.
> 
> 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: Kinney, Michael D <michael.d.kinney@intel.com>; Wolman, Ayellet 
> > <ayellet.wolman@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; 
> > Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> > Subject: [edk2] [PATCH v3 0/6] Implement heap guard feature
> >
> > > 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.
> >
> > Cc: Star Zeng <star.zeng@intel.com>
> > Cc: Eric Dong <eric.dong@intel.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Michael Kinney <michael.d.kinney@intel.com>
> > Cc: Ayellet Wolman <ayellet.wolman@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>
> >
> > Jian J Wang (6):
> >   MdeModulePkg/DxeCore: Implement heap guard feature for UEFI
> >   MdeModulePkg/PiSmmCore: Implement heap guard feature for SMM
> mode
> >   MdeModulePkg/MdeModulePkg.dec,.uni: Add Protocol, PCDs and string
> >     tokens
> >   UefiCpuPkg/CpuDxe: Reduce debug message
> >   UefiCpuPkg/PiSmmCpuDxeSmm: Disable page table protection
> >   MdeModulePkg/DxeIpl: Enable paging for heap guard
> >
> >  MdeModulePkg/Core/Dxe/DxeMain.inf                  |    4 +
> >  MdeModulePkg/Core/Dxe/Mem/HeapGuard.c              | 1184
> > ++++++++++++++++
> >  MdeModulePkg/Core/Dxe/Mem/HeapGuard.h              |  380
> +++++
> >  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    |   29 +-
> >  MdeModulePkg/Core/PiSmmCore/HeapGuard.c            | 1469
> > ++++++++++++++++++++
> >  MdeModulePkg/Core/PiSmmCore/HeapGuard.h            |  399
> ++++++
> >  MdeModulePkg/Core/PiSmmCore/Page.c                 |   51 +-
> >  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           |    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 +-
> >  25 files changed, 4472 insertions(+), 96 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 v3 0/6] Implement heap guard feature

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

Thanks for the feedback. I'll change the patch order in v4.

> -----Original Message-----
> From: Zeng, Star
> Sent: Thursday, October 26, 2017 2:53 PM
> To: Yao, Jiewen <jiewen.yao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Wolman, Ayellet
> <ayellet.wolman@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star
> <star.zeng@intel.com>
> Subject: RE: [edk2] [PATCH v3 0/6] Implement heap guard feature
> 
> I suggest putting [3/6] at the first patch as it adds definitions that are used by
> other patches.
> 
> Thanks,
> Star
> -----Original Message-----
> From: Yao, Jiewen
> Sent: Thursday, October 26, 2017 2:49 PM
> To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Wolman, Ayellet
> <ayellet.wolman@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: RE: [edk2] [PATCH v3 0/6] Implement heap guard feature
> 
> That is great work. Jian.
> 
> Some suggestion for your consideration:
> 
> 0) I suggest add Laszlo to review SMM part, and add Ruiyu to review
> SMM_MEMORY_ATTRIBUTE_PROTOCOL.
> 
> 1) Would you please mention what test we have done for this feature?
> Such as OVMF/realPlatform? IA32/X64?
> 
> Have you validated NT32? Or try to enable protection? :-)
> 
> 2) Is that any dependency of this patch?
> I think there is OPENSSL wrapper reallocate() issue not resolved yet.
> 
> I suggest we check in all dependent patch at first, then check in this one.
> 
> 3) If you need submit V4, please separate
> MdeModulePkg/Include/Protocol/SmmMemoryAttribute.h from 2/6 to be a
> standalone patch. In general, an interface and an implementation are separated.
> 
> 
> Thank you
> Yao Jiewen
> 
> 
> 
> > -----Original Message-----
> > From: Wang, Jian J
> > Sent: Wednesday, October 25, 2017 9:48 AM
> > To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Wolman, Ayellet
> > <ayellet.wolman@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Dong,
> > Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> > Subject: RE: [edk2] [PATCH v3 0/6] Implement heap guard feature
> >
> > Hi,
> >
> > Just a warm reminding. I didn't see any feedbacks on the v3 patch.
> > If no more comments, I'll check in the patch soon.
> >
> > 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: Kinney, Michael D <michael.d.kinney@intel.com>; Wolman, Ayellet
> > > <ayellet.wolman@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> > > Dong, Eric <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> > > Subject: [edk2] [PATCH v3 0/6] Implement heap guard feature
> > >
> > > > 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.
> > >
> > > Cc: Star Zeng <star.zeng@intel.com>
> > > Cc: Eric Dong <eric.dong@intel.com>
> > > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > Cc: Michael Kinney <michael.d.kinney@intel.com>
> > > Cc: Ayellet Wolman <ayellet.wolman@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>
> > >
> > > Jian J Wang (6):
> > >   MdeModulePkg/DxeCore: Implement heap guard feature for UEFI
> > >   MdeModulePkg/PiSmmCore: Implement heap guard feature for SMM
> > mode
> > >   MdeModulePkg/MdeModulePkg.dec,.uni: Add Protocol, PCDs and string
> > >     tokens
> > >   UefiCpuPkg/CpuDxe: Reduce debug message
> > >   UefiCpuPkg/PiSmmCpuDxeSmm: Disable page table protection
> > >   MdeModulePkg/DxeIpl: Enable paging for heap guard
> > >
> > >  MdeModulePkg/Core/Dxe/DxeMain.inf                  |    4 +
> > >  MdeModulePkg/Core/Dxe/Mem/HeapGuard.c              | 1184
> > > ++++++++++++++++
> > >  MdeModulePkg/Core/Dxe/Mem/HeapGuard.h              |  380
> > +++++
> > >  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    |   29 +-
> > >  MdeModulePkg/Core/PiSmmCore/HeapGuard.c            | 1469
> > > ++++++++++++++++++++
> > >  MdeModulePkg/Core/PiSmmCore/HeapGuard.h            |  399
> > ++++++
> > >  MdeModulePkg/Core/PiSmmCore/Page.c                 |   51 +-
> > >  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           |    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 +-
> > >  25 files changed, 4472 insertions(+), 96 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 v3 0/6] Implement heap guard feature

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

Thanks for the feedback.

> -----Original Message-----
> From: Yao, Jiewen
> Sent: Thursday, October 26, 2017 2:49 PM
> To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Wolman, Ayellet
> <ayellet.wolman@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star
> <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> Subject: RE: [edk2] [PATCH v3 0/6] Implement heap guard feature
> 
> That is great work. Jian.
> 
> Some suggestion for your consideration:
> 
> 0) I suggest add Laszlo to review SMM part, and add Ruiyu to review
> SMM_MEMORY_ATTRIBUTE_PROTOCOL.
> 

Ok, already pinged them.

> 1) Would you please mention what test we have done for this feature?
> Such as OVMF/realPlatform? IA32/X64?
> 

I did following test:

Boot to shell (OVMF/Intel platform) (both IA32 and X64)
Boot to Fedora 25 (64 only)

Windows 10 boot loader has a limit of 512-memory-descriptor, which will
cause boot failure. This is due to a fact that enabling this feature will cause
more memory fragments (pool memory). Since this is a debug feature, I suppose
this is an acceptable result.

> Have you validated NT32? Or try to enable protection? :-)
> 

I did before first round of patch but not after. Let me check it again. Enabling
protection in NT32 is a challenge because Windows API has some limitations
which may need a huge changes to our memory management code. I'd suggest
to leave NT32 alone or create a separate task for it.

> 2) Is that any dependency of this patch?
> I think there is OPENSSL wrapper reallocate() issue not resolved yet.
> 
> I suggest we check in all dependent patch at first, then check in this one.
> 

You're right.

> 3) If you need submit V4, please separate
> MdeModulePkg/Include/Protocol/SmmMemoryAttribute.h from 2/6 to be a
> standalone patch. In general, an interface and an implementation are separated.
> 

Sure. 

> 
> Thank you
> Yao Jiewen
> 
> 
> 
> > -----Original Message-----
> > From: Wang, Jian J
> > Sent: Wednesday, October 25, 2017 9:48 AM
> > To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Wolman, Ayellet
> > <ayellet.wolman@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Dong,
> Eric
> > <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> > Subject: RE: [edk2] [PATCH v3 0/6] Implement heap guard feature
> >
> > Hi,
> >
> > Just a warm reminding. I didn't see any feedbacks on the v3 patch.
> > If no more comments, I'll check in the patch soon.
> >
> > 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: Kinney, Michael D <michael.d.kinney@intel.com>; Wolman, Ayellet
> > > <ayellet.wolman@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Dong,
> Eric
> > > <eric.dong@intel.com>; Zeng, Star <star.zeng@intel.com>
> > > Subject: [edk2] [PATCH v3 0/6] Implement heap guard feature
> > >
> > > > 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.
> > >
> > > Cc: Star Zeng <star.zeng@intel.com>
> > > Cc: Eric Dong <eric.dong@intel.com>
> > > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > Cc: Michael Kinney <michael.d.kinney@intel.com>
> > > Cc: Ayellet Wolman <ayellet.wolman@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>
> > >
> > > Jian J Wang (6):
> > >   MdeModulePkg/DxeCore: Implement heap guard feature for UEFI
> > >   MdeModulePkg/PiSmmCore: Implement heap guard feature for SMM
> > mode
> > >   MdeModulePkg/MdeModulePkg.dec,.uni: Add Protocol, PCDs and string
> > >     tokens
> > >   UefiCpuPkg/CpuDxe: Reduce debug message
> > >   UefiCpuPkg/PiSmmCpuDxeSmm: Disable page table protection
> > >   MdeModulePkg/DxeIpl: Enable paging for heap guard
> > >
> > >  MdeModulePkg/Core/Dxe/DxeMain.inf                  |    4 +
> > >  MdeModulePkg/Core/Dxe/Mem/HeapGuard.c              | 1184
> > > ++++++++++++++++
> > >  MdeModulePkg/Core/Dxe/Mem/HeapGuard.h              |  380
> > +++++
> > >  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    |   29 +-
> > >  MdeModulePkg/Core/PiSmmCore/HeapGuard.c            | 1469
> > > ++++++++++++++++++++
> > >  MdeModulePkg/Core/PiSmmCore/HeapGuard.h            |  399
> > ++++++
> > >  MdeModulePkg/Core/PiSmmCore/Page.c                 |   51 +-
> > >  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           |    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 +-
> > >  25 files changed, 4472 insertions(+), 96 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 v3 0/6] Implement heap guard feature

Posted by Laszlo Ersek 6 years, 5 months ago

Hi Jian,

On 10/26/17 09:38, Wang, Jian J wrote:
> Thanks for the feedback.
> 
>> -----Original Message-----
>> From: Yao, Jiewen
>> Sent: Thursday, October 26, 2017 2:49 PM
>> To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Wolman, Ayellet
>> <ayellet.wolman@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star
>> <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
>> Subject: RE: [edk2] [PATCH v3 0/6] Implement heap guard feature
>>
>> That is great work. Jian.
>>
>> Some suggestion for your consideration:
>>
>> 0) I suggest add Laszlo to review SMM part, and add Ruiyu to review
>> SMM_MEMORY_ATTRIBUTE_PROTOCOL.
>>
> 
> Ok, already pinged them.
> 
>> 1) Would you please mention what test we have done for this feature?
>> Such as OVMF/realPlatform? IA32/X64?
>>
> 
> I did following test:
> 
> Boot to shell (OVMF/Intel platform) (both IA32 and X64)
> Boot to Fedora 25 (64 only)

May I ask if you used KVM virtualization (i.e., a Linux host computer)
for this?

https://github.com/tianocore/tianocore.github.io/wiki/Testing-SMM-with-QEMU,-KVM-and-libvirt

> Windows 10 boot loader has a limit of 512-memory-descriptor, which will
> cause boot failure. This is due to a fact that enabling this feature will cause
> more memory fragments (pool memory). Since this is a debug feature, I suppose
> this is an acceptable result.

This feature is large; I can't even attempt to review it in the time
that I could allocate to it.

However, I would like to regression test it (thank you Jiewen for the
reference!) Preferably, given that a v4 is already planned, I should
test v4.

If you can post v4 on Oct 27th (tomorrow), I'll make an effort to test
it in the afternoon / evening, on the 27th. (Please CC me.) Next week I
will be mostly inactive on edk2-devel -- I wouldn't like to block your
work, but I also wouldn't like an OVMF regression.

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

Re: [edk2] [PATCH v3 0/6] Implement heap guard feature

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

Hi Laszlo,

> -----Original Message-----
> From: Laszlo Ersek [mailto:lersek@redhat.com]
> Sent: Thursday, October 26, 2017 9:27 PM
> To: Wang, Jian J <jian.j.wang@intel.com>
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org; Kinney,
> Michael D <michael.d.kinney@intel.com>; Wolman, Ayellet
> <ayellet.wolman@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star
> <star.zeng@intel.com>
> Subject: Re: [edk2] [PATCH v3 0/6] Implement heap guard feature
> 
> Hi Jian,
> 
> On 10/26/17 09:38, Wang, Jian J wrote:
> > Thanks for the feedback.
> >
> >> -----Original Message-----
> >> From: Yao, Jiewen
> >> Sent: Thursday, October 26, 2017 2:49 PM
> >> To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
> >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Wolman, Ayellet
> >> <ayellet.wolman@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star
> >> <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
> >> Subject: RE: [edk2] [PATCH v3 0/6] Implement heap guard feature
> >>
> >> That is great work. Jian.
> >>
> >> Some suggestion for your consideration:
> >>
> >> 0) I suggest add Laszlo to review SMM part, and add Ruiyu to review
> >> SMM_MEMORY_ATTRIBUTE_PROTOCOL.
> >>
> >
> > Ok, already pinged them.
> >
> >> 1) Would you please mention what test we have done for this feature?
> >> Such as OVMF/realPlatform? IA32/X64?
> >>
> >
> > I did following test:
> >
> > Boot to shell (OVMF/Intel platform) (both IA32 and X64)
> > Boot to Fedora 25 (64 only)
> 
> May I ask if you used KVM virtualization (i.e., a Linux host computer)
> for this?
> 
> https://github.com/tianocore/tianocore.github.io/wiki/Testing-SMM-with-
> QEMU,-KVM-and-libvirt
> 

No, I'm using Qemu on Windows. I think Qemu doesn't support VM on Windows
machine but I do enabled SMM mode for it. Please let me know if there's any 
differences between them I should be aware of.

> > Windows 10 boot loader has a limit of 512-memory-descriptor, which will
> > cause boot failure. This is due to a fact that enabling this feature will cause
> > more memory fragments (pool memory). Since this is a debug feature, I
> suppose
> > this is an acceptable result.
> 
> This feature is large; I can't even attempt to review it in the time
> that I could allocate to it.
> 
> However, I would like to regression test it (thank you Jiewen for the
> reference!) Preferably, given that a v4 is already planned, I should
> test v4.
> 
> If you can post v4 on Oct 27th (tomorrow), I'll make an effort to test
> it in the afternoon / evening, on the 27th. (Please CC me.) Next week I
> will be mostly inactive on edk2-devel -- I wouldn't like to block your
> work, but I also wouldn't like an OVMF regression.
> 

Thanks for trying. I'll try my best to send v4 today. I'd remind you in advance
I have already found heap overflow (just read) in UiApp and Openssl code.

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

Re: [edk2] [PATCH v3 0/6] Implement heap guard feature

Posted by Laszlo Ersek 6 years, 5 months ago

On 10/27/17 03:39, Wang, Jian J wrote:
> Hi Laszlo,
> 
>> -----Original Message-----
>> From: Laszlo Ersek [mailto:lersek@redhat.com]
>> Sent: Thursday, October 26, 2017 9:27 PM
>> To: Wang, Jian J <jian.j.wang@intel.com>
>> Cc: Yao, Jiewen <jiewen.yao@intel.com>; edk2-devel@lists.01.org; Kinney,
>> Michael D <michael.d.kinney@intel.com>; Wolman, Ayellet
>> <ayellet.wolman@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star
>> <star.zeng@intel.com>
>> Subject: Re: [edk2] [PATCH v3 0/6] Implement heap guard feature
>>
>> Hi Jian,
>>
>> On 10/26/17 09:38, Wang, Jian J wrote:
>>> Thanks for the feedback.
>>>
>>>> -----Original Message-----
>>>> From: Yao, Jiewen
>>>> Sent: Thursday, October 26, 2017 2:49 PM
>>>> To: Wang, Jian J <jian.j.wang@intel.com>; edk2-devel@lists.01.org
>>>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Wolman, Ayellet
>>>> <ayellet.wolman@intel.com>; Dong, Eric <eric.dong@intel.com>; Zeng, Star
>>>> <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>
>>>> Subject: RE: [edk2] [PATCH v3 0/6] Implement heap guard feature
>>>>
>>>> That is great work. Jian.
>>>>
>>>> Some suggestion for your consideration:
>>>>
>>>> 0) I suggest add Laszlo to review SMM part, and add Ruiyu to review
>>>> SMM_MEMORY_ATTRIBUTE_PROTOCOL.
>>>>
>>>
>>> Ok, already pinged them.
>>>
>>>> 1) Would you please mention what test we have done for this feature?
>>>> Such as OVMF/realPlatform? IA32/X64?
>>>>
>>>
>>> I did following test:
>>>
>>> Boot to shell (OVMF/Intel platform) (both IA32 and X64)
>>> Boot to Fedora 25 (64 only)
>>
>> May I ask if you used KVM virtualization (i.e., a Linux host computer)
>> for this?
>>
>> https://github.com/tianocore/tianocore.github.io/wiki/Testing-SMM-with-
>> QEMU,-KVM-and-libvirt
>>
> 
> No, I'm using Qemu on Windows. I think Qemu doesn't support VM on Windows
> machine but I do enabled SMM mode for it. Please let me know if there's any 
> differences between them I should be aware of.

QEMU on Windows uses software emulation / just in time compilation. It
is called the "TCG accelerator" of QEMU, with TCG standing for "Tiny
Code Generator". This is very useful for development purposes (and for
cross-architectural emulation). It is also very fast relative to other
software emulators.

However, for production use and testing, hardware virtualization should
be used, and that requires the KVM accelerator (= "Kernel Virtual
Machine", meaning a Linux host computer). SMM emulation is imlemented
very differently in TCG and in KVM. Both TCG and KVM support
multiprocessing (multiple logical CPUs in the guest), mapping VCPUs to
separate host-side threads. Most of the time KVM is better at exposing
multiprocessing bugs in guest code.

Generally, KVM (= Linux host) should be used for SMM testing, because
that's how OVMF is used in production.

>>> Windows 10 boot loader has a limit of 512-memory-descriptor, which will
>>> cause boot failure. This is due to a fact that enabling this feature will cause
>>> more memory fragments (pool memory). Since this is a debug feature, I
>> suppose
>>> this is an acceptable result.
>>
>> This feature is large; I can't even attempt to review it in the time
>> that I could allocate to it.
>>
>> However, I would like to regression test it (thank you Jiewen for the
>> reference!) Preferably, given that a v4 is already planned, I should
>> test v4.
>>
>> If you can post v4 on Oct 27th (tomorrow), I'll make an effort to test
>> it in the afternoon / evening, on the 27th. (Please CC me.) Next week I
>> will be mostly inactive on edk2-devel -- I wouldn't like to block your
>> work, but I also wouldn't like an OVMF regression.
>>
> 
> Thanks for trying. I'll try my best to send v4 today. I'd remind you in advance
> I have already found heap overflow (just read) in UiApp and Openssl code.

Great!

My understanding is that the feature is not enabled by default, so I
primarily intend to test that OVMF works as before, with your patches
applied.

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