diff options
author | Star Zeng <star.zeng@intel.com> | 2017-04-19 11:12:18 +0800 |
---|---|---|
committer | Guo Mang <mang.guo@intel.com> | 2017-07-12 11:24:22 +0800 |
commit | 2fce451ff7b72ed06e771d1159e9e87ec1a2e578 (patch) | |
tree | 59c0b64c2c1c03f543f8ba5fb879618df8cbe997 | |
parent | 526a4114a2ad4cea9e96d404b2124d1dfc84c5c3 (diff) | |
download | edk2-platforms-2fce451ff7b72ed06e771d1159e9e87ec1a2e578.tar.xz |
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.a.wu@intel.com>
Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Star Zeng <star.zeng@intel.com>
Reviewed-by: Jiewen Yao <jiewen.yao@intel.com>
Reviewed-by: Hao Wu <hao.a.wu@intel.com>
(cherry picked from commit 861c8dff2f506d602f8612ace12d244c29e63f31)
-rw-r--r-- | Core/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h | 22 | ||||
-rw-r--r-- | Core/MdeModulePkg/Core/PiSmmCore/Pool.c | 44 |
2 files changed, 61 insertions, 5 deletions
diff --git a/Core/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h b/Core/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h index c12805a2dd..b6f815c68d 100644 --- a/Core/MdeModulePkg/Core/PiSmmCore/PiSmmCore.h +++ b/Core/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/Core/MdeModulePkg/Core/PiSmmCore/Pool.c b/Core/MdeModulePkg/Core/PiSmmCore/Pool.c index 43ce869d1e..ebb9f8c49e 100644 --- a/Core/MdeModulePkg/Core/PiSmmCore/Pool.c +++ b/Core/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);
|