public inbox for devel@edk2.groups.io
 help / color / mirror / Atom feed
* [PATCH] MdeModulePkg PiSmmCore: Enhance SMM FreePool to catch buffer overflow
@ 2017-04-19  6:06 Star Zeng
  2017-04-19  7:26 ` Yao, Jiewen
  0 siblings, 1 reply; 3+ messages in thread
From: Star Zeng @ 2017-04-19  6:06 UTC (permalink / raw)
  To: edk2-devel; +Cc: Star Zeng, Jiewen Yao, Hao Wu

This solution is equivalent to DXE core.

AllocatePool() allocates POOL_TAIL after the buffer.
This POOL_TAIL is checked at FreePool().
If the there is buffer overflow, the issue can be caught at FreePool().

This patch could also handle the eight-byte aligned allocation
requirement. The discussion related to the eight-byte aligned
allocation requirement is at
https://lists.01.org/pipermail/edk2-devel/2017-April/009995.html.

According to the PI spec (Vol 4, Section 3.2 SmmAllocatePool()):
The SmmAllocatePool() function ... All allocations are eight-byte aligned.

Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Hao Wu <hao.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
---
 MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 22 ++++++++++++++---
 MdeModulePkg/Core/PiSmmCore/Pool.c      | 44 +++++++++++++++++++++++++++++++--
 2 files changed, 61 insertions(+), 5 deletions(-)

diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
index c12805a2dd96..b6f815c68da0 100644
--- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
+++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
@@ -1196,12 +1196,28 @@ extern LIST_ENTRY  mSmmMemoryMap;
 //
 #define MAX_POOL_INDEX  (MAX_POOL_SHIFT - MIN_POOL_SHIFT + 1)
 
+#define POOL_HEAD_SIGNATURE   SIGNATURE_32('p','h','d','0')
+
 typedef struct {
-  UINTN           Size;
-  BOOLEAN         Available;
-  EFI_MEMORY_TYPE Type;
+  UINT32            Signature;
+  BOOLEAN           Available;
+  EFI_MEMORY_TYPE   Type;
+  UINTN             Size;
 } POOL_HEADER;
 
+#define POOL_TAIL_SIGNATURE   SIGNATURE_32('p','t','a','l')
+
+typedef struct {
+  UINT32            Signature;
+  UINT32            Reserved;
+  UINTN             Size;
+} POOL_TAIL;
+
+#define POOL_OVERHEAD (sizeof(POOL_HEADER) + sizeof(POOL_TAIL))
+
+#define HEAD_TO_TAIL(a)   \
+  ((POOL_TAIL *) (((CHAR8 *) (a)) + (a)->Size - sizeof(POOL_TAIL)));
+
 typedef struct {
   POOL_HEADER  Header;
   LIST_ENTRY   Link;
diff --git a/MdeModulePkg/Core/PiSmmCore/Pool.c b/MdeModulePkg/Core/PiSmmCore/Pool.c
index 43ce869d1e1a..ebb9f8c49ee9 100644
--- a/MdeModulePkg/Core/PiSmmCore/Pool.c
+++ b/MdeModulePkg/Core/PiSmmCore/Pool.c
@@ -133,6 +133,7 @@ InternalAllocPoolByIndex (
 {
   EFI_STATUS            Status;
   FREE_POOL_HEADER      *Hdr;
+  POOL_TAIL             *Tail;
   EFI_PHYSICAL_ADDRESS  Address;
   SMM_POOL_TYPE         SmmPoolType;
 
@@ -154,18 +155,26 @@ InternalAllocPoolByIndex (
   } else {
     Status = InternalAllocPoolByIndex (PoolType, PoolIndex + 1, &Hdr);
     if (!EFI_ERROR (Status)) {
+      Hdr->Header.Signature = 0;
       Hdr->Header.Size >>= 1;
       Hdr->Header.Available = TRUE;
-      Hdr->Header.Type = PoolType;
+      Hdr->Header.Type = 0;
+      Tail = HEAD_TO_TAIL(&Hdr->Header);
+      Tail->Signature = 0;
+      Tail->Size = 0;
       InsertHeadList (&mSmmPoolLists[SmmPoolType][PoolIndex], &Hdr->Link);
       Hdr = (FREE_POOL_HEADER*)((UINT8*)Hdr + Hdr->Header.Size);
     }
   }
 
   if (!EFI_ERROR (Status)) {
+    Hdr->Header.Signature = POOL_HEAD_SIGNATURE;
     Hdr->Header.Size = MIN_POOL_SIZE << PoolIndex;
     Hdr->Header.Available = FALSE;
     Hdr->Header.Type = PoolType;
+    Tail = HEAD_TO_TAIL(&Hdr->Header);
+    Tail->Signature = POOL_TAIL_SIGNATURE;
+    Tail->Size = Hdr->Header.Size;
   }
 
   *FreePoolHdr = Hdr;
@@ -187,6 +196,7 @@ InternalFreePoolByIndex (
 {
   UINTN                 PoolIndex;
   SMM_POOL_TYPE         SmmPoolType;
+  POOL_TAIL             *PoolTail;
 
   ASSERT ((FreePoolHdr->Header.Size & (FreePoolHdr->Header.Size - 1)) == 0);
   ASSERT (((UINTN)FreePoolHdr & (FreePoolHdr->Header.Size - 1)) == 0);
@@ -195,7 +205,12 @@ InternalFreePoolByIndex (
   SmmPoolType = UefiMemoryTypeToSmmPoolType(FreePoolHdr->Header.Type);
 
   PoolIndex = (UINTN) (HighBitSet32 ((UINT32)FreePoolHdr->Header.Size) - MIN_POOL_SHIFT);
+  FreePoolHdr->Header.Signature = 0;
   FreePoolHdr->Header.Available = TRUE;
+  FreePoolHdr->Header.Type = 0;
+  PoolTail = HEAD_TO_TAIL(&FreePoolHdr->Header);
+  PoolTail->Signature = 0;
+  PoolTail->Size = 0;
   ASSERT (PoolIndex < MAX_POOL_INDEX);
   InsertHeadList (&mSmmPoolLists[SmmPoolType][PoolIndex], &FreePoolHdr->Link);
   return EFI_SUCCESS;
@@ -223,6 +238,7 @@ SmmInternalAllocatePool (
   )
 {
   POOL_HEADER           *PoolHdr;
+  POOL_TAIL             *PoolTail;
   FREE_POOL_HEADER      *FreePoolHdr;
   EFI_STATUS            Status;
   EFI_PHYSICAL_ADDRESS  Address;
@@ -235,7 +251,10 @@ SmmInternalAllocatePool (
     return EFI_INVALID_PARAMETER;
   }
 
-  Size += sizeof (*PoolHdr);
+  //
+  // Adjust the size by the pool header & tail overhead
+  //
+  Size += POOL_OVERHEAD;
   if (Size > MAX_POOL_SIZE) {
     Size = EFI_SIZE_TO_PAGES (Size);
     Status = SmmInternalAllocatePages (AllocateAnyPages, PoolType, Size, &Address);
@@ -244,9 +263,13 @@ SmmInternalAllocatePool (
     }
 
     PoolHdr = (POOL_HEADER*)(UINTN)Address;
+    PoolHdr->Signature = POOL_HEAD_SIGNATURE;
     PoolHdr->Size = EFI_PAGES_TO_SIZE (Size);
     PoolHdr->Available = FALSE;
     PoolHdr->Type = PoolType;
+    PoolTail = HEAD_TO_TAIL(PoolHdr);
+    PoolTail->Signature = POOL_TAIL_SIGNATURE;
+    PoolTail->Size = PoolHdr->Size;
     *Buffer = PoolHdr + 1;
     return Status;
   }
@@ -317,13 +340,30 @@ SmmInternalFreePool (
   )
 {
   FREE_POOL_HEADER  *FreePoolHdr;
+  POOL_TAIL         *PoolTail;
 
   if (Buffer == NULL) {
     return EFI_INVALID_PARAMETER;
   }
 
   FreePoolHdr = (FREE_POOL_HEADER*)((POOL_HEADER*)Buffer - 1);
+  ASSERT (FreePoolHdr->Header.Signature == POOL_HEAD_SIGNATURE);
   ASSERT (!FreePoolHdr->Header.Available);
+  PoolTail = HEAD_TO_TAIL(&FreePoolHdr->Header);
+  ASSERT (PoolTail->Signature == POOL_TAIL_SIGNATURE);
+  ASSERT (FreePoolHdr->Header.Size == PoolTail->Size);
+
+  if (FreePoolHdr->Header.Signature != POOL_HEAD_SIGNATURE) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (PoolTail->Signature != POOL_TAIL_SIGNATURE) {
+    return EFI_INVALID_PARAMETER;
+  }
+
+  if (FreePoolHdr->Header.Size != PoolTail->Size) {
+    return EFI_INVALID_PARAMETER;
+  }
 
   if (FreePoolHdr->Header.Size > MAX_POOL_SIZE) {
     ASSERT (((UINTN)FreePoolHdr & EFI_PAGE_MASK) == 0);
-- 
2.7.0.windows.1



^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] MdeModulePkg PiSmmCore: Enhance SMM FreePool to catch buffer overflow
  2017-04-19  6:06 [PATCH] MdeModulePkg PiSmmCore: Enhance SMM FreePool to catch buffer overflow Star Zeng
@ 2017-04-19  7:26 ` Yao, Jiewen
  2017-04-19 11:23   ` Wu, Hao A
  0 siblings, 1 reply; 3+ messages in thread
From: Yao, Jiewen @ 2017-04-19  7:26 UTC (permalink / raw)
  To: Zeng, Star, edk2-devel@lists.01.org; +Cc: Wu, Hao

Reviewed-by: jiewen.yao@intel.com

> -----Original Message-----
> From: Zeng, Star
> Sent: Wednesday, April 19, 2017 2:07 PM
> To: edk2-devel@lists.01.org
> Cc: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>; Wu,
> Hao <hao.wu@intel.com>
> Subject: [PATCH] MdeModulePkg PiSmmCore: Enhance SMM FreePool to catch
> buffer overflow
> 
> This solution is equivalent to DXE core.
> 
> AllocatePool() allocates POOL_TAIL after the buffer.
> This POOL_TAIL is checked at FreePool().
> If the there is buffer overflow, the issue can be caught at FreePool().
> 
> This patch could also handle the eight-byte aligned allocation
> requirement. The discussion related to the eight-byte aligned
> allocation requirement is at
> https://lists.01.org/pipermail/edk2-devel/2017-April/009995.html.
> 
> According to the PI spec (Vol 4, Section 3.2 SmmAllocatePool()):
> The SmmAllocatePool() function ... All allocations are eight-byte aligned.
> 
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Hao Wu <hao.wu@intel.com>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Star Zeng <star.zeng@intel.com>
> ---
>  MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 22 ++++++++++++++---
>  MdeModulePkg/Core/PiSmmCore/Pool.c      | 44
> +++++++++++++++++++++++++++++++--
>  2 files changed, 61 insertions(+), 5 deletions(-)
> 
> diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
> b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
> index c12805a2dd96..b6f815c68da0 100644
> --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
> +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
> @@ -1196,12 +1196,28 @@ extern LIST_ENTRY  mSmmMemoryMap;
>  //
>  #define MAX_POOL_INDEX  (MAX_POOL_SHIFT - MIN_POOL_SHIFT + 1)
> 
> +#define POOL_HEAD_SIGNATURE   SIGNATURE_32('p','h','d','0')
> +
>  typedef struct {
> -  UINTN           Size;
> -  BOOLEAN         Available;
> -  EFI_MEMORY_TYPE Type;
> +  UINT32            Signature;
> +  BOOLEAN           Available;
> +  EFI_MEMORY_TYPE   Type;
> +  UINTN             Size;
>  } POOL_HEADER;
> 
> +#define POOL_TAIL_SIGNATURE   SIGNATURE_32('p','t','a','l')
> +
> +typedef struct {
> +  UINT32            Signature;
> +  UINT32            Reserved;
> +  UINTN             Size;
> +} POOL_TAIL;
> +
> +#define POOL_OVERHEAD (sizeof(POOL_HEADER) + sizeof(POOL_TAIL))
> +
> +#define HEAD_TO_TAIL(a)   \
> +  ((POOL_TAIL *) (((CHAR8 *) (a)) + (a)->Size - sizeof(POOL_TAIL)));
> +
>  typedef struct {
>    POOL_HEADER  Header;
>    LIST_ENTRY   Link;
> diff --git a/MdeModulePkg/Core/PiSmmCore/Pool.c
> b/MdeModulePkg/Core/PiSmmCore/Pool.c
> index 43ce869d1e1a..ebb9f8c49ee9 100644
> --- a/MdeModulePkg/Core/PiSmmCore/Pool.c
> +++ b/MdeModulePkg/Core/PiSmmCore/Pool.c
> @@ -133,6 +133,7 @@ InternalAllocPoolByIndex (
>  {
>    EFI_STATUS            Status;
>    FREE_POOL_HEADER      *Hdr;
> +  POOL_TAIL             *Tail;
>    EFI_PHYSICAL_ADDRESS  Address;
>    SMM_POOL_TYPE         SmmPoolType;
> 
> @@ -154,18 +155,26 @@ InternalAllocPoolByIndex (
>    } else {
>      Status = InternalAllocPoolByIndex (PoolType, PoolIndex + 1, &Hdr);
>      if (!EFI_ERROR (Status)) {
> +      Hdr->Header.Signature = 0;
>        Hdr->Header.Size >>= 1;
>        Hdr->Header.Available = TRUE;
> -      Hdr->Header.Type = PoolType;
> +      Hdr->Header.Type = 0;
> +      Tail = HEAD_TO_TAIL(&Hdr->Header);
> +      Tail->Signature = 0;
> +      Tail->Size = 0;
>        InsertHeadList (&mSmmPoolLists[SmmPoolType][PoolIndex],
> &Hdr->Link);
>        Hdr = (FREE_POOL_HEADER*)((UINT8*)Hdr + Hdr->Header.Size);
>      }
>    }
> 
>    if (!EFI_ERROR (Status)) {
> +    Hdr->Header.Signature = POOL_HEAD_SIGNATURE;
>      Hdr->Header.Size = MIN_POOL_SIZE << PoolIndex;
>      Hdr->Header.Available = FALSE;
>      Hdr->Header.Type = PoolType;
> +    Tail = HEAD_TO_TAIL(&Hdr->Header);
> +    Tail->Signature = POOL_TAIL_SIGNATURE;
> +    Tail->Size = Hdr->Header.Size;
>    }
> 
>    *FreePoolHdr = Hdr;
> @@ -187,6 +196,7 @@ InternalFreePoolByIndex (
>  {
>    UINTN                 PoolIndex;
>    SMM_POOL_TYPE         SmmPoolType;
> +  POOL_TAIL             *PoolTail;
> 
>    ASSERT ((FreePoolHdr->Header.Size & (FreePoolHdr->Header.Size - 1)) == 0);
>    ASSERT (((UINTN)FreePoolHdr & (FreePoolHdr->Header.Size - 1)) == 0);
> @@ -195,7 +205,12 @@ InternalFreePoolByIndex (
>    SmmPoolType =
> UefiMemoryTypeToSmmPoolType(FreePoolHdr->Header.Type);
> 
>    PoolIndex = (UINTN) (HighBitSet32 ((UINT32)FreePoolHdr->Header.Size) -
> MIN_POOL_SHIFT);
> +  FreePoolHdr->Header.Signature = 0;
>    FreePoolHdr->Header.Available = TRUE;
> +  FreePoolHdr->Header.Type = 0;
> +  PoolTail = HEAD_TO_TAIL(&FreePoolHdr->Header);
> +  PoolTail->Signature = 0;
> +  PoolTail->Size = 0;
>    ASSERT (PoolIndex < MAX_POOL_INDEX);
>    InsertHeadList (&mSmmPoolLists[SmmPoolType][PoolIndex],
> &FreePoolHdr->Link);
>    return EFI_SUCCESS;
> @@ -223,6 +238,7 @@ SmmInternalAllocatePool (
>    )
>  {
>    POOL_HEADER           *PoolHdr;
> +  POOL_TAIL             *PoolTail;
>    FREE_POOL_HEADER      *FreePoolHdr;
>    EFI_STATUS            Status;
>    EFI_PHYSICAL_ADDRESS  Address;
> @@ -235,7 +251,10 @@ SmmInternalAllocatePool (
>      return EFI_INVALID_PARAMETER;
>    }
> 
> -  Size += sizeof (*PoolHdr);
> +  //
> +  // Adjust the size by the pool header & tail overhead
> +  //
> +  Size += POOL_OVERHEAD;
>    if (Size > MAX_POOL_SIZE) {
>      Size = EFI_SIZE_TO_PAGES (Size);
>      Status = SmmInternalAllocatePages (AllocateAnyPages, PoolType, Size,
> &Address);
> @@ -244,9 +263,13 @@ SmmInternalAllocatePool (
>      }
> 
>      PoolHdr = (POOL_HEADER*)(UINTN)Address;
> +    PoolHdr->Signature = POOL_HEAD_SIGNATURE;
>      PoolHdr->Size = EFI_PAGES_TO_SIZE (Size);
>      PoolHdr->Available = FALSE;
>      PoolHdr->Type = PoolType;
> +    PoolTail = HEAD_TO_TAIL(PoolHdr);
> +    PoolTail->Signature = POOL_TAIL_SIGNATURE;
> +    PoolTail->Size = PoolHdr->Size;
>      *Buffer = PoolHdr + 1;
>      return Status;
>    }
> @@ -317,13 +340,30 @@ SmmInternalFreePool (
>    )
>  {
>    FREE_POOL_HEADER  *FreePoolHdr;
> +  POOL_TAIL         *PoolTail;
> 
>    if (Buffer == NULL) {
>      return EFI_INVALID_PARAMETER;
>    }
> 
>    FreePoolHdr = (FREE_POOL_HEADER*)((POOL_HEADER*)Buffer - 1);
> +  ASSERT (FreePoolHdr->Header.Signature == POOL_HEAD_SIGNATURE);
>    ASSERT (!FreePoolHdr->Header.Available);
> +  PoolTail = HEAD_TO_TAIL(&FreePoolHdr->Header);
> +  ASSERT (PoolTail->Signature == POOL_TAIL_SIGNATURE);
> +  ASSERT (FreePoolHdr->Header.Size == PoolTail->Size);
> +
> +  if (FreePoolHdr->Header.Signature != POOL_HEAD_SIGNATURE) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (PoolTail->Signature != POOL_TAIL_SIGNATURE) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> +
> +  if (FreePoolHdr->Header.Size != PoolTail->Size) {
> +    return EFI_INVALID_PARAMETER;
> +  }
> 
>    if (FreePoolHdr->Header.Size > MAX_POOL_SIZE) {
>      ASSERT (((UINTN)FreePoolHdr & EFI_PAGE_MASK) == 0);
> --
> 2.7.0.windows.1



^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] MdeModulePkg PiSmmCore: Enhance SMM FreePool to catch buffer overflow
  2017-04-19  7:26 ` Yao, Jiewen
@ 2017-04-19 11:23   ` Wu, Hao A
  0 siblings, 0 replies; 3+ messages in thread
From: Wu, Hao A @ 2017-04-19 11:23 UTC (permalink / raw)
  To: Yao, Jiewen, Zeng, Star, edk2-devel@lists.01.org

Reviewed-by: Hao Wu <hao.a.wu@intel.com>


Best Regards,
Hao Wu


> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org] On Behalf Of Yao,
> Jiewen
> Sent: Wednesday, April 19, 2017 3:27 PM
> To: Zeng, Star; edk2-devel@lists.01.org
> Cc: Wu, Hao
> Subject: Re: [edk2] [PATCH] MdeModulePkg PiSmmCore: Enhance SMM
> FreePool to catch buffer overflow
> 
> Reviewed-by: jiewen.yao@intel.com
> 
> > -----Original Message-----
> > From: Zeng, Star
> > Sent: Wednesday, April 19, 2017 2:07 PM
> > To: edk2-devel@lists.01.org
> > Cc: Zeng, Star <star.zeng@intel.com>; Yao, Jiewen <jiewen.yao@intel.com>;
> Wu,
> > Hao <hao.wu@intel.com>
> > Subject: [PATCH] MdeModulePkg PiSmmCore: Enhance SMM FreePool to catch
> > buffer overflow
> >
> > This solution is equivalent to DXE core.
> >
> > AllocatePool() allocates POOL_TAIL after the buffer.
> > This POOL_TAIL is checked at FreePool().
> > If the there is buffer overflow, the issue can be caught at FreePool().
> >
> > This patch could also handle the eight-byte aligned allocation
> > requirement. The discussion related to the eight-byte aligned
> > allocation requirement is at
> > https://lists.01.org/pipermail/edk2-devel/2017-April/009995.html.
> >
> > According to the PI spec (Vol 4, Section 3.2 SmmAllocatePool()):
> > The SmmAllocatePool() function ... All allocations are eight-byte aligned.
> >
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Hao Wu <hao.wu@intel.com>
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Star Zeng <star.zeng@intel.com>
> > ---
> >  MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 22 ++++++++++++++---
> >  MdeModulePkg/Core/PiSmmCore/Pool.c      | 44
> > +++++++++++++++++++++++++++++++--
> >  2 files changed, 61 insertions(+), 5 deletions(-)
> >
> > diff --git a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
> > b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
> > index c12805a2dd96..b6f815c68da0 100644
> > --- a/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
> > +++ b/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h
> > @@ -1196,12 +1196,28 @@ extern LIST_ENTRY  mSmmMemoryMap;
> >  //
> >  #define MAX_POOL_INDEX  (MAX_POOL_SHIFT - MIN_POOL_SHIFT + 1)
> >
> > +#define POOL_HEAD_SIGNATURE   SIGNATURE_32('p','h','d','0')
> > +
> >  typedef struct {
> > -  UINTN           Size;
> > -  BOOLEAN         Available;
> > -  EFI_MEMORY_TYPE Type;
> > +  UINT32            Signature;
> > +  BOOLEAN           Available;
> > +  EFI_MEMORY_TYPE   Type;
> > +  UINTN             Size;
> >  } POOL_HEADER;
> >
> > +#define POOL_TAIL_SIGNATURE   SIGNATURE_32('p','t','a','l')
> > +
> > +typedef struct {
> > +  UINT32            Signature;
> > +  UINT32            Reserved;
> > +  UINTN             Size;
> > +} POOL_TAIL;
> > +
> > +#define POOL_OVERHEAD (sizeof(POOL_HEADER) + sizeof(POOL_TAIL))
> > +
> > +#define HEAD_TO_TAIL(a)   \
> > +  ((POOL_TAIL *) (((CHAR8 *) (a)) + (a)->Size - sizeof(POOL_TAIL)));
> > +
> >  typedef struct {
> >    POOL_HEADER  Header;
> >    LIST_ENTRY   Link;
> > diff --git a/MdeModulePkg/Core/PiSmmCore/Pool.c
> > b/MdeModulePkg/Core/PiSmmCore/Pool.c
> > index 43ce869d1e1a..ebb9f8c49ee9 100644
> > --- a/MdeModulePkg/Core/PiSmmCore/Pool.c
> > +++ b/MdeModulePkg/Core/PiSmmCore/Pool.c
> > @@ -133,6 +133,7 @@ InternalAllocPoolByIndex (
> >  {
> >    EFI_STATUS            Status;
> >    FREE_POOL_HEADER      *Hdr;
> > +  POOL_TAIL             *Tail;
> >    EFI_PHYSICAL_ADDRESS  Address;
> >    SMM_POOL_TYPE         SmmPoolType;
> >
> > @@ -154,18 +155,26 @@ InternalAllocPoolByIndex (
> >    } else {
> >      Status = InternalAllocPoolByIndex (PoolType, PoolIndex + 1, &Hdr);
> >      if (!EFI_ERROR (Status)) {
> > +      Hdr->Header.Signature = 0;
> >        Hdr->Header.Size >>= 1;
> >        Hdr->Header.Available = TRUE;
> > -      Hdr->Header.Type = PoolType;
> > +      Hdr->Header.Type = 0;
> > +      Tail = HEAD_TO_TAIL(&Hdr->Header);
> > +      Tail->Signature = 0;
> > +      Tail->Size = 0;
> >        InsertHeadList (&mSmmPoolLists[SmmPoolType][PoolIndex],
> > &Hdr->Link);
> >        Hdr = (FREE_POOL_HEADER*)((UINT8*)Hdr + Hdr->Header.Size);
> >      }
> >    }
> >
> >    if (!EFI_ERROR (Status)) {
> > +    Hdr->Header.Signature = POOL_HEAD_SIGNATURE;
> >      Hdr->Header.Size = MIN_POOL_SIZE << PoolIndex;
> >      Hdr->Header.Available = FALSE;
> >      Hdr->Header.Type = PoolType;
> > +    Tail = HEAD_TO_TAIL(&Hdr->Header);
> > +    Tail->Signature = POOL_TAIL_SIGNATURE;
> > +    Tail->Size = Hdr->Header.Size;
> >    }
> >
> >    *FreePoolHdr = Hdr;
> > @@ -187,6 +196,7 @@ InternalFreePoolByIndex (
> >  {
> >    UINTN                 PoolIndex;
> >    SMM_POOL_TYPE         SmmPoolType;
> > +  POOL_TAIL             *PoolTail;
> >
> >    ASSERT ((FreePoolHdr->Header.Size & (FreePoolHdr->Header.Size - 1)) == 0);
> >    ASSERT (((UINTN)FreePoolHdr & (FreePoolHdr->Header.Size - 1)) == 0);
> > @@ -195,7 +205,12 @@ InternalFreePoolByIndex (
> >    SmmPoolType =
> > UefiMemoryTypeToSmmPoolType(FreePoolHdr->Header.Type);
> >
> >    PoolIndex = (UINTN) (HighBitSet32 ((UINT32)FreePoolHdr->Header.Size) -
> > MIN_POOL_SHIFT);
> > +  FreePoolHdr->Header.Signature = 0;
> >    FreePoolHdr->Header.Available = TRUE;
> > +  FreePoolHdr->Header.Type = 0;
> > +  PoolTail = HEAD_TO_TAIL(&FreePoolHdr->Header);
> > +  PoolTail->Signature = 0;
> > +  PoolTail->Size = 0;
> >    ASSERT (PoolIndex < MAX_POOL_INDEX);
> >    InsertHeadList (&mSmmPoolLists[SmmPoolType][PoolIndex],
> > &FreePoolHdr->Link);
> >    return EFI_SUCCESS;
> > @@ -223,6 +238,7 @@ SmmInternalAllocatePool (
> >    )
> >  {
> >    POOL_HEADER           *PoolHdr;
> > +  POOL_TAIL             *PoolTail;
> >    FREE_POOL_HEADER      *FreePoolHdr;
> >    EFI_STATUS            Status;
> >    EFI_PHYSICAL_ADDRESS  Address;
> > @@ -235,7 +251,10 @@ SmmInternalAllocatePool (
> >      return EFI_INVALID_PARAMETER;
> >    }
> >
> > -  Size += sizeof (*PoolHdr);
> > +  //
> > +  // Adjust the size by the pool header & tail overhead
> > +  //
> > +  Size += POOL_OVERHEAD;
> >    if (Size > MAX_POOL_SIZE) {
> >      Size = EFI_SIZE_TO_PAGES (Size);
> >      Status = SmmInternalAllocatePages (AllocateAnyPages, PoolType, Size,
> > &Address);
> > @@ -244,9 +263,13 @@ SmmInternalAllocatePool (
> >      }
> >
> >      PoolHdr = (POOL_HEADER*)(UINTN)Address;
> > +    PoolHdr->Signature = POOL_HEAD_SIGNATURE;
> >      PoolHdr->Size = EFI_PAGES_TO_SIZE (Size);
> >      PoolHdr->Available = FALSE;
> >      PoolHdr->Type = PoolType;
> > +    PoolTail = HEAD_TO_TAIL(PoolHdr);
> > +    PoolTail->Signature = POOL_TAIL_SIGNATURE;
> > +    PoolTail->Size = PoolHdr->Size;
> >      *Buffer = PoolHdr + 1;
> >      return Status;
> >    }
> > @@ -317,13 +340,30 @@ SmmInternalFreePool (
> >    )
> >  {
> >    FREE_POOL_HEADER  *FreePoolHdr;
> > +  POOL_TAIL         *PoolTail;
> >
> >    if (Buffer == NULL) {
> >      return EFI_INVALID_PARAMETER;
> >    }
> >
> >    FreePoolHdr = (FREE_POOL_HEADER*)((POOL_HEADER*)Buffer - 1);
> > +  ASSERT (FreePoolHdr->Header.Signature == POOL_HEAD_SIGNATURE);
> >    ASSERT (!FreePoolHdr->Header.Available);
> > +  PoolTail = HEAD_TO_TAIL(&FreePoolHdr->Header);
> > +  ASSERT (PoolTail->Signature == POOL_TAIL_SIGNATURE);
> > +  ASSERT (FreePoolHdr->Header.Size == PoolTail->Size);
> > +
> > +  if (FreePoolHdr->Header.Signature != POOL_HEAD_SIGNATURE) {
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  if (PoolTail->Signature != POOL_TAIL_SIGNATURE) {
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> > +
> > +  if (FreePoolHdr->Header.Size != PoolTail->Size) {
> > +    return EFI_INVALID_PARAMETER;
> > +  }
> >
> >    if (FreePoolHdr->Header.Size > MAX_POOL_SIZE) {
> >      ASSERT (((UINTN)FreePoolHdr & EFI_PAGE_MASK) == 0);
> > --
> > 2.7.0.windows.1
> 
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2017-04-19 11:23 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-04-19  6:06 [PATCH] MdeModulePkg PiSmmCore: Enhance SMM FreePool to catch buffer overflow Star Zeng
2017-04-19  7:26 ` Yao, Jiewen
2017-04-19 11:23   ` Wu, Hao A

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox