Re: [edk2] [PATCH v4 6/6] MdeModulePkg/Core: add freed-memory guard feature

2018-10-25 Thread Wang, Jian J
Sure. Thanks.

Regards,
Jian


> -Original Message-
> From: Zeng, Star
> Sent: Friday, October 26, 2018 9:19 AM
> To: Wang, Jian J ; edk2-devel@lists.01.org
> Cc: Kinney, Michael D ; Ni, Ruiyu
> ; Yao, Jiewen ; Laszlo Ersek
> ; Zeng, Star 
> Subject: Re: [edk2] [PATCH v4 6/6] MdeModulePkg/Core: add freed-memory
> guard feature
> 
> On 2018/10/25 15:18, Jian J Wang wrote:
> >> v4 changes:
> >> a. replace hard-coded memory attributes with the value got from
> >> CoreGetMemorySpaceDescriptor()
> >> b. remove the enclosure of CoreAcquireGcdMemoryLock() and
> >> CoreReleaseGcdMemoryLock() around CoreAddRange()
> >
> > Freed-memory guard is used to detect UAF (Use-After-Free) memory issue
> > which is illegal access to memory which has been freed. The principle
> > behind is similar to heap guard feature, that is we'll turn all pool
> 
> Remember to use "pool/page heap guard feature" instead of "heap guard
> feature" here. No need to send new patch series for it.
> 
> Thanks,
> Star
> 
> > memory allocation to page allocation and mark them to be not-present
> > once they are freed.
> >
> > This also implies that, once a page is allocated and freed, it cannot
> > be re-allocated. This will bring another issue, which is that there's
> > risk that memory space will be used out. To address it, the memory
> > service add logic to put part (at most 64 pages a time) of freed pages
> > back into page pool, so that the memory service can still have memory
> > to allocate, when all memory space have been allocated once. This is
> > called memory promotion. The promoted pages are always from the eldest
> > pages which haven been freed.
> >
> > This feature brings another problem is that memory map descriptors will
> > be increased enormously (200+ -> 2000+). One of change in this patch
> > is to update MergeMemoryMap() in file PropertiesTable.c to allow merge
> > freed pages back into the memory map. Now the number can stay at around
> > 510.
> >
> > Cc: Star Zeng 
> > Cc: Michael D Kinney 
> > Cc: Jiewen Yao 
> > Cc: Ruiyu Ni 
> > Cc: Laszlo Ersek 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jian J Wang 
> > ---
> >   MdeModulePkg/Core/Dxe/Mem/HeapGuard.c | 409
> +-
> >   MdeModulePkg/Core/Dxe/Mem/HeapGuard.h |  65 +++-
> >   MdeModulePkg/Core/Dxe/Mem/Page.c  |  42 ++-
> >   MdeModulePkg/Core/Dxe/Mem/Pool.c  |  23 +-
> >   MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c |   2 +-
> >   MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c  |  18 +-
> >   6 files changed, 525 insertions(+), 34 deletions(-)
> >
> > diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> > index 663f969c0d..449a022658 100644
> > --- a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> > +++ b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
> > @@ -44,6 +44,11 @@ GLOBAL_REMOVE_IF_UNREFERENCED UINTN
> mLevelShift[GUARDED_HEAP_MAP_TABLE_DEPTH]
> >   GLOBAL_REMOVE_IF_UNREFERENCED UINTN
> mLevelMask[GUARDED_HEAP_MAP_TABLE_DEPTH]
> >   = GUARDED_HEAP_MAP_TABLE_DEPTH_MASKS;
> >
> > +//
> > +// Used for promoting freed but not used pages.
> > +//
> > +GLOBAL_REMOVE_IF_UNREFERENCED EFI_PHYSICAL_ADDRESS
> mLastPromotedPage = BASE_4GB;
> > +
> >   /**
> > Set corresponding bits in bitmap table to 1 according to the address.
> >
> > @@ -379,7 +384,7 @@ ClearGuardedMemoryBits (
> >
> > @return An integer containing the guarded memory bitmap.
> >   **/
> > -UINTN
> > +UINT64
> >   GetGuardedMemoryBits (
> > IN EFI_PHYSICAL_ADDRESSAddress,
> > IN UINTN   NumberOfPages
> > @@ -387,7 +392,7 @@ GetGuardedMemoryBits (
> >   {
> > UINT64*BitMap;
> > UINTN Bits;
> > -  UINTN Result;
> > +  UINT64Result;
> > UINTN Shift;
> > UINTN BitsToUnitEnd;
> >
> > @@ -660,15 +665,16 @@ IsPageTypeToGuard (
> >   /**
> > Check to see if the heap guard is enabled for page and/or pool 
> > allocation.
> >
> > +  @param[in]  GuardType   Specify the sub-type(s) of Heap Guard.
> > +
> > @return TRUE/FALSE.
> >   **/
> >   BOOLEAN
> >   IsHeapGuardEnabled (
> > -  VOID
> > +  UINT8   GuardType
> > )
> >   {
> &g

Re: [edk2] [PATCH v4 6/6] MdeModulePkg/Core: add freed-memory guard feature

2018-10-25 Thread Zeng, Star

On 2018/10/25 15:18, Jian J Wang wrote:

v4 changes:
a. replace hard-coded memory attributes with the value got from
CoreGetMemorySpaceDescriptor()
b. remove the enclosure of CoreAcquireGcdMemoryLock() and
CoreReleaseGcdMemoryLock() around CoreAddRange()


Freed-memory guard is used to detect UAF (Use-After-Free) memory issue
which is illegal access to memory which has been freed. The principle
behind is similar to heap guard feature, that is we'll turn all pool


Remember to use "pool/page heap guard feature" instead of "heap guard 
feature" here. No need to send new patch series for it.


Thanks,
Star


memory allocation to page allocation and mark them to be not-present
once they are freed.

This also implies that, once a page is allocated and freed, it cannot
be re-allocated. This will bring another issue, which is that there's
risk that memory space will be used out. To address it, the memory
service add logic to put part (at most 64 pages a time) of freed pages
back into page pool, so that the memory service can still have memory
to allocate, when all memory space have been allocated once. This is
called memory promotion. The promoted pages are always from the eldest
pages which haven been freed.

This feature brings another problem is that memory map descriptors will
be increased enormously (200+ -> 2000+). One of change in this patch
is to update MergeMemoryMap() in file PropertiesTable.c to allow merge
freed pages back into the memory map. Now the number can stay at around
510.

Cc: Star Zeng 
Cc: Michael D Kinney 
Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
  MdeModulePkg/Core/Dxe/Mem/HeapGuard.c | 409 +-
  MdeModulePkg/Core/Dxe/Mem/HeapGuard.h |  65 +++-
  MdeModulePkg/Core/Dxe/Mem/Page.c  |  42 ++-
  MdeModulePkg/Core/Dxe/Mem/Pool.c  |  23 +-
  MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c |   2 +-
  MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c  |  18 +-
  6 files changed, 525 insertions(+), 34 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c 
b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
index 663f969c0d..449a022658 100644
--- a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
+++ b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
@@ -44,6 +44,11 @@ GLOBAL_REMOVE_IF_UNREFERENCED UINTN 
mLevelShift[GUARDED_HEAP_MAP_TABLE_DEPTH]
  GLOBAL_REMOVE_IF_UNREFERENCED UINTN mLevelMask[GUARDED_HEAP_MAP_TABLE_DEPTH]
  = GUARDED_HEAP_MAP_TABLE_DEPTH_MASKS;
  
+//

+// Used for promoting freed but not used pages.
+//
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_PHYSICAL_ADDRESS mLastPromotedPage = 
BASE_4GB;
+
  /**
Set corresponding bits in bitmap table to 1 according to the address.
  
@@ -379,7 +384,7 @@ ClearGuardedMemoryBits (
  
@return An integer containing the guarded memory bitmap.

  **/
-UINTN
+UINT64
  GetGuardedMemoryBits (
IN EFI_PHYSICAL_ADDRESSAddress,
IN UINTN   NumberOfPages
@@ -387,7 +392,7 @@ GetGuardedMemoryBits (
  {
UINT64*BitMap;
UINTN Bits;
-  UINTN Result;
+  UINT64Result;
UINTN Shift;
UINTN BitsToUnitEnd;
  
@@ -660,15 +665,16 @@ IsPageTypeToGuard (

  /**
Check to see if the heap guard is enabled for page and/or pool allocation.
  
+  @param[in]  GuardType   Specify the sub-type(s) of Heap Guard.

+
@return TRUE/FALSE.
  **/
  BOOLEAN
  IsHeapGuardEnabled (
-  VOID
+  UINT8   GuardType
)
  {
-  return IsMemoryTypeToGuard (EfiMaxMemoryType, AllocateAnyPages,
-  GUARD_HEAP_TYPE_POOL|GUARD_HEAP_TYPE_PAGE);
+  return IsMemoryTypeToGuard (EfiMaxMemoryType, AllocateAnyPages, GuardType);
  }
  
  /**

@@ -1203,6 +1209,380 @@ SetAllGuardPages (
}
  }
  
+/**

+  Find the address of top-most guarded free page.
+
+  @param[out]  AddressStart address of top-most guarded free page.
+
+  @return VOID.
+**/
+VOID
+GetLastGuardedFreePageAddress (
+  OUT EFI_PHYSICAL_ADDRESS  *Address
+  )
+{
+  EFI_PHYSICAL_ADDRESSAddressGranularity;
+  EFI_PHYSICAL_ADDRESSBaseAddress;
+  UINTN   Level;
+  UINT64  Map;
+  INTNIndex;
+
+  ASSERT (mMapLevel >= 1);
+
+  BaseAddress = 0;
+  Map = mGuardedMemoryMap;
+  for (Level = GUARDED_HEAP_MAP_TABLE_DEPTH - mMapLevel;
+   Level < GUARDED_HEAP_MAP_TABLE_DEPTH;
+   ++Level) {
+AddressGranularity = LShiftU64 (1, mLevelShift[Level]);
+
+//
+// Find the non-NULL entry at largest index.
+//
+for (Index = (INTN)mLevelMask[Level]; Index >= 0 ; --Index) {
+  if (((UINT64 *)(UINTN)Map)[Index] != 0) {
+BaseAddress += MultU64x32 (AddressGranularity, (UINT32)Index);
+Map = ((UINT64 *)(UINTN)Map)[Index];
+break;
+  }
+}
+  }
+
+  //
+  // Find the non-zero MSB then get the page address.
+  //
+  

[edk2] [PATCH v4 6/6] MdeModulePkg/Core: add freed-memory guard feature

2018-10-25 Thread Jian J Wang
> v4 changes:
> a. replace hard-coded memory attributes with the value got from
>CoreGetMemorySpaceDescriptor()
> b. remove the enclosure of CoreAcquireGcdMemoryLock() and
>CoreReleaseGcdMemoryLock() around CoreAddRange()

Freed-memory guard is used to detect UAF (Use-After-Free) memory issue
which is illegal access to memory which has been freed. The principle
behind is similar to heap guard feature, that is we'll turn all pool
memory allocation to page allocation and mark them to be not-present
once they are freed.

This also implies that, once a page is allocated and freed, it cannot
be re-allocated. This will bring another issue, which is that there's
risk that memory space will be used out. To address it, the memory
service add logic to put part (at most 64 pages a time) of freed pages
back into page pool, so that the memory service can still have memory
to allocate, when all memory space have been allocated once. This is
called memory promotion. The promoted pages are always from the eldest
pages which haven been freed.

This feature brings another problem is that memory map descriptors will
be increased enormously (200+ -> 2000+). One of change in this patch
is to update MergeMemoryMap() in file PropertiesTable.c to allow merge
freed pages back into the memory map. Now the number can stay at around
510.

Cc: Star Zeng 
Cc: Michael D Kinney 
Cc: Jiewen Yao 
Cc: Ruiyu Ni 
Cc: Laszlo Ersek 
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang 
---
 MdeModulePkg/Core/Dxe/Mem/HeapGuard.c | 409 +-
 MdeModulePkg/Core/Dxe/Mem/HeapGuard.h |  65 +++-
 MdeModulePkg/Core/Dxe/Mem/Page.c  |  42 ++-
 MdeModulePkg/Core/Dxe/Mem/Pool.c  |  23 +-
 MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c |   2 +-
 MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c  |  18 +-
 6 files changed, 525 insertions(+), 34 deletions(-)

diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c 
b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
index 663f969c0d..449a022658 100644
--- a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
+++ b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c
@@ -44,6 +44,11 @@ GLOBAL_REMOVE_IF_UNREFERENCED UINTN 
mLevelShift[GUARDED_HEAP_MAP_TABLE_DEPTH]
 GLOBAL_REMOVE_IF_UNREFERENCED UINTN mLevelMask[GUARDED_HEAP_MAP_TABLE_DEPTH]
 = GUARDED_HEAP_MAP_TABLE_DEPTH_MASKS;
 
+//
+// Used for promoting freed but not used pages.
+//
+GLOBAL_REMOVE_IF_UNREFERENCED EFI_PHYSICAL_ADDRESS mLastPromotedPage = 
BASE_4GB;
+
 /**
   Set corresponding bits in bitmap table to 1 according to the address.
 
@@ -379,7 +384,7 @@ ClearGuardedMemoryBits (
 
   @return An integer containing the guarded memory bitmap.
 **/
-UINTN
+UINT64
 GetGuardedMemoryBits (
   IN EFI_PHYSICAL_ADDRESSAddress,
   IN UINTN   NumberOfPages
@@ -387,7 +392,7 @@ GetGuardedMemoryBits (
 {
   UINT64*BitMap;
   UINTN Bits;
-  UINTN Result;
+  UINT64Result;
   UINTN Shift;
   UINTN BitsToUnitEnd;
 
@@ -660,15 +665,16 @@ IsPageTypeToGuard (
 /**
   Check to see if the heap guard is enabled for page and/or pool allocation.
 
+  @param[in]  GuardType   Specify the sub-type(s) of Heap Guard.
+
   @return TRUE/FALSE.
 **/
 BOOLEAN
 IsHeapGuardEnabled (
-  VOID
+  UINT8   GuardType
   )
 {
-  return IsMemoryTypeToGuard (EfiMaxMemoryType, AllocateAnyPages,
-  GUARD_HEAP_TYPE_POOL|GUARD_HEAP_TYPE_PAGE);
+  return IsMemoryTypeToGuard (EfiMaxMemoryType, AllocateAnyPages, GuardType);
 }
 
 /**
@@ -1203,6 +1209,380 @@ SetAllGuardPages (
   }
 }
 
+/**
+  Find the address of top-most guarded free page.
+
+  @param[out]  AddressStart address of top-most guarded free page.
+
+  @return VOID.
+**/
+VOID
+GetLastGuardedFreePageAddress (
+  OUT EFI_PHYSICAL_ADDRESS  *Address
+  )
+{
+  EFI_PHYSICAL_ADDRESSAddressGranularity;
+  EFI_PHYSICAL_ADDRESSBaseAddress;
+  UINTN   Level;
+  UINT64  Map;
+  INTNIndex;
+
+  ASSERT (mMapLevel >= 1);
+
+  BaseAddress = 0;
+  Map = mGuardedMemoryMap;
+  for (Level = GUARDED_HEAP_MAP_TABLE_DEPTH - mMapLevel;
+   Level < GUARDED_HEAP_MAP_TABLE_DEPTH;
+   ++Level) {
+AddressGranularity = LShiftU64 (1, mLevelShift[Level]);
+
+//
+// Find the non-NULL entry at largest index.
+//
+for (Index = (INTN)mLevelMask[Level]; Index >= 0 ; --Index) {
+  if (((UINT64 *)(UINTN)Map)[Index] != 0) {
+BaseAddress += MultU64x32 (AddressGranularity, (UINT32)Index);
+Map = ((UINT64 *)(UINTN)Map)[Index];
+break;
+  }
+}
+  }
+
+  //
+  // Find the non-zero MSB then get the page address.
+  //
+  while (Map != 0) {
+Map = RShiftU64 (Map, 1);
+BaseAddress += EFI_PAGES_TO_SIZE (1);
+  }
+
+  *Address = BaseAddress;
+}
+
+/**
+  Record freed pages.
+
+  @param[in]  BaseAddress   Base address of just freed pages.
+