[edk2] [PATCH] Maintainers.txt: remove unexpected unicode BOM
The BOM was introduced by commit 6c05b958df532345a35b418b05effcf7fd51fc4e accidentally. Cc: Yao Jiewen Cc: Zhang, Chao B Cc: Laszlo Ersek Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- Maintainers.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Maintainers.txt b/Maintainers.txt index 08a676b236..61c57587a6 100644 --- a/Maintainers.txt +++ b/Maintainers.txt @@ -1,4 +1,4 @@ -???EDK II Maintainers +EDK II Maintainers == This file provides information about the primary maintainers for -- 2.17.1.windows.2 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 2] UefiCpuPkg: restore strict page attributes via #DB in nonstop mode only
> v2: Per Laszlo's comments, repack origianl two patches into one with > title changed and relevant commits added REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1576 The root cause of this issue is that non-stop mode of Heap Guard and NULL Detection set TF bit (single-step) in EFLAG unconditionally in the common handler in CpuExceptionLib. If PcdCpuSmmStaticPageTable is FALSE, the SMM will only create page table for memory below 4G. If SMM tries to access memory beyond 4G, a page fault exception will be triggered and the memory to access will be added to page table so that SMM code can continue the access. Because of above issue, the TF bit is set after the page fault is handled and then fall into another DEBUG exception. Since non-stop mode of Heap Guard and NULL Detection are not enabled, no special DEBUG exception handler is registered. The default handler just prints exception context and go into dead loop. Actually EFLAGS can be changed in any standard exception handler. There's no need to do single-step setup in assembly code. So the fix is to move the logic to C code part of page fault exception handler so that we can fully validate the configuration and prevent TF bit from being set unexpectedly. Fixes: dcc026217fdc363f55c217039fc43d344f69fed6 16b918bbaf51211a32ae04d9d8a5ba6ccca25a6a Test: - Pass special test of accessing memory beyond 4G in SMM mode - Boot to OS with Qemu emulator platform (Fedora27, Ubuntu18.04, Windows7, Windows10) Cc: Eric Dong Cc: Laszlo Ersek Cc: Ruiyu Ni Cc: Star Zeng Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang Acked-by: Laszlo Ersek --- UefiCpuPkg/CpuDxe/CpuPageTable.c | 11 ++- .../Ia32/ExceptionHandlerAsm.nasm | 7 --- .../X64/ExceptionHandlerAsm.nasm | 4 3 files changed, 10 insertions(+), 12 deletions(-) diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c index 4bee8c7772..812537417d 100644 --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c @@ -1300,7 +1300,16 @@ PageFaultExceptionHandler ( // Display ExceptionType, CPU information and Image information // DumpCpuContext (ExceptionType, SystemContext); - if (!NonStopMode) { + if (NonStopMode) { +// +// Set TF in EFLAGS +// +if (mPagingContext.MachineType == IMAGE_FILE_MACHINE_I386) { + SystemContext.SystemContextIa32->Eflags |= (UINT32)BIT8; +} else { + SystemContext.SystemContextX64->Rflags |= (UINT64)BIT8; +} + } else { CpuDeadLoop (); } } diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm index 6fcf5fb23f..45d6474091 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm @@ -383,13 +383,6 @@ ErrorCodeAndVectorOnStack: pop dword [ebp - 4] mov esp, ebp pop ebp - -; Enable TF bit after page fault handler runs -cmp dword [esp], 14 ; #PF? -jne .5 -bts dword [esp + 16], 8 ; EFLAGS - -.5: add esp, 8 cmp dword [esp - 16], 0 ; check EXCEPTION_HANDLER_CONTEXT.OldIdtHandler jz DoReturn diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm index f842af2336..7b97810d10 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm @@ -336,10 +336,6 @@ HasErrorCode: pop r15 mov rsp, rbp -cmp qword [rbp + 8], 14 ; #PF? -jne .1 -bts qword [rsp + 40], 8 ; RFLAGS.TF -.1: pop rbp add rsp, 16 cmp qword [rsp - 32], 0 ; check EXCEPTION_HANDLER_CONTEXT.OldIdtHandler -- 2.17.1.windows.2 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 2/2] UefiCpuPkg/CpuDxe: remove code to set TF bit in EFLAGS
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1576 The root cause of this issue is that non-stop mode of Heap Guard and NULL Detection set TF bit (single-step) in EFLAG unconditionally in the common handler in CpuExceptionLib. If PcdCpuSmmStaticPageTable is FALSE, the SMM will only create page table for memory below 4G. If SMM tries to access memory beyond 4G, a page fault exception will be triggered and the memory to access will be added to page table so that SMM code can continue the access. Because of above issue, the TF bit is set after the page fault is handled and then fall into another DEBUG exception. Since non-stop mode of Heap Guard and NULL Detection are not enabled, no special DEBUG exception handler is registered. The default handler just prints exception context and go into dead loop. Actually EFLAGS can be changed in any standard exception handler. There's no need to do single-step setup in assembly code. So the fix is to move the logic to C code part of page fault exception handler so that we can fully validate the configuration and prevent TF bit from being set unexpectedly. Cc: Eric Dong Cc: Laszlo Ersek Cc: Ruiyu Ni Cc: Star Zeng Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- .../CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm | 7 --- .../CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm| 4 2 files changed, 11 deletions(-) diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm index 6fcf5fb23f..45d6474091 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm @@ -383,13 +383,6 @@ ErrorCodeAndVectorOnStack: pop dword [ebp - 4] mov esp, ebp pop ebp - -; Enable TF bit after page fault handler runs -cmp dword [esp], 14 ; #PF? -jne .5 -bts dword [esp + 16], 8 ; EFLAGS - -.5: add esp, 8 cmp dword [esp - 16], 0 ; check EXCEPTION_HANDLER_CONTEXT.OldIdtHandler jz DoReturn diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm index f842af2336..7b97810d10 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm @@ -336,10 +336,6 @@ HasErrorCode: pop r15 mov rsp, rbp -cmp qword [rbp + 8], 14 ; #PF? -jne .1 -bts qword [rsp + 40], 8 ; RFLAGS.TF -.1: pop rbp add rsp, 16 cmp qword [rsp - 32], 0 ; check EXCEPTION_HANDLER_CONTEXT.OldIdtHandler -- 2.17.1.windows.2 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 0/2] Fix SMM driver hang at accessing memory beyond 4G
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1576 Test: - Pass special test of accessing memory beyond 4G - Boot to OS with Qemu emulator platform (Fedora27, Ubuntu18.04, Windows7, Windows10) Jian J Wang (2): UefiCpuPkg/CpuDxe: set TF bit in EFLAGS in C code UefiCpuPkg/CpuDxe: remove code to set TF bit in EFLAGS UefiCpuPkg/CpuDxe/CpuPageTable.c | 11 ++- .../Ia32/ExceptionHandlerAsm.nasm | 7 --- .../X64/ExceptionHandlerAsm.nasm | 4 3 files changed, 10 insertions(+), 12 deletions(-) -- 2.17.1.windows.2 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 1/2] UefiCpuPkg/CpuDxe: set TF bit in EFLAGS in C code
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1576 The root cause of this issue is that non-stop mode of Heap Guard and NULL Detection set TF bit (single-step) in EFLAG unconditionally in the common handler in CpuExceptionLib. If PcdCpuSmmStaticPageTable is FALSE, the SMM will only create page table for memory below 4G. If SMM tries to access memory beyond 4G, a page fault exception will be triggered and the memory to access will be added to page table so that SMM code can continue the access. Because of above issue, the TF bit is set after the page fault is handled and then fall into another DEBUG exception. Since non-stop mode of Heap Guard and NULL Detection are not enabled, no special DEBUG exception handler is registered. The default handler just prints exception context and go into dead loop. Actually EFLAGS can be changed in any standard exception handler. There's no need to do single-step setup in assembly code. So the fix is to move the logic to C code part of page fault exception handler so that we can fully validate the configuration and prevent TF bit from being set unexpectedly. Cc: Eric Dong Cc: Laszlo Ersek Cc: Ruiyu Ni Cc: Star Zeng Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/CpuDxe/CpuPageTable.c | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c index 4bee8c7772..812537417d 100644 --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c @@ -1300,7 +1300,16 @@ PageFaultExceptionHandler ( // Display ExceptionType, CPU information and Image information // DumpCpuContext (ExceptionType, SystemContext); - if (!NonStopMode) { + if (NonStopMode) { +// +// Set TF in EFLAGS +// +if (mPagingContext.MachineType == IMAGE_FILE_MACHINE_I386) { + SystemContext.SystemContextIa32->Eflags |= (UINT32)BIT8; +} else { + SystemContext.SystemContextX64->Rflags |= (UINT64)BIT8; +} + } else { CpuDeadLoop (); } } -- 2.17.1.windows.2 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 2/3] MdeModulePkg/DxeCore: Ensure FfsFileHeader 8 bytes aligned [CVE-2018-3630]
From: Star Zeng REF: https://bugzilla.tianocore.org/show_bug.cgi?id=864 To follow PI spec, ensure FfsFileHeader 8 bytes aligned. For the integrity of FV(especially non-MemoryMapped FV) layout, let CachedFv point to FV beginning, but not (FV + FV header). And current code only handles (FwVolHeader->ExtHeaderOffset != 0) path, update code to also handle (FwVolHeader->ExtHeaderOffset == 0) path. Cc: Jiewen Yao Cc: Liming Gao Cc: Jian J Wang Cc: Hao Wu Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng --- MdeModulePkg/Core/Dxe/FwVol/FwVol.c | 65 +++-- 1 file changed, 14 insertions(+), 51 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/FwVol/FwVol.c b/MdeModulePkg/Core/Dxe/FwVol/FwVol.c index 93ddcc3591..28fce46a95 100644 --- a/MdeModulePkg/Core/Dxe/FwVol/FwVol.c +++ b/MdeModulePkg/Core/Dxe/FwVol/FwVol.c @@ -3,7 +3,7 @@ Layers on top of Firmware Block protocol to produce a file abstraction of FV based files. -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved. This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License which accompanies this distribution. The full text of the license may be found at @@ -329,8 +329,6 @@ FvCheck ( FFS_FILE_LIST_ENTRY *FfsFileEntry; EFI_FFS_FILE_HEADER *FfsHeader; UINT8 *CacheLocation; - UINTN LbaOffset; - UINTN HeaderSize; UINTN Index; EFI_LBA LbaIndex; UINTN Size; @@ -353,11 +351,7 @@ FvCheck ( return Status; } - // - // Size is the size of the FV minus the head. We have already allocated - // the header to check to make sure the volume is valid - // - Size = (UINTN)(FwVolHeader->FvLength - FwVolHeader->HeaderLength); + Size = (UINTN) FwVolHeader->FvLength; if ((FvbAttributes & EFI_FVB2_MEMORY_MAPPED) != 0) { FvDevice->IsMemoryMapped = TRUE; @@ -369,7 +363,7 @@ FvCheck ( // // Don't cache memory mapped FV really. // -FvDevice->CachedFv = (UINT8 *) (UINTN) (PhysicalAddress + FwVolHeader->HeaderLength); +FvDevice->CachedFv = (UINT8 *) (UINTN) PhysicalAddress; } else { FvDevice->IsMemoryMapped = FALSE; FvDevice->CachedFv = AllocatePool (Size); @@ -380,52 +374,27 @@ FvCheck ( } // - // Remember a pointer to the end fo the CachedFv + // Remember a pointer to the end of the CachedFv // FvDevice->EndOfCachedFv = FvDevice->CachedFv + Size; if (!FvDevice->IsMemoryMapped) { // -// Copy FV minus header into memory using the block map we have all ready -// read into memory. +// Copy FV into memory using the block map. // BlockMap = FwVolHeader->BlockMap; CacheLocation = FvDevice->CachedFv; LbaIndex = 0; -LbaOffset = 0; -HeaderSize = FwVolHeader->HeaderLength; while ((BlockMap->NumBlocks != 0) || (BlockMap->Length != 0)) { - Index = 0; - Size = BlockMap->Length; - if (HeaderSize > 0) { -// -// Skip header size -// -for (; Index < BlockMap->NumBlocks && HeaderSize >= BlockMap->Length; Index ++) { - HeaderSize -= BlockMap->Length; - LbaIndex ++; -} - -// -// Check whether FvHeader is crossing the multi block range. -// -if (Index >= BlockMap->NumBlocks) { - BlockMap++; - continue; -} else if (HeaderSize > 0) { - LbaOffset = HeaderSize; - Size = BlockMap->Length - HeaderSize; - HeaderSize = 0; -} - } - // // read the FV data // - for (; Index < BlockMap->NumBlocks; Index ++) { -Status = Fvb->Read (Fvb, + Size = BlockMap->Length; + for (Index = 0; Index < BlockMap->NumBlocks; Index++) { +Status = Fvb->Read ( +Fvb, LbaIndex, -LbaOffset, +0, , CacheLocation ); @@ -438,13 +407,7 @@ FvCheck ( } LbaIndex++; -CacheLocation += Size; - -// -// After we skip Fv Header always read from start of block -// -LbaOffset = 0; -Size = BlockMap->Length; +CacheLocation += BlockMap->Length; } BlockMap++; @@ -475,12 +438,12 @@ FvCheck ( // // Searching for files starts on an 8 byte aligned boundary after the end of the Extended Header if it exists. // -FwVolExtHeader = (EFI_FIRM
[edk2] [PATCH 3/3] IntelFrameworkModulePkg/FwVolDxe: Ensure FfsFileHeader 8 bytes aligned [CVE-2018-3630]
From: Star Zeng REF: https://bugzilla.tianocore.org/show_bug.cgi?id=864 To follow PI spec, ensure FfsFileHeader 8 bytes aligned. Current code only handles (FwVolHeader->ExtHeaderOffset != 0) path, update code to also handle (FwVolHeader->ExtHeaderOffset == 0) path. Cc: Jiewen Yao Cc: Liming Gao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng --- .../Universal/FirmwareVolume/FwVolDxe/FwVol.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/IntelFrameworkModulePkg/Universal/FirmwareVolume/FwVolDxe/FwVol.c b/IntelFrameworkModulePkg/Universal/FirmwareVolume/FwVolDxe/FwVol.c index 9a031bab14..9a892240b4 100644 --- a/IntelFrameworkModulePkg/Universal/FirmwareVolume/FwVolDxe/FwVol.c +++ b/IntelFrameworkModulePkg/Universal/FirmwareVolume/FwVolDxe/FwVol.c @@ -4,7 +4,7 @@ Layers on top of Firmware Block protocol to produce a file abstraction of FV based files. - Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. + Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved. This program and the accompanying materials are licensed and made available under the terms and conditions @@ -510,10 +510,10 @@ FvCheck ( // FwVolExtHeader = (EFI_FIRMWARE_VOLUME_EXT_HEADER *) (UINTN) (FvDevice->CachedFv + FvDevice->FwVolHeader->ExtHeaderOffset); Ptr = (UINT8 *) FwVolExtHeader + FwVolExtHeader->ExtHeaderSize; -Ptr = (UINT8 *) ALIGN_POINTER (Ptr, 8); } else { Ptr = (UINT8 *) (UINTN) (FvDevice->CachedFv + FvDevice->FwVolHeader->HeaderLength); } + Ptr = (UINT8 *) ALIGN_POINTER (Ptr, 8); TopFvAddress = (UINT8 *) (UINTN) (FvDevice->CachedFv + FvDevice->FwVolHeader->FvLength); // -- 2.17.1.windows.2 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 1/3] MdeModulePkg/PeiCore: Ensure FfsFileHeader 8 bytes aligned [CVE-2018-3630]
From: Star Zeng REF: https://bugzilla.tianocore.org/show_bug.cgi?id=864 To follow PI spec, ensure FfsFileHeader 8 bytes aligned. Current code only handles (FwVolHeader->ExtHeaderOffset != 0) path, update code to also handle (FwVolHeader->ExtHeaderOffset == 0) path. Cc: Jiewen Yao Cc: Liming Gao Cc: Jian J Wang Cc: Hao Wu Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Star Zeng --- MdeModulePkg/Core/Pei/FwVol/FwVol.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/MdeModulePkg/Core/Pei/FwVol/FwVol.c b/MdeModulePkg/Core/Pei/FwVol/FwVol.c index 0a67b96bf1..56440eacf0 100644 --- a/MdeModulePkg/Core/Pei/FwVol/FwVol.c +++ b/MdeModulePkg/Core/Pei/FwVol/FwVol.c @@ -2,7 +2,7 @@ Pei Core Firmware File System service routines. Copyright (c) 2015 HP Development Company, L.P. -Copyright (c) 2006 - 2018, Intel Corporation. All rights reserved. +Copyright (c) 2006 - 2019, Intel Corporation. All rights reserved. This program and the accompanying materials are licensed and made available under the terms and conditions of the BSD License which accompanies this distribution. The full text of the license may be found at @@ -316,10 +316,10 @@ FindFileEx ( // FwVolExtHeader = (EFI_FIRMWARE_VOLUME_EXT_HEADER *) ((UINT8 *) FwVolHeader + FwVolHeader->ExtHeaderOffset); FfsFileHeader = (EFI_FFS_FILE_HEADER *) ((UINT8 *) FwVolExtHeader + FwVolExtHeader->ExtHeaderSize); - FfsFileHeader = (EFI_FFS_FILE_HEADER *) ALIGN_POINTER (FfsFileHeader, 8); } else { FfsFileHeader = (EFI_FFS_FILE_HEADER *)((UINT8 *) FwVolHeader + FwVolHeader->HeaderLength); } +FfsFileHeader = (EFI_FFS_FILE_HEADER *) ALIGN_POINTER (FfsFileHeader, 8); } else { if (IS_FFS_FILE2 (*FileHeader)) { if (!IsFfs3Fv) { -- 2.17.1.windows.2 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 0/3] Ensure FfsFileHeader 8 bytes aligned [CVE-2018-3630]
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=864 To follow PI spec, ensure FfsFileHeader 8 bytes aligned. Current code only handles (FwVolHeader->ExtHeaderOffset != 0) path, update code to also handle (FwVolHeader->ExtHeaderOffset == 0) path. Star Zeng (3): MdeModulePkg/PeiCore: Ensure FfsFileHeader 8 bytes aligned MdeModulePkg/DxeCore: Ensure FfsFileHeader 8 bytes aligned IntelFrameworkModulePkg/FwVolDxe: Ensure FfsFileHeader 8 bytes aligned .../Universal/FirmwareVolume/FwVolDxe/FwVol.c | 4 +- MdeModulePkg/Core/Dxe/FwVol/FwVol.c | 65 --- MdeModulePkg/Core/Pei/FwVol/FwVol.c | 4 +- 3 files changed, 18 insertions(+), 55 deletions(-) -- 2.17.1.windows.2 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 1/2] MdePkg/UefiDevicePathLib: Add sanity check for FilePath device path
> v2: fix wrong detection of FilePath device path REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1497 Current implementation of IsDevicePathValid() is not enough for type of MEDIA_FILEPATH_DP, which has NULL-terminated string in the device path. This patch add a simple NULL character check at Length position. Cc: Liming Gao Cc: Ray Ni Cc: Michael D Kinney Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c | 9 + 1 file changed, 9 insertions(+) diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c index 5d7635fe3e..dd1bddc1c2 100644 --- a/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c +++ b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c @@ -95,6 +95,15 @@ IsDevicePathValid ( return FALSE; } } + +// +// FilePath must be a NULL-terminated string. +// +if (DevicePathType (DevicePath) == MEDIA_DEVICE_PATH && +DevicePathSubType (DevicePath) == MEDIA_FILEPATH_DP && +*(CHAR16 *)((UINT8 *)DevicePath + NodeLength - 2) != 0) { + return FALSE; +} } // -- 2.17.1.windows.2 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 0/2] Add sanity check for FilePath device path
> v2: fix wrong detection of FilePath device path REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1497 Tests - Pass specific DevicePathLib test cases - Boot shell - Boot to Fedora 26 (Qemu/x64) - Boot to Ubuntu 18.04 (Qemu/x64) - Boot to Windows 10 (Qemu/x64) - Boot to Windows 7 (Qemu/x64) Jian J Wang (2): MdePkg/UefiDevicePathLib: Add sanity check for FilePath device path MdePkg/UefiDevicePathLibDevicePathProtocol: Add sanity check for FilePath device path MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c | 9 + .../UefiDevicePathLib.c | 9 + 2 files changed, 18 insertions(+) -- 2.17.1.windows.2 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 2/2] MdePkg/UefiDevicePathLibDevicePathProtocol: Add sanity check for FilePath device path
> v2: fix wrong detection of FilePath device path REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1497 Current implementation of IsDevicePathValid() is not enough for type of MEDIA_FILEPATH_DP, which has NULL-terminated string in the device path. This patch add a simple NULL character check at Length position. Cc: Liming Gao Cc: Ray Ni Cc: Michael D Kinney Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- .../UefiDevicePathLib.c | 9 + 1 file changed, 9 insertions(+) diff --git a/MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLib.c b/MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLib.c index 9a0ee42fd1..c8e78d2373 100644 --- a/MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLib.c +++ b/MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLib.c @@ -138,6 +138,15 @@ IsDevicePathValid ( return FALSE; } } + +// +// FilePath must be a NULL-terminated string. +// +if (DevicePathType (DevicePath) == MEDIA_DEVICE_PATH && +DevicePathSubType (DevicePath) == MEDIA_FILEPATH_DP && +*(CHAR16 *)((UINT8 *)DevicePath + NodeLength - 2) != 0) { + return FALSE; +} } // -- 2.17.1.windows.2 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 1/2] MdePkg/UefiDevicePathLib: Add sanity check for FilePath device path
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1497 Current implementation of IsDevicePathValid() is not enough for type of MEDIA_FILEPATH_DP, which has NULL-terminated string in the device path. This patch add a simple NULL character check at Length position. Cc: Liming Gao Cc: Ray Ni Cc: Michael D Kinney Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c | 8 1 file changed, 8 insertions(+) diff --git a/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c index 665e5a4adc..6d87744510 100644 --- a/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c +++ b/MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c @@ -96,6 +96,14 @@ IsDevicePathValid ( return FALSE; } } + +// +// FilePath must be a NULL-terminated string. +// +if (DevicePathType (DevicePath) == MEDIA_FILEPATH_DP && +*(CHAR16 *)((UINT8 *)DevicePath + NodeLength - 2) != 0) { + return FALSE; +} } // -- 2.19.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 0/2] Add sanity check for FilePath device path
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1497 Jian J Wang (2): MdePkg/UefiDevicePathLib: Add sanity check for FilePath device path MdePkg/UefiDevicePathLibDevicePathProtocol: Add sanity check for FilePath device path MdePkg/Library/UefiDevicePathLib/DevicePathUtilities.c| 8 .../UefiDevicePathLib.c | 8 2 files changed, 16 insertions(+) -- 2.19.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 2/2] MdePkg/UefiDevicePathLibDevicePathProtocol: Add sanity check for FilePath device path
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1497 Current implementation of IsDevicePathValid() is not enough for type of MEDIA_FILEPATH_DP, which has NULL-terminated string in the device path. This patch add a simple NULL character check at Length position. Cc: Liming Gao Cc: Ray Ni Cc: Michael D Kinney Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- .../UefiDevicePathLib.c | 8 1 file changed, 8 insertions(+) diff --git a/MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLib.c b/MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLib.c index 9a0ee42fd1..5dc413de44 100644 --- a/MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLib.c +++ b/MdePkg/Library/UefiDevicePathLibDevicePathProtocol/UefiDevicePathLib.c @@ -138,6 +138,14 @@ IsDevicePathValid ( return FALSE; } } + +// +// FilePath must be a NULL-terminated string. +// +if (DevicePathType (DevicePath) == MEDIA_FILEPATH_DP && +*(CHAR16 *)((UINT8 *)DevicePath + NodeLength - 2) != 0) { + return FALSE; +} } // -- 2.19.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] CryptoPkg/BaseCryptLib: split CryptPkcs7Verify.c on behalf of runtime
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1493 Pkcs7GetAttachedContent() implementation in current CryptPkcs7Verify.c is actually shared by RuntimeCryptLib.inf, SmmCryptLib.inf and BaseCryptLib.inf, which are not correct since there's no use scenario for runtime and AllocatePool() used in this method can only be called in boot time. This patch fix this issue by splitting file CryptPkcs7Verify.c into 3 parts. CryptPkcs7VerifyCommon.c (shared among Base, SMM, Runtime) CryptPkcs7VerifyBase.c(shared between Base, SMM) CryptPkcs7VerifyRuntime.c (for Runtime only) CryptPkcs7VerifyBase.c will have original implementation of Pkcs7GetAttachedContent() as CryptPkcs7Verify.c. CryptPkcs7VerifyRuntime.c provide a NULL version of Pkcs7GetAttachedContent(). No functionality and interface change is involved in this patch. Cc: Ting Ye Cc: Qin Long Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- .../Library/BaseCryptLib/BaseCryptLib.inf | 5 +- .../Library/BaseCryptLib/InternalCryptLib.h | 32 + .../Library/BaseCryptLib/PeiCryptLib.inf | 3 +- .../BaseCryptLib/Pk/CryptPkcs7VerifyBase.c| 131 ++ ...Pkcs7Verify.c => CryptPkcs7VerifyCommon.c} | 112 +-- .../BaseCryptLib/Pk/CryptPkcs7VerifyRuntime.c | 45 ++ .../Library/BaseCryptLib/RuntimeCryptLib.inf | 6 +- .../Library/BaseCryptLib/SmmCryptLib.inf | 5 +- 8 files changed, 220 insertions(+), 119 deletions(-) create mode 100644 CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyBase.c rename CryptoPkg/Library/BaseCryptLib/Pk/{CryptPkcs7Verify.c => CryptPkcs7VerifyCommon.c} (85%) create mode 100644 CryptoPkg/Library/BaseCryptLib/Pk/CryptPkcs7VerifyRuntime.c diff --git a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf index f29445ce34..c5558eedb7 100644 --- a/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf +++ b/CryptoPkg/Library/BaseCryptLib/BaseCryptLib.inf @@ -6,7 +6,7 @@ # This external input must be validated carefully to avoid security issues such as # buffer overflow or integer overflow. # -# Copyright (c) 2009 - 2018, Intel Corporation. All rights reserved. +# Copyright (c) 2009 - 2019, Intel Corporation. All rights reserved. # This program and the accompanying materials # are licensed and made available under the terms and conditions of the BSD License # which accompanies this distribution. The full text of the license may be found at @@ -49,7 +49,8 @@ Pk/CryptRsaExt.c Pk/CryptPkcs5Pbkdf2.c Pk/CryptPkcs7Sign.c - Pk/CryptPkcs7Verify.c + Pk/CryptPkcs7VerifyCommon.c + Pk/CryptPkcs7VerifyBase.c Pk/CryptDh.c Pk/CryptX509.c Pk/CryptAuthenticode.c diff --git a/CryptoPkg/Library/BaseCryptLib/InternalCryptLib.h b/CryptoPkg/Library/BaseCryptLib/InternalCryptLib.h index 8cccf72567..026793f664 100644 --- a/CryptoPkg/Library/BaseCryptLib/InternalCryptLib.h +++ b/CryptoPkg/Library/BaseCryptLib/InternalCryptLib.h @@ -33,4 +33,36 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #define OBJ_length(o) ((o)->length) #endif +/** + Check input P7Data is a wrapped ContentInfo structure or not. If not construct + a new structure to wrap P7Data. + + Caution: This function may receive untrusted input. + UEFI Authenticated Variable is external input, so this function will do basic + check for PKCS#7 data structure. + + @param[in] P7Data Pointer to the PKCS#7 message to verify. + @param[in] P7Length Length of the PKCS#7 message in bytes. + @param[out] WrapFlag If TRUE P7Data is a ContentInfo structure, otherwise + return FALSE. + @param[out] WrapData If return status of this function is TRUE: + 1) when WrapFlag is TRUE, pointer to P7Data. + 2) when WrapFlag is FALSE, pointer to a new ContentInfo + structure. It's caller's responsibility to free this + buffer. + @param[out] WrapDataSize Length of ContentInfo structure in bytes. + + @retval TRUE The operation is finished successfully. + @retval FALSEThe operation is failed due to lack of resources. + +**/ +BOOLEAN +WrapPkcs7Data ( + IN CONST UINT8 *P7Data, + IN UINTNP7Length, + OUT BOOLEAN *WrapFlag, + OUT UINT8**WrapData, + OUT UINTN*WrapDataSize + ); + #endif diff --git a/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf b/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf index e7b4b2f618..386810e442 100644 --- a/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf +++ b/CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf @@ -56,7 +56,8 @@ Pk/CryptRsaExtNull.c Pk/CryptPkcs5Pbkdf2Null.c Pk/CryptPkcs7SignNull.c - Pk/CryptPkcs7Verify.c + Pk/CryptPkcs7VerifyCommon.c + Pk/CryptPkcs7VerifyBase.c Pk/CryptDhNull.c Pk/CryptX509Null.c diff --git a/CryptoPk
[edk2] [PATCH] Upgrade OpenSSL to 1.1.0j
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1393 BZ#1089 (https://bugzilla.tianocore.org/show_bug.cgi?id=1089) requests to upgrade the OpenSSL to the latest 1.1.1 release. Since OpenSSL-1.1.1 has many changes, more porting efforts and feature evaluation are needed. This might lead to a situation that it cannot catch the Q1'19 stable tag. One of the solution is upgrade current version (1.1.0h) to 1.1.0j. According to following web page in openssl.org, all security issues solved in 1.1.1 have been also back-ported to 1.1.0.j. This can make sure that no security vulnerabilities left in edk2 master before 1.1.1. https://www.openssl.org/news/vulnerabilities-1.1.1.html Cc: Ting Ye Cc: Gang Wei Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- CryptoPkg/CryptoPkg.dsc | 1 + .../Library/Include/openssl/opensslconf.h | 20 --- CryptoPkg/Library/OpensslLib/OpensslLib.inf | 3 +++ .../Library/OpensslLib/OpensslLibCrypto.inf | 3 +++ CryptoPkg/Library/OpensslLib/openssl | 2 +- CryptoPkg/Library/OpensslLib/process_files.pl | 0 6 files changed, 21 insertions(+), 8 deletions(-) mode change 100644 => 100755 CryptoPkg/Library/OpensslLib/process_files.pl diff --git a/CryptoPkg/CryptoPkg.dsc b/CryptoPkg/CryptoPkg.dsc index a0334d628b..321abe4d4c 100644 --- a/CryptoPkg/CryptoPkg.dsc +++ b/CryptoPkg/CryptoPkg.dsc @@ -121,6 +121,7 @@ CryptoPkg/Library/BaseCryptLib/PeiCryptLib.inf CryptoPkg/Library/BaseCryptLib/RuntimeCryptLib.inf CryptoPkg/Library/TlsLib/TlsLib.inf + CryptoPkg/Library/OpensslLib/OpensslLib.inf [Components.IA32, Components.X64] CryptoPkg/Library/BaseCryptLib/SmmCryptLib.inf diff --git a/CryptoPkg/Library/Include/openssl/opensslconf.h b/CryptoPkg/Library/Include/openssl/opensslconf.h index 1917d7ab24..28dd9ab93c 100644 --- a/CryptoPkg/Library/Include/openssl/opensslconf.h +++ b/CryptoPkg/Library/Include/openssl/opensslconf.h @@ -2,7 +2,7 @@ * WARNING: do not edit! * Generated from include/openssl/opensslconf.h.in * - * Copyright 2016 The OpenSSL Project Authors. All Rights Reserved. + * Copyright 2016-2018 The OpenSSL Project Authors. All Rights Reserved. * * Licensed under the OpenSSL license (the "License"). You may not use * this file except in compliance with the License. You can obtain a copy @@ -235,12 +235,18 @@ extern "C" { * still won't see them if the library has been built to disable deprecated * functions. */ -#if defined(OPENSSL_NO_DEPRECATED) -# define DECLARE_DEPRECATED(f) -#elif __GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ > 0) -# define DECLARE_DEPRECATED(f)f __attribute__ ((deprecated)); -#else -# define DECLARE_DEPRECATED(f) f; +#ifndef DECLARE_DEPRECATED +# if defined(OPENSSL_NO_DEPRECATED) +# define DECLARE_DEPRECATED(f) +# else +# define DECLARE_DEPRECATED(f) f; +# ifdef __GNUC__ +# if __GNUC__ > 3 || (__GNUC__ == 3 && __GNUC_MINOR__ > 0) +#undef DECLARE_DEPRECATED +#define DECLARE_DEPRECATED(f)f __attribute__ ((deprecated)); +# endif +# endif +# endif #endif #ifndef OPENSSL_FILE diff --git a/CryptoPkg/Library/OpensslLib/OpensslLib.inf b/CryptoPkg/Library/OpensslLib/OpensslLib.inf index 0300856cf2..6162d29143 100644 --- a/CryptoPkg/Library/OpensslLib/OpensslLib.inf +++ b/CryptoPkg/Library/OpensslLib/OpensslLib.inf @@ -175,6 +175,7 @@ $(OPENSSL_PATH)/crypto/conf/conf_mall.c $(OPENSSL_PATH)/crypto/conf/conf_mod.c $(OPENSSL_PATH)/crypto/conf/conf_sap.c + $(OPENSSL_PATH)/crypto/conf/conf_ssl.c $(OPENSSL_PATH)/crypto/cpt_err.c $(OPENSSL_PATH)/crypto/cryptlib.c $(OPENSSL_PATH)/crypto/cversion.c @@ -281,6 +282,7 @@ $(OPENSSL_PATH)/crypto/evp/pmeth_lib.c $(OPENSSL_PATH)/crypto/evp/scrypt.c $(OPENSSL_PATH)/crypto/ex_data.c + $(OPENSSL_PATH)/crypto/getenv.c $(OPENSSL_PATH)/crypto/hmac/hm_ameth.c $(OPENSSL_PATH)/crypto/hmac/hm_pmeth.c $(OPENSSL_PATH)/crypto/hmac/hmac.c @@ -418,6 +420,7 @@ $(OPENSSL_PATH)/crypto/x509/x509_err.c $(OPENSSL_PATH)/crypto/x509/x509_ext.c $(OPENSSL_PATH)/crypto/x509/x509_lu.c + $(OPENSSL_PATH)/crypto/x509/x509_meth.c $(OPENSSL_PATH)/crypto/x509/x509_obj.c $(OPENSSL_PATH)/crypto/x509/x509_r2x.c $(OPENSSL_PATH)/crypto/x509/x509_req.c diff --git a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf index 23be4e1e14..b04bf62b4e 100644 --- a/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf +++ b/CryptoPkg/Library/OpensslLib/OpensslLibCrypto.inf @@ -175,6 +175,7 @@ $(OPENSSL_PATH)/crypto/conf/conf_mall.c $(OPENSSL_PATH)/crypto/conf/conf_mod.c $(OPENSSL_PATH)/crypto/conf/conf_sap.c + $(OPENSSL_PATH)/crypto/conf/conf_ssl.c $(OPENSSL_PATH)/crypto/cpt_err.c $(OPENSSL_PATH)/crypto/cryptlib.c $(OPENSSL_PATH)/crypto/cversion.c @@ -281,6 +282,7 @@ $(OPENSSL_PATH)/crypto/evp/pmeth_lib.c $(OPENSSL_PATH)/cryp
[edk2] [PATCH] CryptoPkg/IntrinsicLib: add missing BaseLib declaration
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=596 BaseLib interfaces are used in this library but not declared in module's inf file. This patch fix this situation to keep inf and its code in consistency. No functionality or interface change are involved. Cc: Qin Long Cc: Ting Ye Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf | 1 + 1 file changed, 1 insertion(+) diff --git a/CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf b/CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf index 579da34aff..a91c850013 100644 --- a/CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf +++ b/CryptoPkg/Library/IntrinsicLib/IntrinsicLib.inf @@ -51,6 +51,7 @@ MdePkg/MdePkg.dec [LibraryClasses] + BaseLib BaseMemoryLib [BuildOptions] -- 2.19.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 1/2] MdeModulePkg/Core: fill logic hole in MemoryProtectionCpuArchProtocolNotify
> v3: fixed one more memory leak in the same function and updated > commit message accordingly. At the end of of MemoryProtectionCpuArchProtocolNotify there's cleanup code to free resource. But at line 978, 994, 1005 the function returns directly. This patch use "goto" to replace "return" to make sure the resource is freed before exit. 1029: CoreCloseEvent (Event); 1030: return; There's another memory leak after calling gBS->LocateHandleBuffer() in the same function: Status = gBS->LocateHandleBuffer ( ByProtocol, , NULL, , ); HandleBuffer is allocated in above call but never freed. This patch will also add code to free it. Cc: Star Zeng Cc: Jiewen Yao Cc: Ruiyu Ni Cc: Leif Lindholm Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 9 + 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c index 6298b67db1..8a93c5362a 100644 --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c @@ -975,7 +975,7 @@ MemoryProtectionCpuArchProtocolNotify ( DEBUG ((DEBUG_INFO, "MemoryProtectionCpuArchProtocolNotify:\n")); Status = CoreLocateProtocol (, NULL, (VOID **)); if (EFI_ERROR (Status)) { -return; +goto Done; } // @@ -991,7 +991,7 @@ MemoryProtectionCpuArchProtocolNotify ( HeapGuardCpuArchProtocolNotify (); if (mImageProtectionPolicy == 0) { -return; +goto Done; } Status = gBS->LocateHandleBuffer ( @@ -1002,7 +1002,7 @@ MemoryProtectionCpuArchProtocolNotify ( ); if (EFI_ERROR (Status) && (NoHandles == 0)) { -return ; +goto Done; } for (Index = 0; Index < NoHandles; Index++) { @@ -1025,9 +1025,10 @@ MemoryProtectionCpuArchProtocolNotify ( ProtectUefiImage (LoadedImage, LoadedImageDevicePath); } + FreePool (HandleBuffer); +Done: CoreCloseEvent (Event); - return; } /** -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 2/2] MdeModulePkg/Core: fix ineffective guard page issue
>v3: no change REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1295 This issue originates from following patch which allows to enable paging if PcdImageProtectionPolicy and PcdDxeNxMemoryProtectionPolicy (in addition to PcdSetNxForStack) are set to enable related features. 5267926134d17e86672b84fd57b438f05ffa68e1 Due to above change, PcdImageProtectionPolicy will be set to 0 by default in many platforms, which, in turn, cause following code in MdeModulePkg\Core\Dxe\Misc\MemoryProtection.c fail the creation of notify event of CpuArchProtocol. 1138: if (mImageProtectionPolicy != 0 || PcdGet64 (PcdDxeNxMemoryProtectionPolicy) != 0) { 1139: Status = CoreCreateEvent ( ... 1142: MemoryProtectionCpuArchProtocolNotify, ... 1145: ); Then following call flow won't be done and Guard pages will not be set as not-present in SetAllGuardPages() eventually. MemoryProtectionCpuArchProtocolNotify() => HeapGuardCpuArchProtocolNotify() => SetAllGuardPages() The solution is removing the if(...) statement so that the notify event will always be created and registered. This won't cause unnecessary code execution because, in the notify event handler, the related PCDs like PcdImageProtectionPolicy and PcdDxeNxMemoryProtectionPolicy will be checked again before doing related jobs. Cc: Star Zeng Cc: Jiewen Yao Cc: Ruiyu Ni Cc: Leif Lindholm Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 36 +-- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c index 8a93c5362a..c43e0d08ae 100644 --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c @@ -1136,26 +1136,24 @@ CoreInitializeMemoryProtection ( ASSERT (GetPermissionAttributeForMemoryType (EfiBootServicesData) == GetPermissionAttributeForMemoryType (EfiConventionalMemory)); - if (mImageProtectionPolicy != 0 || PcdGet64 (PcdDxeNxMemoryProtectionPolicy) != 0) { -Status = CoreCreateEvent ( - EVT_NOTIFY_SIGNAL, - TPL_CALLBACK, - MemoryProtectionCpuArchProtocolNotify, - NULL, - - ); -ASSERT_EFI_ERROR(Status); + Status = CoreCreateEvent ( + EVT_NOTIFY_SIGNAL, + TPL_CALLBACK, + MemoryProtectionCpuArchProtocolNotify, + NULL, + + ); + ASSERT_EFI_ERROR(Status); -// -// Register for protocol notifactions on this event -// -Status = CoreRegisterProtocolNotify ( - , - Event, - - ); -ASSERT_EFI_ERROR(Status); - } + // + // Register for protocol notifactions on this event + // + Status = CoreRegisterProtocolNotify ( + , + Event, + + ); + ASSERT_EFI_ERROR(Status); // // Register a callback to disable NULL pointer detection at EndOfDxe -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 0/2] fix ineffective guard page issue
>v3: found and fixed another memory leak REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1295 Cc: Star Zeng Cc: Jiewen Yao Cc: Ruiyu Ni Cc: Leif Lindholm Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang Jian J Wang (2): MdeModulePkg/Core: fill logic hole in MemoryProtectionCpuArchProtocolNotify MdeModulePkg/Core: fix ineffective guard page issue MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 45 +-- 1 file changed, 22 insertions(+), 23 deletions(-) -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 2/2] MdeModulePkg/Core: fix ineffective guard page issue
> v2: re-generate this patch per Leif's comments. No logic changes. REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1295 This issue originates from following patch which allows to enable paging if PcdImageProtectionPolicy and PcdDxeNxMemoryProtectionPolicy (in addition to PcdSetNxForStack) are set to enable related features. 5267926134d17e86672b84fd57b438f05ffa68e1 Due to above change, PcdImageProtectionPolicy will be set to 0 by default in many platforms, which, in turn, cause following code in MdeModulePkg\Core\Dxe\Misc\MemoryProtection.c fail the creation of notify event of CpuArchProtocol. 1138: if (mImageProtectionPolicy != 0 || PcdGet64 (PcdDxeNxMemoryProtectionPolicy) != 0) { 1139: Status = CoreCreateEvent ( ... 1142: MemoryProtectionCpuArchProtocolNotify, ... 1145: ); Then following call flow won't be done and Guard pages will not be set as not-present in SetAllGuardPages() eventually. MemoryProtectionCpuArchProtocolNotify() => HeapGuardCpuArchProtocolNotify() => SetAllGuardPages() The solution is removing the if(...) statement so that the notify event will always be created and registered. This won't cause unnecessary code execution because, in the notify event handler, the related PCDs like PcdImageProtectionPolicy and PcdDxeNxMemoryProtectionPolicy will be checked again before doing related jobs. Cc: Star Zeng Cc: Jiewen Yao Cc: Ruiyu Ni Cc: Leif Lindholm Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 36 +-- 1 file changed, 17 insertions(+), 19 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c index 98fdc2c618..30798b05b9 100644 --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c @@ -1135,26 +1135,24 @@ CoreInitializeMemoryProtection ( ASSERT (GetPermissionAttributeForMemoryType (EfiBootServicesData) == GetPermissionAttributeForMemoryType (EfiConventionalMemory)); - if (mImageProtectionPolicy != 0 || PcdGet64 (PcdDxeNxMemoryProtectionPolicy) != 0) { -Status = CoreCreateEvent ( - EVT_NOTIFY_SIGNAL, - TPL_CALLBACK, - MemoryProtectionCpuArchProtocolNotify, - NULL, - - ); -ASSERT_EFI_ERROR(Status); + Status = CoreCreateEvent ( + EVT_NOTIFY_SIGNAL, + TPL_CALLBACK, + MemoryProtectionCpuArchProtocolNotify, + NULL, + + ); + ASSERT_EFI_ERROR(Status); -// -// Register for protocol notifactions on this event -// -Status = CoreRegisterProtocolNotify ( - , - Event, - - ); -ASSERT_EFI_ERROR(Status); - } + // + // Register for protocol notifactions on this event + // + Status = CoreRegisterProtocolNotify ( + , + Event, + + ); + ASSERT_EFI_ERROR(Status); // // Register a callback to disable NULL pointer detection at EndOfDxe -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 1/2] MdeModulePkg/Core: fill logic hole in MemoryProtectionCpuArchProtocolNotify
> v2: re-generate this patch per Leif's comments. No logic changes. At the end of of MemoryProtectionCpuArchProtocolNotify there's cleanup code to free resource. But at line 978, 994, 1005 the function returns directly. This patch use "goto" to replace "return" to make sure the resource is freed before exit. 1029: CoreCloseEvent (Event); 1030: return; Cc: Star Zeng Cc: Jiewen Yao Cc: Ruiyu Ni Cc: Leif Lindholm Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 8 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c index 6298b67db1..98fdc2c618 100644 --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c @@ -975,7 +975,7 @@ MemoryProtectionCpuArchProtocolNotify ( DEBUG ((DEBUG_INFO, "MemoryProtectionCpuArchProtocolNotify:\n")); Status = CoreLocateProtocol (, NULL, (VOID **)); if (EFI_ERROR (Status)) { -return; +goto Done; } // @@ -991,7 +991,7 @@ MemoryProtectionCpuArchProtocolNotify ( HeapGuardCpuArchProtocolNotify (); if (mImageProtectionPolicy == 0) { -return; +goto Done; } Status = gBS->LocateHandleBuffer ( @@ -1002,7 +1002,7 @@ MemoryProtectionCpuArchProtocolNotify ( ); if (EFI_ERROR (Status) && (NoHandles == 0)) { -return ; +goto Done; } for (Index = 0; Index < NoHandles; Index++) { @@ -1026,8 +1026,8 @@ MemoryProtectionCpuArchProtocolNotify ( ProtectUefiImage (LoadedImage, LoadedImageDevicePath); } +Done: CoreCloseEvent (Event); - return; } /** -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 0/2] *fix ineffective guard page issue
>v2: re-generate the patch per Leif's comments REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1295 Cc: Star Zeng Cc: Jiewen Yao Cc: Ruiyu Ni Cc: Leif Lindholm Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang Jian J Wang (2): MdeModulePkg/Core: fill logic hole in MemoryProtectionCpuArchProtocolNotify MdeModulePkg/Core: fix ineffective guard page issue MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 44 +-- 1 file changed, 21 insertions(+), 23 deletions(-) -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 0/2] fix ineffective guard page issue
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1295 Jian J Wang (2): MdeModulePkg/Core: fill logic hole in MemoryProtectionCpuArchProtocolNotify MdeModulePkg/Core: fix ineffective guard page issue MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 44 +-- 1 file changed, 21 insertions(+), 23 deletions(-) -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 2/2] MdeModulePkg/Core: fix ineffective guard page issue
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1295 This issue originates from following patch which allows to enable paging if PcdImageProtectionPolicy and PcdDxeNxMemoryProtectionPolicy (in addition to PcdSetNxForStack) are set to enable related features. 5267926134d17e86672b84fd57b438f05ffa68e1 Due to above change, PcdImageProtectionPolicy will be set to 0 by default in many platforms, which, in turn, cause following code in MdeModulePkg\Core\Dxe\Misc\MemoryProtection.c fail the creation of notify event of CpuArchProtocol. 1138: if (mImageProtectionPolicy != 0 || PcdGet64 (PcdDxeNxMemoryProtectionPolicy) != 0) { 1139: Status = CoreCreateEvent ( ... 1142: MemoryProtectionCpuArchProtocolNotify, ... 1145: ); Then following call flow won't be done and Guard pages will not be set as not-present in SetAllGuardPages() eventually. MemoryProtectionCpuArchProtocolNotify() => HeapGuardCpuArchProtocolNotify() => SetAllGuardPages() The solution is removing the if(...) statement so that the notify event will always be created and handler be registered. This won't cause unnecessary code execution because, in the notify event handler, the related PCDs like PcdImageProtectionPolicy and PcdDxeNxMemoryProtectionPolicy will be checked again to do its job. Cc: Star Zeng Cc: Jiewen Yao Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c index 30e5c5153c..30798b05b9 100644 --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c @@ -1135,7 +1135,6 @@ CoreInitializeMemoryProtection ( ASSERT (GetPermissionAttributeForMemoryType (EfiBootServicesData) == GetPermissionAttributeForMemoryType (EfiConventionalMemory)); - if (mImageProtectionPolicy != 0 || PcdGet64 (PcdDxeNxMemoryProtectionPolicy) != 0) { Status = CoreCreateEvent ( EVT_NOTIFY_SIGNAL, TPL_CALLBACK, @@ -1154,7 +1153,6 @@ CoreInitializeMemoryProtection ( ); ASSERT_EFI_ERROR(Status); - } // // Register a callback to disable NULL pointer detection at EndOfDxe -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 1/2] MdeModulePkg/Core: fill logic hole in MemoryProtectionCpuArchProtocolNotify
At the end of of MemoryProtectionCpuArchProtocolNotify there's cleanup code to free resource. But at line 978, 994, 1005 the function returns directly. This patch use "goto" to replace "return" to make sure the resource is freed before exit. 1029: CoreCloseEvent (Event); 1030: return; Cc: Star Zeng Cc: Jiewen Yao Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 42 +-- 1 file changed, 21 insertions(+), 21 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c index 6298b67db1..30e5c5153c 100644 --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c @@ -975,7 +975,7 @@ MemoryProtectionCpuArchProtocolNotify ( DEBUG ((DEBUG_INFO, "MemoryProtectionCpuArchProtocolNotify:\n")); Status = CoreLocateProtocol (, NULL, (VOID **)); if (EFI_ERROR (Status)) { -return; +goto Done; } // @@ -991,7 +991,7 @@ MemoryProtectionCpuArchProtocolNotify ( HeapGuardCpuArchProtocolNotify (); if (mImageProtectionPolicy == 0) { -return; +goto Done; } Status = gBS->LocateHandleBuffer ( @@ -1002,7 +1002,7 @@ MemoryProtectionCpuArchProtocolNotify ( ); if (EFI_ERROR (Status) && (NoHandles == 0)) { -return ; +goto Done; } for (Index = 0; Index < NoHandles; Index++) { @@ -1026,8 +1026,8 @@ MemoryProtectionCpuArchProtocolNotify ( ProtectUefiImage (LoadedImage, LoadedImageDevicePath); } +Done: CoreCloseEvent (Event); - return; } /** @@ -1136,24 +1136,24 @@ CoreInitializeMemoryProtection ( GetPermissionAttributeForMemoryType (EfiConventionalMemory)); if (mImageProtectionPolicy != 0 || PcdGet64 (PcdDxeNxMemoryProtectionPolicy) != 0) { -Status = CoreCreateEvent ( - EVT_NOTIFY_SIGNAL, - TPL_CALLBACK, - MemoryProtectionCpuArchProtocolNotify, - NULL, - - ); -ASSERT_EFI_ERROR(Status); + Status = CoreCreateEvent ( + EVT_NOTIFY_SIGNAL, + TPL_CALLBACK, + MemoryProtectionCpuArchProtocolNotify, + NULL, + + ); + ASSERT_EFI_ERROR(Status); -// -// Register for protocol notifactions on this event -// -Status = CoreRegisterProtocolNotify ( - , - Event, - - ); -ASSERT_EFI_ERROR(Status); + // + // Register for protocol notifactions on this event + // + Status = CoreRegisterProtocolNotify ( + , + Event, + + ); + ASSERT_EFI_ERROR(Status); } // -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] MdeModulePkg/Core: fix an issue of potential NULL pointer access
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1286 This issue is introduced by bb685071c2602cf786ea84c69bbebf2158194a38. The *MemorySpaceMap assigned with NULL (line 1710) value might be accessed (line 1726/1730) without any sanity check. Although it won't happen in practice because of line 1722, we still need to add check against NULL to make static code analyzer happy. 1710 *MemorySpaceMap = NULL; ... 1722 if (DescriptorCount == *NumberOfDescriptors) { ... 1726Descriptor = *MemorySpaceMap; ... 1730BuildMemoryDescriptor (Descriptor, Entry); Tests: Pass build and boot to shell. Cc: Hao Wu Cc: Star Zeng Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c index 8bbdf7129f..a76d2db73c 100644 --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c @@ -1719,7 +1719,7 @@ CoreGetMemorySpaceMap ( // AllocatePool() called below has to be running outside the GCD lock. // DescriptorCount = CoreCountGcdMapEntry (); -if (DescriptorCount == *NumberOfDescriptors) { +if (DescriptorCount == *NumberOfDescriptors && *MemorySpaceMap != NULL) { // // Fill in the MemorySpaceMap if no memory space map change. // -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] MdeModulePkg/Core: correct one coding style
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1284 Non-Boolean comparisons should use a compare operator (==, !=, >, < >=, <=) Cc: Dandan Bi Cc: Star Zeng Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/Core/Dxe/Mem/HeapGuard.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c index 521e0d7b2a..9f89df8d8f 100644 --- a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c +++ b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c @@ -1559,7 +1559,7 @@ PromoteGuardedFreePages ( } } - if (AvailablePages) { + if (AvailablePages != 0) { DEBUG ((DEBUG_INFO, "Promoted pages: %lX (%lx)\r\n", Start, (UINT64)AvailablePages)); ClearGuardedMemoryBits (Start, AvailablePages); -- 2.19.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: optimize TLB flush operation
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1281 This optimization has two purpose: 1. fix BZ#1281 which caused by flushing TLB for AP 2. improve performance for SMM heap guard The code change is simple: just flush TLB for current processor. Since processor's (including AP) SMI entry code will always initialize CR3, it looks like that there's no need to add extra code in AP handler, called from SMI entry, to flush TLB again. Let each processor itself guarantee the TLB integrity can improve memory operations performance if Heap Guard is enabled. This has been proved by CpuDxe driver. Please check following patches for details. 41a9c3fd110bed93c4fdf088eea18412bb2dfcde 0dbb0f1a5ce6a9ec5213c85e5d4244cf5b061417 Stop flush TLB for APs (DXE) upon change 199de89677deff30eda7ad17793b30042cce Let AP (DXE) flush TLB in its wake-up procedure Tests: a. Verified that issue in BZ#1281 is gone b. Verified SMM heap guard works well on any processor c. OVMF boot (Fedora26, Ubuntu18.04, Windows 10) Cc: Eric Dong Cc: Laszlo Ersek Cc: Jiewen Yao Cc: Star Zeng Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c | 45 +++--- 1 file changed, 6 insertions(+), 39 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c index 684b14dc28..e0bf0cd5ac 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmCpuMemoryManagement.c @@ -464,41 +464,6 @@ ConvertMemoryPageAttributes ( return RETURN_SUCCESS; } -/** - FlushTlb on current processor. - - @param[in,out] Buffer Pointer to private data buffer. -**/ -VOID -EFIAPI -FlushTlbOnCurrentProcessor ( - IN OUT VOID *Buffer - ) -{ - CpuFlushTlb (); -} - -/** - FlushTlb for all processors. -**/ -VOID -FlushTlbForAll ( - VOID - ) -{ - UINTN Index; - - FlushTlbOnCurrentProcessor (NULL); - - for (Index = 0; Index < gSmst->NumberOfCpus; Index++) { -if (Index != gSmst->CurrentlyExecutingCpu) { - // Force to start up AP in blocking mode, - SmmBlockingStartupThisAp (FlushTlbOnCurrentProcessor, Index, NULL); - // Do not check return status, because AP might not be present in some corner cases. -} - } -} - /** This function sets the attributes for the memory region specified by BaseAddress and Length from their current attributes to the attributes specified by Attributes. @@ -538,9 +503,10 @@ SmmSetMemoryAttributesEx ( if (!EFI_ERROR(Status)) { if (IsModified) { // - // Flush TLB as last step + // Flush TLB as last step. No need to do it for APs, which sould take care + // of it in the wake-up procedure. // - FlushTlbForAll(); + CpuFlushTlb (); } } @@ -586,9 +552,10 @@ SmmClearMemoryAttributesEx ( if (!EFI_ERROR(Status)) { if (IsModified) { // - // Flush TLB as last step + // Flush TLB as last step. No need to do it for APs, which sould take care + // of it in the wake-up procedure. // - FlushTlbForAll(); + CpuFlushTlb (); } } -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] MdeModulePkg/Core: fix an IA32 build failure
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1277 The failure is caused by data type conversion between UINTN and UINT64, which is checked in at 63ebde8ef6d4ff497d054ccc010904ecd4441198. Cc: Star Zeng Cc: Liming Gao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/Core/Dxe/Mem/HeapGuard.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c index 449a022658..521e0d7b2a 100644 --- a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c +++ b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c @@ -463,7 +463,7 @@ IsGuardPage ( IN EFI_PHYSICAL_ADDRESSAddress ) { - UINTN BitMap; + UINT64BitMap; // // There must be at least one guarded page before and/or after given @@ -1368,7 +1368,7 @@ GuardAllFreedPages ( UINT64Address; UINT64GuardPage; INTN Level; - UINTN BitIndex; + UINT64BitIndex; UINTN GuardPageNumber; if (mGuardedMemoryMap == 0 || @@ -1475,12 +1475,12 @@ MergeGuardPages ( } Bitmap = 0; - Pages = EFI_SIZE_TO_PAGES (MaxAddress - MemoryMapEntry->PhysicalStart); - Pages -= MemoryMapEntry->NumberOfPages; + Pages = EFI_SIZE_TO_PAGES ((UINTN)(MaxAddress - MemoryMapEntry->PhysicalStart)); + Pages -= (INTN)MemoryMapEntry->NumberOfPages; while (Pages > 0) { if (Bitmap == 0) { EndAddress = MemoryMapEntry->PhysicalStart + - EFI_PAGES_TO_SIZE (MemoryMapEntry->NumberOfPages); + EFI_PAGES_TO_SIZE ((UINTN)MemoryMapEntry->NumberOfPages); Bitmap = GetGuardedMemoryBits (EndAddress, GUARDED_HEAP_MAP_ENTRY_BITS); } -- 2.19.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v4 4/6] UefiCpuPkg/CpuDxe: prevent recursive calling of InitializePageTablePool
> v4 changes: none The freed-memory guard feature will cause an recursive calling of InitializePageTablePool(). This is due to a fact that AllocateAlignedPages() is used to allocate page table pool memory. This function will most likely call gBS->FreePages to free unaligned pages and then cause another round of page attributes change, like below (freed pages will be always marked not-present if freed-memory guard is enabled) FreePages() <===| => CpuSetMemoryAttributes()| => | => InitializePageTablePool() | => AllocateAlignedPages() | => FreePages() | The solution is add a global variable as a lock in page table pool allocation function and fail any other requests if it has not been done. Since this issue will only happen if free-memory guard is enabled, it won't affect CpuSetMemoryAttributes() in default build of a BIOS. If free-memory guard is enabled, it only affect the pages (failed to mark them as not-present) freed in AllocateAlignedPages(). Since those freed pages haven't been used yet (their addresses not yet exposed to code outside AllocateAlignedPages), it won't compromise the freed-memory guard feature. This change will just fail the CpuSetMemoryAttributes() called from FreePages() but it won't fail the FreePages(). So the error status won't be propagated upper layer of code. Cc: Eric Dong Cc: Laszlo Ersek Cc: Star Zeng Cc: Michael D Kinney Cc: Jiewen Yao Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/CpuDxe/CpuPageTable.c | 23 +-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c index 33e8ee2d2c..4bee8c7772 100644 --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c @@ -100,6 +100,7 @@ PAGE_ATTRIBUTE_TABLE mPageAttributeTable[] = { }; PAGE_TABLE_POOL *mPageTablePool = NULL; +BOOLEAN mPageTablePoolLock = FALSE; PAGE_TABLE_LIB_PAGING_CONTEXT mPagingContext; EFI_SMM_BASE2_PROTOCOL*mSmmBase2 = NULL; @@ -1046,6 +1047,16 @@ InitializePageTablePool ( VOID *Buffer; BOOLEAN IsModified; + // + // Do not allow re-entrance. + // + if (mPageTablePoolLock) { +return FALSE; + } + + mPageTablePoolLock = TRUE; + IsModified = FALSE; + // // Always reserve at least PAGE_TABLE_POOL_UNIT_PAGES, including one page for // header. @@ -1056,9 +1067,15 @@ InitializePageTablePool ( Buffer = AllocateAlignedPages (PoolPages, PAGE_TABLE_POOL_ALIGNMENT); if (Buffer == NULL) { DEBUG ((DEBUG_ERROR, "ERROR: Out of aligned pages\r\n")); -return FALSE; +goto Done; } + DEBUG (( +DEBUG_INFO, +"Paging: added %lu pages to page table pool\r\n", +(UINT64)PoolPages +)); + // // Link all pools into a list for easier track later. // @@ -1092,7 +1109,9 @@ InitializePageTablePool ( ); ASSERT (IsModified == TRUE); - return TRUE; +Done: + mPageTablePoolLock = FALSE; + return IsModified; } /** -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v4 5/6] MdeModulePkg/Core: prevent re-acquire GCD memory lock
> v4 changes: > a. add more comments from Laszlo This issue is hidden in current code but exposed by introduction of freed-memory guard feature due to the fact that the feature will turn all pool allocation to page allocation. The solution is move the memory allocation in CoreGetMemorySpaceMap() and CoreGetIoSpaceMap() to be out of the GCD memory map lock. CoreDumpGcdMemorySpaceMap() => CoreGetMemorySpaceMap() => CoreAcquireGcdMemoryLock () * AllocatePool() => InternalAllocatePool() => CoreAllocatePool() => CoreAllocatePoolI() => CoreAllocatePoolPagesI() => CoreAllocatePoolPages() => FindFreePages() => PromoteMemoryResource() => CoreAcquireGcdMemoryLock() ** 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/Gcd/Gcd.c | 87 + 1 file changed, 62 insertions(+), 25 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c index d9c65a8045..f99d6bb933 100644 --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c @@ -1691,10 +1691,10 @@ CoreGetMemorySpaceMap ( OUT EFI_GCD_MEMORY_SPACE_DESCRIPTOR **MemorySpaceMap ) { - EFI_STATUS Status; LIST_ENTRY *Link; EFI_GCD_MAP_ENTRY*Entry; EFI_GCD_MEMORY_SPACE_DESCRIPTOR *Descriptor; + UINTNDescriptorCount; // // Make sure parameters are valid @@ -1706,38 +1706,75 @@ CoreGetMemorySpaceMap ( return EFI_INVALID_PARAMETER; } - CoreAcquireGcdMemoryLock (); + *NumberOfDescriptors = 0; + *MemorySpaceMap = NULL; // - // Count the number of descriptors + // Take the lock, for entering the loop with the lock held. // - *NumberOfDescriptors = CoreCountGcdMapEntry (); + CoreAcquireGcdMemoryLock (); + while (TRUE) { +// +// Count the number of descriptors. It might be done more than once because +// there's code which has to be running outside the GCD lock. +// +DescriptorCount = CoreCountGcdMapEntry (); +if (DescriptorCount == *NumberOfDescriptors) { + // + // Fill in the MemorySpaceMap if no memory space map change. + // + Descriptor = *MemorySpaceMap; + Link = mGcdMemorySpaceMap.ForwardLink; + while (Link != ) { +Entry = CR (Link, EFI_GCD_MAP_ENTRY, Link, EFI_GCD_MAP_SIGNATURE); +BuildMemoryDescriptor (Descriptor, Entry); +Descriptor++; +Link = Link->ForwardLink; + } + // + // We're done; exit the loop with the lock held. + // + break; +} - // - // Allocate the MemorySpaceMap - // - *MemorySpaceMap = AllocatePool (*NumberOfDescriptors * sizeof (EFI_GCD_MEMORY_SPACE_DESCRIPTOR)); - if (*MemorySpaceMap == NULL) { -Status = EFI_OUT_OF_RESOURCES; -goto Done; - } +// +// Release the lock before memory allocation, because it might cause +// GCD lock conflict in one of calling path in AllocatPool(). +// +CoreReleaseGcdMemoryLock (); + +// +// Allocate memory to store the MemorySpaceMap. Note it might be already +// allocated if there's map descriptor change during memory allocation at +// last time. +// +if (*MemorySpaceMap != NULL) { + FreePool (*MemorySpaceMap); +} +*MemorySpaceMap = AllocatePool (DescriptorCount * +sizeof (EFI_GCD_MEMORY_SPACE_DESCRIPTOR)); +if (*MemorySpaceMap == NULL) { + *NumberOfDescriptors = 0; + return EFI_OUT_OF_RESOURCES; +} + +// +// Save the descriptor count got before for another round of check to make +// sure we won't miss any, since we have code running outside the GCD lock. +// +*NumberOfDescriptors = DescriptorCount; +// +// Re-acquire the lock, for the next iteration. +// +CoreAcquireGcdMemoryLock (); + } // - // Fill in the MemorySpaceMap + // We exited the loop with the lock held, release it. // - Descriptor = *MemorySpaceMap; - Link = mGcdMemorySpaceMap.ForwardLink; - while (Link != ) { -Entry = CR (Link, EFI_GCD_MAP_ENTRY, Link, EFI_GCD_MAP_SIGNATURE); -BuildMemoryDescriptor (Descriptor, Entry); -Descriptor++; -Link = Link->ForwardLink; - } - Status = EFI_SUCCESS; - -Done: CoreReleaseGcdMemoryLock (); - return Status; + + return EFI_SUCCESS; } -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v4 6/6] MdeModulePkg/Core: add freed-memory guard feature
> 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
[edk2] [PATCH v4 3/6] UefiCpuPkg/CpuDxe: consider freed-memory guard in non-stop mode
> v4 changes: none Non-stop mode was introduced / explained in commit 8f2613628acf ("MdeModulePkg/MdeModulePkg.dec: add new settings for PCDs", 2018-08-30). The macro HEAP_GUARD_NONSTOP_MODE was added to CpuDxe in commit dcc026217fdc ("UefiCpuPkg/CpuDxe: implement non-stop mode for uefi", 2018-08-30). Another instance of the macro HEAP_GUARD_NONSTOP_MODE was added to PiSmmCpuDxeSmm -- with BIT1|BIT0 replaced with BIT3|BIT2 -- in commit 09afd9a42a7f ("UefiCpuPkg/PiSmmCpuDxeSmm: implement non-stop mode for SMM", 2018-08-30) Since the freed-memory guard is for UEFI-only. This patch only updates HEAP_GUARD_NONSTOP_MODE in "UefiCpuPkg/CpuDxe/CpuDxe.h" (add BIT4). Cc: Eric Dong Cc: Laszlo Ersek Cc: Star Zeng Cc: Michael D Kinney Cc: Jiewen Yao Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/CpuDxe/CpuDxe.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.h b/UefiCpuPkg/CpuDxe/CpuDxe.h index 064ea05bba..3183a3f7f4 100644 --- a/UefiCpuPkg/CpuDxe/CpuDxe.h +++ b/UefiCpuPkg/CpuDxe/CpuDxe.h @@ -58,7 +58,7 @@ ) #define HEAP_GUARD_NONSTOP_MODE \ -((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT6|BIT1|BIT0)) > BIT6) +((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT6|BIT4|BIT1|BIT0)) > BIT6) #define NULL_DETECTION_NONSTOP_MODE \ ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & (BIT6|BIT0)) > BIT6) -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v4 2/6] MdeModulePkg: introduce UEFI freed-memory guard bit in HeapGuard PCD
> v4 changes: > a. refine PCD description of PcdHeapGuardPropertyMask UAF (Use-After-Free) memory issue is kind of illegal access to memory which has been freed. It can be detected by a new freed-memory guard enforced onto freed memory. BIT4 of following PCD is used to enable the freed-memory guard feature. gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask Please note this feature is for debug purpose and should not be enabled in product BIOS, and cannot be enabled with pool/page heap guard at the same time. It's disabled by default. 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/MdeModulePkg.dec | 16 MdeModulePkg/MdeModulePkg.uni | 14 ++ 2 files changed, 22 insertions(+), 8 deletions(-) diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 2009dbc5fd..428eeeb670 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -1011,14 +1011,22 @@ gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType|0x0|UINT64|0x30001053 ## This mask is to control Heap Guard behavior. - # Note that due to the limit of pool memory implementation and the alignment - # requirement of UEFI spec, BIT7 is a try-best setting which cannot guarantee - # that the returned pool is exactly adjacent to head guard page or tail guard - # page. + # + # Note: + # a) Heap Guard is for debug purpose and should not be enabled in product + # BIOS. + # b) Due to the limit of pool memory implementation and the alignment + # requirement of UEFI spec, BIT7 is a try-best setting which cannot + # guarantee that the returned pool is exactly adjacent to head guard + # page or tail guard page. + # c) UEFI freed-memory guard and UEFI pool/page guard cannot be enabled + # at the same time. + # # BIT0 - Enable UEFI page guard. # BIT1 - Enable UEFI pool guard. # BIT2 - Enable SMM page guard. # BIT3 - Enable SMM pool guard. + # BIT4 - Enable UEFI freed-memory guard (Use-After-Free memory detection). # BIT6 - Enable non-stop mode. # BIT7 - The direction of Guard Page for Pool Guard. # 0 - The returned pool is near the tail guard page. diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni index 9d2e473fa9..5fa7a6ae30 100644 --- a/MdeModulePkg/MdeModulePkg.uni +++ b/MdeModulePkg/MdeModulePkg.uni @@ -1224,14 +1224,20 @@ #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdHeapGuardPropertyMask_PROMPT #language en-US "The Heap Guard feature mask" #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdHeapGuardPropertyMask_HELP #language en-US "This mask is to control Heap Guard behavior.\n" - "Note that due to the limit of pool memory implementation and the alignment\n" - "requirement of UEFI spec, BIT7 is a try-best setting which cannot guarantee\n" - "that the returned pool is exactly adjacent to head guard page or tail guard\n" - "page.\n" + " Note:\n" + " a) Heap Guard is for debug purpose and should not be enabled in product" + " BIOS.\n" + " b) Due to the limit of pool memory implementation and the alignment" + " requirement of UEFI spec, BIT7 is a try-best setting which cannot" + " guarantee that the returned pool is exactly adjacent to head guard" + " page or tail guard page.\n" + " c) UEFI freed-memory guard and UEFI pool/page guard cannot be enabled" + "
[edk2] [PATCH v4 1/6] MdeModulePkg: cleanup Heap Guard pool/page type PCD documentation
> v4 changes: none This cleanup is meant for avoiding misuse of newly introduced BIT4 (UAF detection) of PCD PcdHeapGuardPropertyMask, because it applies to all types of physical memory. In another words, PcdHeapGuardPoolType and PcdHeapGuardPageType don't have effect to the BIT4 of PcdHeapGuardPropertyMask. 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/MdeModulePkg.dec | 4 MdeModulePkg/MdeModulePkg.uni | 2 ++ 2 files changed, 6 insertions(+) diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 6037504fa7..2009dbc5fd 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -955,6 +955,8 @@ # free pages for all of them. The page allocation for the type related to # cleared bits keeps the same as ususal. # + # This PCD is only valid if BIT0 and/or BIT2 are set in PcdHeapGuardPropertyMask. + # # Below is bit mask for this PCD: (Order is same as UEFI spec) # EfiReservedMemoryType 0x0001 # EfiLoaderCode 0x0002 @@ -984,6 +986,8 @@ # if there's enough free memory for all of them. The pool allocation for the # type related to cleared bits keeps the same as ususal. # + # This PCD is only valid if BIT1 and/or BIT3 are set in PcdHeapGuardPropertyMask. + # # Below is bit mask for this PCD: (Order is same as UEFI spec) # EfiReservedMemoryType 0x0001 # EfiLoaderCode 0x0002 diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni index a6bcb627cf..9d2e473fa9 100644 --- a/MdeModulePkg/MdeModulePkg.uni +++ b/MdeModulePkg/MdeModulePkg.uni @@ -1171,6 +1171,7 @@ " before and after corresponding type of pages allocated if there's enough\n" " free pages for all of them. The page allocation for the type related to\n" " cleared bits keeps the same as ususal.\n\n" + " This PCD is only valid if BIT0 and/or BIT2 are set in PcdHeapGuardPropertyMask.\n\n" " Below is bit mask for this PCD: (Order is same as UEFI spec)\n" " EfiReservedMemoryType 0x0001\n" " EfiLoaderCode 0x0002\n" @@ -1198,6 +1199,7 @@ " before and after corresponding type of pages which the allocated pool occupies,\n" " if there's enough free memory for all of them. The pool allocation for the\n" " type related to cleared bits keeps the same as ususal.\n\n" + " This PCD is only valid if BIT1 and/or BIT3 are set in PcdHeapGuardPropertyMask.\n\n" " Below is bit mask for this PCD: (Order is same as UEFI spec)\n" " EfiReservedMemoryType 0x0001\n" " EfiLoaderCode 0x0002\n" -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v4 0/6] Introduce freed-memory guard feature
> v4 changes: > Updated per comments from Star. Please refer to individual patch > file for details (#2/5/6) Freed-memory guard is a new feauture used to detect UAF (Use-After-Free) memory issue. Tests: a. Feature basic unit/functionality test b. OVMF regression test Jian J Wang (6): MdeModulePkg: cleanup Heap Guard pool/page type PCD documentation MdeModulePkg: introduce UEFI freed-memory guard bit in HeapGuard PCD UefiCpuPkg/CpuDxe: consider freed-memory guard in non-stop mode UefiCpuPkg/CpuDxe: prevent recursive calling of InitializePageTablePool MdeModulePkg/Core: prevent re-acquire GCD memory lock MdeModulePkg/Core: add freed-memory guard feature MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 87 -- 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 +- MdeModulePkg/MdeModulePkg.dec | 20 +- MdeModulePkg/MdeModulePkg.uni | 16 +- UefiCpuPkg/CpuDxe/CpuDxe.h| 2 +- UefiCpuPkg/CpuDxe/CpuPageTable.c | 23 +- 11 files changed, 637 insertions(+), 70 deletions(-) -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 6/6] MdeModulePkg/Core: add freed-memory guard feature
> v3 changes: > a. Merge from #4 and #5 of v2 patch > b. Coding style cleanup 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. The freed memory block will not be added back into memory pool. This means that, once a page is allocated and freed, it cannot be re-allocated. This will bring an 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 | 41 ++- MdeModulePkg/Core/Dxe/Mem/Pool.c | 23 +- MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 2 +- MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 18 +- 6 files changed, 524 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. + @param[in] Pages Number of freed pages. + + @return VOID. +**/
[edk2] [PATCH v3 2/6] MdeModulePkg: introduce UEFI freed-memory guard bit in HeapGuard PCD
> v3 changes: > a. split from v2 #1 patch file. > b. refine the commit message and title. UAF (Use-After-Free) memory issue is kind of illegal access to memory which has been freed. It can be detected by a new freed-memory guard enforced onto freed memory. BIT4 of following PCD is used to enable the freed-memory guard feature. gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask Please note this feature is for debug purpose and should not be enabled in product BIOS, and cannot be enabled with pool/page heap guard at the same time. It's disabled by default. 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/MdeModulePkg.dec | 6 ++ MdeModulePkg/MdeModulePkg.uni | 4 +++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 2009dbc5fd..255b92ea67 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -1011,14 +1011,20 @@ gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType|0x0|UINT64|0x30001053 ## This mask is to control Heap Guard behavior. + # # Note that due to the limit of pool memory implementation and the alignment # requirement of UEFI spec, BIT7 is a try-best setting which cannot guarantee # that the returned pool is exactly adjacent to head guard page or tail guard # page. + # + # Note that UEFI freed-memory guard and pool/page guard cannot be enabled + # at the same time. + # # BIT0 - Enable UEFI page guard. # BIT1 - Enable UEFI pool guard. # BIT2 - Enable SMM page guard. # BIT3 - Enable SMM pool guard. + # BIT4 - Enable UEFI freed-memory guard (Use-After-Free memory detection). # BIT6 - Enable non-stop mode. # BIT7 - The direction of Guard Page for Pool Guard. # 0 - The returned pool is near the tail guard page. diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni index 9d2e473fa9..e72b893509 100644 --- a/MdeModulePkg/MdeModulePkg.uni +++ b/MdeModulePkg/MdeModulePkg.uni @@ -1227,11 +1227,13 @@ "Note that due to the limit of pool memory implementation and the alignment\n" "requirement of UEFI spec, BIT7 is a try-best setting which cannot guarantee\n" "that the returned pool is exactly adjacent to head guard page or tail guard\n" - "page.\n" + "page.\n\n" + "Note that UEFI freed-memory guard and pool/page guard cannot be enabled at the same time.\n\n" " BIT0 - Enable UEFI page guard.\n" " BIT1 - Enable UEFI pool guard.\n" " BIT2 - Enable SMM page guard.\n" " BIT3 - Enable SMM pool guard.\n" + " BIT4 - Enable UEFI freed-memory guard (Use-After-Free memory detection).\n" " BIT7 - The direction of Guard Page for Pool Guard.\n" " 0 - The returned pool is near the tail guard page.\n" " 1 - The returned pool is near the head guard page." -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 5/6] MdeModulePkg/Core: prevent re-acquire GCD memory lock
> v3 changes: > a. drop the changes to CoreGetIoSpaceMap() because it won't cause >problem > b. refine the logic in changes of CoreGetMemorySpaceMap() >and add more comments This issue is hidden in current code but exposed by introduction of freed-memory guard feature due to the fact that the feature will turn all pool allocation to page allocation. The solution is move the memory allocation in CoreGetMemorySpaceMap() and CoreGetIoSpaceMap() to be out of the GCD memory map lock. Although it's rare, a while-loop is added to make sure that the count of descriptor of memory map is the same after memory allocation, because it's executed outside the GCD memory lock. CoreDumpGcdMemorySpaceMap() => CoreGetMemorySpaceMap() => CoreAcquireGcdMemoryLock () * AllocatePool() => InternalAllocatePool() => CoreAllocatePool() => CoreAllocatePoolI() => CoreAllocatePoolPagesI() => CoreAllocatePoolPages() => FindFreePages() => PromoteMemoryResource() => CoreAcquireGcdMemoryLock() ** 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/Gcd/Gcd.c | 80 +++-- 1 file changed, 54 insertions(+), 26 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c index d9c65a8045..bc02945721 100644 --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c @@ -1691,10 +1691,10 @@ CoreGetMemorySpaceMap ( OUT EFI_GCD_MEMORY_SPACE_DESCRIPTOR **MemorySpaceMap ) { - EFI_STATUS Status; LIST_ENTRY *Link; EFI_GCD_MAP_ENTRY*Entry; EFI_GCD_MEMORY_SPACE_DESCRIPTOR *Descriptor; + UINTNDescriptorCount; // // Make sure parameters are valid @@ -1706,38 +1706,66 @@ CoreGetMemorySpaceMap ( return EFI_INVALID_PARAMETER; } + *NumberOfDescriptors = 0; + *MemorySpaceMap = NULL; + CoreAcquireGcdMemoryLock (); - // - // Count the number of descriptors - // - *NumberOfDescriptors = CoreCountGcdMapEntry (); + while (TRUE) { +// +// Count the number of descriptors. It might be done more than once because +// there's code which has to be running outside the GCD lock. +// +DescriptorCount = CoreCountGcdMapEntry (); +if (DescriptorCount == *NumberOfDescriptors) { + // + // Fill in the MemorySpaceMap if no memory space map change. + // + Descriptor = *MemorySpaceMap; + Link = mGcdMemorySpaceMap.ForwardLink; + while (Link != ) { +Entry = CR (Link, EFI_GCD_MAP_ENTRY, Link, EFI_GCD_MAP_SIGNATURE); +BuildMemoryDescriptor (Descriptor, Entry); +Descriptor++; +Link = Link->ForwardLink; + } - // - // Allocate the MemorySpaceMap - // - *MemorySpaceMap = AllocatePool (*NumberOfDescriptors * sizeof (EFI_GCD_MEMORY_SPACE_DESCRIPTOR)); - if (*MemorySpaceMap == NULL) { -Status = EFI_OUT_OF_RESOURCES; -goto Done; - } + break; +} - // - // Fill in the MemorySpaceMap - // - Descriptor = *MemorySpaceMap; - Link = mGcdMemorySpaceMap.ForwardLink; - while (Link != ) { -Entry = CR (Link, EFI_GCD_MAP_ENTRY, Link, EFI_GCD_MAP_SIGNATURE); -BuildMemoryDescriptor (Descriptor, Entry); -Descriptor++; -Link = Link->ForwardLink; +// +// Release the lock before memory allocation, because it might cause +// GCD lock conflict in one of calling path in AllocatPool(). +// +CoreReleaseGcdMemoryLock (); + +// +// Allocate memory to store the MemorySpaceMap. Note it might be already +// allocated if there's map descriptor change during memory allocation at +// last time. +// +if (*MemorySpaceMap != NULL) { + FreePool (*MemorySpaceMap); +} + +*MemorySpaceMap = AllocatePool (DescriptorCount * +sizeof (EFI_GCD_MEMORY_SPACE_DESCRIPTOR)); +if (*MemorySpaceMap == NULL) { + *NumberOfDescriptors = 0; + return EFI_OUT_OF_RESOURCES; +} + +// +// Save the descriptor number got before for another round of check to make +// sure we won't miss any, since we have code running outside the GCD lock. +// +*NumberOfDescriptors = DescriptorCount; +CoreAcquireGcdMemoryLock (); } - Status = EFI_SUCCESS; -Done: CoreReleaseGcdMemoryLock (); - return Status; + + return EFI_SUCCESS; } -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 0/6] Introduce freed-memory guard feature
> v3 changes: > Updated per comments from Laszlo. Please refer to individual patch > file for details Freed-memory guard is a new feauture used to detect UAF (Use-After-Free) memory issue. Tests: a. Feature basic unit/functionality test b. OVMF regression test Jian J Wang (6): MdeModulePkg: cleanup Heap Guard pool/page type PCD documentation MdeModulePkg: introduce UEFI freed-memory guard bit in HeapGuard PCD UefiCpuPkg/CpuDxe: consider freed-memory guard in non-stop mode UefiCpuPkg/CpuDxe: prevent recursive calling of InitializePageTablePool MdeModulePkg/Core: prevent re-acquire GCD memory lock MdeModulePkg/Core: add freed-memory guard feature MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 80 +++-- MdeModulePkg/Core/Dxe/Mem/HeapGuard.c | 409 +- MdeModulePkg/Core/Dxe/Mem/HeapGuard.h | 65 +++- MdeModulePkg/Core/Dxe/Mem/Page.c | 41 ++- MdeModulePkg/Core/Dxe/Mem/Pool.c | 23 +- MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 2 +- MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 18 +- MdeModulePkg/MdeModulePkg.dec | 10 + MdeModulePkg/MdeModulePkg.uni | 6 +- UefiCpuPkg/CpuDxe/CpuDxe.h| 2 +- UefiCpuPkg/CpuDxe/CpuPageTable.c | 23 +- 11 files changed, 615 insertions(+), 64 deletions(-) -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 3/6] UefiCpuPkg/CpuDxe: consider freed-memory guard in non-stop mode
> v3 changes: > a. split from #2 patch of v2 > b. refine the commit message > c. refine the title Non-stop mode was introduced / explained in commit 8f2613628acf ("MdeModulePkg/MdeModulePkg.dec: add new settings for PCDs", 2018-08-30). The macro HEAP_GUARD_NONSTOP_MODE was added to CpuDxe in commit dcc026217fdc ("UefiCpuPkg/CpuDxe: implement non-stop mode for uefi", 2018-08-30). Another instance of the macro HEAP_GUARD_NONSTOP_MODE was added to PiSmmCpuDxeSmm -- with BIT1|BIT0 replaced with BIT3|BIT2 -- in commit 09afd9a42a7f ("UefiCpuPkg/PiSmmCpuDxeSmm: implement non-stop mode for SMM", 2018-08-30) Since the freed-memory guard is for UEFI-only. This patch only updates HEAP_GUARD_NONSTOP_MODE in "UefiCpuPkg/CpuDxe/CpuDxe.h" (add BIT4). Cc: Laszlo Ersek Cc: Star Zeng Cc: Michael D Kinney Cc: Jiewen Yao Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/CpuDxe/CpuDxe.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.h b/UefiCpuPkg/CpuDxe/CpuDxe.h index 064ea05bba..3183a3f7f4 100644 --- a/UefiCpuPkg/CpuDxe/CpuDxe.h +++ b/UefiCpuPkg/CpuDxe/CpuDxe.h @@ -58,7 +58,7 @@ ) #define HEAP_GUARD_NONSTOP_MODE \ -((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT6|BIT1|BIT0)) > BIT6) +((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT6|BIT4|BIT1|BIT0)) > BIT6) #define NULL_DETECTION_NONSTOP_MODE \ ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & (BIT6|BIT0)) > BIT6) -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 1/6] MdeModulePkg: cleanup Heap Guard pool/page type PCD documentation
> v3 changes: > a. split from #1 patch of v2 > b. update title This cleanup is meant for avoiding misuse of newly introduced BIT4 (UAF detection) of PCD PcdHeapGuardPropertyMask, because it applies to all types of physical memory. In another words, PcdHeapGuardPoolType and PcdHeapGuardPageType don't have effect to the BIT4 of PcdHeapGuardPropertyMask. 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/MdeModulePkg.dec | 4 MdeModulePkg/MdeModulePkg.uni | 2 ++ 2 files changed, 6 insertions(+) diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 6037504fa7..2009dbc5fd 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -955,6 +955,8 @@ # free pages for all of them. The page allocation for the type related to # cleared bits keeps the same as ususal. # + # This PCD is only valid if BIT0 and/or BIT2 are set in PcdHeapGuardPropertyMask. + # # Below is bit mask for this PCD: (Order is same as UEFI spec) # EfiReservedMemoryType 0x0001 # EfiLoaderCode 0x0002 @@ -984,6 +986,8 @@ # if there's enough free memory for all of them. The pool allocation for the # type related to cleared bits keeps the same as ususal. # + # This PCD is only valid if BIT1 and/or BIT3 are set in PcdHeapGuardPropertyMask. + # # Below is bit mask for this PCD: (Order is same as UEFI spec) # EfiReservedMemoryType 0x0001 # EfiLoaderCode 0x0002 diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni index a6bcb627cf..9d2e473fa9 100644 --- a/MdeModulePkg/MdeModulePkg.uni +++ b/MdeModulePkg/MdeModulePkg.uni @@ -1171,6 +1171,7 @@ " before and after corresponding type of pages allocated if there's enough\n" " free pages for all of them. The page allocation for the type related to\n" " cleared bits keeps the same as ususal.\n\n" + " This PCD is only valid if BIT0 and/or BIT2 are set in PcdHeapGuardPropertyMask.\n\n" " Below is bit mask for this PCD: (Order is same as UEFI spec)\n" " EfiReservedMemoryType 0x0001\n" " EfiLoaderCode 0x0002\n" @@ -1198,6 +1199,7 @@ " before and after corresponding type of pages which the allocated pool occupies,\n" " if there's enough free memory for all of them. The pool allocation for the\n" " type related to cleared bits keeps the same as ususal.\n\n" + " This PCD is only valid if BIT1 and/or BIT3 are set in PcdHeapGuardPropertyMask.\n\n" " Below is bit mask for this PCD: (Order is same as UEFI spec)\n" " EfiReservedMemoryType 0x0001\n" " EfiLoaderCode 0x0002\n" -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v3 4/6] UefiCpuPkg/CpuDxe: prevent recursive calling of InitializePageTablePool
> v3 changes: > a. split from #2 patch > b. refine the commit message and title > c. remove "else" branch The freed-memory guard feature will cause an recursive calling of InitializePageTablePool(). This is due to a fact that AllocateAlignedPages() is used to allocate page table pool memory. This function will most likely call gBS->FreePages to free unaligned pages and then cause another round of page attributes change, like below (freed pages will be always marked not-present if freed-memory guard is enabled) FreePages() <===| => CpuSetMemoryAttributes()| => | => InitializePageTablePool() | => AllocateAlignedPages() | => FreePages() | The solution is add a global variable as a lock in page table pool allocation function and fail any other requests if it has not been done. Since this issue will only happen if free-memory guard is enabled, it won't affect CpuSetMemoryAttributes() in default build of a BIOS. If free-memory guard is enabled, it only affect the pages (failed to mark them as not-present) freed in AllocateAlignedPages(). Since those freed pages haven't been used yet (their addresses not yet exposed to code outside AllocateAlignedPages), it won't compromise the freed-memory guard feature. This change will just fail the CpuSetMemoryAttributes() called from FreePages() but it won't fail the FreePages(). So the error status won't be propagated to upper layer of code. Cc: Laszlo Ersek Cc: Star Zeng Cc: Michael D Kinney Cc: Jiewen Yao Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/CpuDxe/CpuPageTable.c | 23 +-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c index 33e8ee2d2c..4bee8c7772 100644 --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c @@ -100,6 +100,7 @@ PAGE_ATTRIBUTE_TABLE mPageAttributeTable[] = { }; PAGE_TABLE_POOL *mPageTablePool = NULL; +BOOLEAN mPageTablePoolLock = FALSE; PAGE_TABLE_LIB_PAGING_CONTEXT mPagingContext; EFI_SMM_BASE2_PROTOCOL*mSmmBase2 = NULL; @@ -1046,6 +1047,16 @@ InitializePageTablePool ( VOID *Buffer; BOOLEAN IsModified; + // + // Do not allow re-entrance. + // + if (mPageTablePoolLock) { +return FALSE; + } + + mPageTablePoolLock = TRUE; + IsModified = FALSE; + // // Always reserve at least PAGE_TABLE_POOL_UNIT_PAGES, including one page for // header. @@ -1056,9 +1067,15 @@ InitializePageTablePool ( Buffer = AllocateAlignedPages (PoolPages, PAGE_TABLE_POOL_ALIGNMENT); if (Buffer == NULL) { DEBUG ((DEBUG_ERROR, "ERROR: Out of aligned pages\r\n")); -return FALSE; +goto Done; } + DEBUG (( +DEBUG_INFO, +"Paging: added %lu pages to page table pool\r\n", +(UINT64)PoolPages +)); + // // Link all pools into a list for easier track later. // @@ -1092,7 +1109,9 @@ InitializePageTablePool ( ); ASSERT (IsModified == TRUE); - return TRUE; +Done: + mPageTablePoolLock = FALSE; + return IsModified; } /** -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 5/5] MdeModulePkg/Core: fix-up for changes introduced by freed-memory guard
> v2 changes: > a. Update IsHeapGuardEnabled() calling because its prototype change Two changes are fixed up: a. Prototype change of function IsHeapGuardEnabled() b. More memory map descriptors are introduced by new feature. MergeMemoryMap() is updated to merge freed-pages into adjacent memory descriptor to reduce the overall descriptor number. 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/Misc/MemoryProtection.c | 2 +- MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 16 +++- 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c index fa8f8fe91a..6298b67db1 100644 --- a/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c +++ b/MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c @@ -1250,7 +1250,7 @@ ApplyMemoryProtectionPolicy ( // Don't overwrite Guard pages, which should be the first and/or last page, // if any. // - if (IsHeapGuardEnabled ()) { + if (IsHeapGuardEnabled (GUARD_HEAP_TYPE_PAGE|GUARD_HEAP_TYPE_POOL)) { if (IsGuardPage (Memory)) { Memory += EFI_PAGE_SIZE; Length -= EFI_PAGE_SIZE; diff --git a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c index 05eb4f422b..f6595c90ed 100644 --- a/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c +++ b/MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c @@ -32,6 +32,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include #include "DxeMain.h" +#include "HeapGuard.h" #define PREVIOUS_MEMORY_DESCRIPTOR(MemoryDescriptor, Size) \ ((EFI_MEMORY_DESCRIPTOR *)((UINT8 *)(MemoryDescriptor) - (Size))) @@ -205,16 +206,13 @@ MergeMemoryMap ( NextMemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (MemoryMapEntry, DescriptorSize); do { - MemoryBlockLength = (UINT64) (EfiPagesToSize (MemoryMapEntry->NumberOfPages)); + MergeGuardPages (NewMemoryMapEntry, NextMemoryMapEntry->PhysicalStart); + MemoryBlockLength = (UINT64) (EfiPagesToSize (NewMemoryMapEntry->NumberOfPages)); if (((UINTN)NextMemoryMapEntry < (UINTN)MemoryMapEnd) && - (MemoryMapEntry->Type == NextMemoryMapEntry->Type) && - (MemoryMapEntry->Attribute == NextMemoryMapEntry->Attribute) && - ((MemoryMapEntry->PhysicalStart + MemoryBlockLength) == NextMemoryMapEntry->PhysicalStart)) { -MemoryMapEntry->NumberOfPages += NextMemoryMapEntry->NumberOfPages; -if (NewMemoryMapEntry != MemoryMapEntry) { - NewMemoryMapEntry->NumberOfPages += NextMemoryMapEntry->NumberOfPages; -} - + (NewMemoryMapEntry->Type == NextMemoryMapEntry->Type) && + (NewMemoryMapEntry->Attribute == NextMemoryMapEntry->Attribute) && + ((NewMemoryMapEntry->PhysicalStart + MemoryBlockLength) == NextMemoryMapEntry->PhysicalStart)) { +NewMemoryMapEntry->NumberOfPages += NextMemoryMapEntry->NumberOfPages; NextMemoryMapEntry = NEXT_MEMORY_DESCRIPTOR (NextMemoryMapEntry, DescriptorSize); continue; } else { -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 4/5] MdeModulePkg/Core: add freed-memory guard feature
> v2 changes: > a. Change prototype and implementation of IsHeapGuardEnabled() >to allow it to check freed-memory guard feature. > b. Drop IsUafEnabled() because of a. > c. Move the sanity check of freed-memory guard and heap guard >into HeapGuardCpuArchProtocolNotify() > d. Add GuardFreedPagesChecked() to avoid duplicate feature check > e. Coding style cleanup 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. 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 | 63 +- MdeModulePkg/Core/Dxe/Mem/Page.c | 41 +++- MdeModulePkg/Core/Dxe/Mem/Pool.c | 21 +- 4 files changed, 513 insertions(+), 21 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c index 663f969c0d..5666420a6d 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. + @param[in] Pages Number of freed pages. + + @return VOID. +**/ +VOID +MarkFreedPages ( + IN EFI_PHYSICAL_ADDRESS BaseAddress, + IN UINTNPages + ) +{ + SetGuardedMemoryBits (BaseAddress, Pages); +} + +/** + R
[edk2] [PATCH v2 2/5] UefiCpuPkg/CpuDxe: fix an infinite loop issue
> v2 changes: > a. Change the type of mPageTablePoolLock to be BOOLEAN. Related code >logic is also updated and refined. > b. Add non-stop mode for freed-memory guard feature The freed-memory guard feature will cause an infinite calling of InitializePageTablePool(). This is due to a fact that AllocateAlignedPages() is used to allocate page table pool memory. This function will most likely call gBS->FreePages to free unaligned pages and then cause another round of page attributes change, like below FreePages() <===| => SetMemoryAttributes() | => | => InitializePageTablePool() | => AllocateAlignedPages() | => FreePages() | The solution is add a global variable as a lock in page table pool allocation function and fail any other requests if it has not been done. This patch also add non-stop mode for freed-memory guard. Cc: Laszlo Ersek Cc: Star Zeng Cc: Michael D Kinney Cc: Jiewen Yao Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/CpuDxe/CpuDxe.h | 2 +- UefiCpuPkg/CpuDxe/CpuPageTable.c | 19 +-- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.h b/UefiCpuPkg/CpuDxe/CpuDxe.h index 064ea05bba..3183a3f7f4 100644 --- a/UefiCpuPkg/CpuDxe/CpuDxe.h +++ b/UefiCpuPkg/CpuDxe/CpuDxe.h @@ -58,7 +58,7 @@ ) #define HEAP_GUARD_NONSTOP_MODE \ -((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT6|BIT1|BIT0)) > BIT6) +((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT6|BIT4|BIT1|BIT0)) > BIT6) #define NULL_DETECTION_NONSTOP_MODE \ ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & (BIT6|BIT0)) > BIT6) diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c index 33e8ee2d2c..b7beaf935b 100644 --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c @@ -100,6 +100,7 @@ PAGE_ATTRIBUTE_TABLE mPageAttributeTable[] = { }; PAGE_TABLE_POOL *mPageTablePool = NULL; +BOOLEAN mPageTablePoolLock = FALSE; PAGE_TABLE_LIB_PAGING_CONTEXT mPagingContext; EFI_SMM_BASE2_PROTOCOL*mSmmBase2 = NULL; @@ -1046,6 +1047,16 @@ InitializePageTablePool ( VOID *Buffer; BOOLEAN IsModified; + // + // Do not allow re-entrance. + // + if (mPageTablePoolLock) { +return FALSE; + } + + mPageTablePoolLock = TRUE; + IsModified = FALSE; + // // Always reserve at least PAGE_TABLE_POOL_UNIT_PAGES, including one page for // header. @@ -1056,7 +1067,9 @@ InitializePageTablePool ( Buffer = AllocateAlignedPages (PoolPages, PAGE_TABLE_POOL_ALIGNMENT); if (Buffer == NULL) { DEBUG ((DEBUG_ERROR, "ERROR: Out of aligned pages\r\n")); -return FALSE; +goto Done; + } else { +DEBUG ((DEBUG_INFO, "Paging: added %ld pages to page table pool\r\n", (UINT64)PoolPages)); } // @@ -1092,7 +1105,9 @@ InitializePageTablePool ( ); ASSERT (IsModified == TRUE); - return TRUE; +Done: + mPageTablePoolLock = FALSE; + return IsModified; } /** -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 3/5] MdeModulePkg/Core: fix a lock issue in GCD memory map dump
> v2 changes: > a. Update implementation of CoreGetMemorySpaceMap() and >CoreGetIoSpaceMap() to avoid lock failure. Drop the code to >detect debug print level used to achieve the same effect. This issue is hidden in current code but exposed by introduction of freed-memory guard feature due to the fact that the feature will turn all pool allocation to page allocation. The solution is move the memory allocation in CoreGetMemorySpaceMap() and CoreGetIoSpaceMap() to be out of the GCD memory map lock. CoreDumpGcdMemorySpaceMap() => CoreGetMemorySpaceMap() => CoreAcquireGcdMemoryLock () * AllocatePool() => InternalAllocatePool() => CoreAllocatePool() => CoreAllocatePoolI() => CoreAllocatePoolPagesI() => CoreAllocatePoolPages() => FindFreePages() => PromoteMemoryResource() => CoreAcquireGcdMemoryLock() ** 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/Gcd/Gcd.c | 140 1 file changed, 86 insertions(+), 54 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c index d9c65a8045..133c3dcd87 100644 --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c @@ -1691,10 +1691,10 @@ CoreGetMemorySpaceMap ( OUT EFI_GCD_MEMORY_SPACE_DESCRIPTOR **MemorySpaceMap ) { - EFI_STATUS Status; LIST_ENTRY *Link; EFI_GCD_MAP_ENTRY*Entry; EFI_GCD_MEMORY_SPACE_DESCRIPTOR *Descriptor; + UINTNNumber; // // Make sure parameters are valid @@ -1706,38 +1706,54 @@ CoreGetMemorySpaceMap ( return EFI_INVALID_PARAMETER; } - CoreAcquireGcdMemoryLock (); - // // Count the number of descriptors // - *NumberOfDescriptors = CoreCountGcdMapEntry (); + Number = 0; + *NumberOfDescriptors = 0; + *MemorySpaceMap = NULL; + while (TRUE) { +// +// Allocate the MemorySpaceMap +// +if (Number != 0) { + if (*MemorySpaceMap != NULL) { +FreePool (*MemorySpaceMap); + } - // - // Allocate the MemorySpaceMap - // - *MemorySpaceMap = AllocatePool (*NumberOfDescriptors * sizeof (EFI_GCD_MEMORY_SPACE_DESCRIPTOR)); - if (*MemorySpaceMap == NULL) { -Status = EFI_OUT_OF_RESOURCES; -goto Done; - } + *MemorySpaceMap = AllocatePool (Number * sizeof(EFI_GCD_MEMORY_SPACE_DESCRIPTOR)); + if (*MemorySpaceMap == NULL) { +return EFI_OUT_OF_RESOURCES; + } - // - // Fill in the MemorySpaceMap - // - Descriptor = *MemorySpaceMap; - Link = mGcdMemorySpaceMap.ForwardLink; - while (Link != ) { -Entry = CR (Link, EFI_GCD_MAP_ENTRY, Link, EFI_GCD_MAP_SIGNATURE); -BuildMemoryDescriptor (Descriptor, Entry); -Descriptor++; -Link = Link->ForwardLink; + *NumberOfDescriptors = Number; +} + +CoreAcquireGcdMemoryLock (); + +Number = CoreCountGcdMapEntry (); +if (Number == *NumberOfDescriptors) { + // + // Fill in the MemorySpaceMap + // + Descriptor = *MemorySpaceMap; + Link = mGcdMemorySpaceMap.ForwardLink; + while (Link != && Number != 0) { +Entry = CR (Link, EFI_GCD_MAP_ENTRY, Link, EFI_GCD_MAP_SIGNATURE); +BuildMemoryDescriptor (Descriptor, Entry); +Descriptor++; +Number--; +Link = Link->ForwardLink; + } + + CoreReleaseGcdMemoryLock (); + break; +} + +CoreReleaseGcdMemoryLock (); } - Status = EFI_SUCCESS; -Done: - CoreReleaseGcdMemoryLock (); - return Status; + return EFI_SUCCESS; } @@ -1964,10 +1980,10 @@ CoreGetIoSpaceMap ( OUT EFI_GCD_IO_SPACE_DESCRIPTOR **IoSpaceMap ) { - EFI_STATUS Status; LIST_ENTRY *Link; EFI_GCD_MAP_ENTRY*Entry; EFI_GCD_IO_SPACE_DESCRIPTOR *Descriptor; + UINTNNumber; // // Make sure parameters are valid @@ -1979,38 +1995,54 @@ CoreGetIoSpaceMap ( return EFI_INVALID_PARAMETER; } - CoreAcquireGcdIoLock (); + Number = 0; + *NumberOfDescriptors = 0; + *IoSpaceMap = NULL; + while (TRUE) { +// +// Allocate the IoSpaceMap +// +if (Number != 0) { + if (*IoSpaceMap != NULL) { +FreePool (*IoSpaceMap); + } - // - // Count the number of descriptors - // - *NumberOfDescriptors = CoreCountGcdMapEntry (); + *IoSpaceMap = AllocatePool (Number * sizeof(EFI_GCD_IO_SPACE_DESCRIPTOR)); + if (*IoSpaceMap == NULL) { +return EFI_OUT_OF_RESOURCES; + } - // - // Allocate the IoSpaceMap - // - *IoSpaceMap = AllocatePool (*NumberOfDescriptors * sizeof (EFI_GCD_IO_SPACE_DESCRIPTOR)); - if (*IoSpaceMap == NULL) { -Status = EFI_OUT_OF_RESOURCES; -goto Done; - } + *NumberOfDescriptors = Number; +} - // - //
[edk2] [PATCH v2 1/5] MdeModulePkg/MdeModulePkg.dec: update PCD description for new feature
> v2 changes: > a. Drop PCD PcdUseAfterFreeDetectionPropertyMask. Use BIT4 of >PcdHeapGuardPropertyMask instead. Update related descriptions in both >dec and uni files. 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, this patch series add logic 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 freed. BIT4 of following PCD is used to enable the freed-memory guard feature. gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask Please note this feature cannot be enabled with heap guard at the same time. 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/MdeModulePkg.dec | 10 ++ MdeModulePkg/MdeModulePkg.uni | 6 +- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 6037504fa7..255b92ea67 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -955,6 +955,8 @@ # free pages for all of them. The page allocation for the type related to # cleared bits keeps the same as ususal. # + # This PCD is only valid if BIT0 and/or BIT2 are set in PcdHeapGuardPropertyMask. + # # Below is bit mask for this PCD: (Order is same as UEFI spec) # EfiReservedMemoryType 0x0001 # EfiLoaderCode 0x0002 @@ -984,6 +986,8 @@ # if there's enough free memory for all of them. The pool allocation for the # type related to cleared bits keeps the same as ususal. # + # This PCD is only valid if BIT1 and/or BIT3 are set in PcdHeapGuardPropertyMask. + # # Below is bit mask for this PCD: (Order is same as UEFI spec) # EfiReservedMemoryType 0x0001 # EfiLoaderCode 0x0002 @@ -1007,14 +1011,20 @@ gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType|0x0|UINT64|0x30001053 ## This mask is to control Heap Guard behavior. + # # Note that due to the limit of pool memory implementation and the alignment # requirement of UEFI spec, BIT7 is a try-best setting which cannot guarantee # that the returned pool is exactly adjacent to head guard page or tail guard # page. + # + # Note that UEFI freed-memory guard and pool/page guard cannot be enabled + # at the same time. + # # BIT0 - Enable UEFI page guard. # BIT1 - Enable UEFI pool guard. # BIT2 - Enable SMM page guard. # BIT3 - Enable SMM pool guard. + # BIT4 - Enable UEFI freed-memory guard (Use-After-Free memory detection). # BIT6 - Enable non-stop mode. # BIT7 - The direction of Guard Page for Pool Guard. # 0 - The returned pool is near the tail guard page. diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni index a6bcb627cf..e72b893509 100644 --- a/MdeModulePkg/MdeModulePkg.uni +++ b/MdeModulePkg/MdeModulePkg.uni @@ -1171,6 +1171,7 @@ " before and after corresponding type of pages allocated if there's enough\n" " free pages for all of them. The page allocation for the type related to\n" " cleared bits keeps the same as ususal.\n\n" + " This PCD is only valid if BIT0 and/or BIT2 are set in PcdHeapGuardPropertyMask.\n\n" " Below is bit mask for this PCD: (Order is same as UEFI spec)\n" " EfiReservedMemoryType 0x0001\n" " EfiLoaderCode 0x0002\n" @@ -1198,6 +1199,7 @@ &q
[edk2] [PATCH v2 0/5] Add freed-memory guard feature
> v2 changes: > a. Drop PCD PcdUseAfterFreeDetectionPropertyMask. Use BIT4 of >PcdHeapGuardPropertyMask instead. Add more descriptions about >the new usage in dec/uni file as well. > b. Use global of BOOLEAN other than EFI_LOCK to avoid reentrance >of calling InitializePageTablePool() > c. Update implementation of CoreGetMemorySpaceMap() and >CoreGetIoSpaceMap() to avoid lock failure. Drop the code to >detect debug print level used to achieve the same effect. > d. Change prototype and implementation of IsHeapGuardEnabled() >to allow it to check freed-memory guard feature. > e. Move the sanity check of freed-memory guard and heap guard >into HeapGuardCpuArchProtocolNotify() > f. Add GuardFreedPagesChecked() to avoid duplicate feature check > g. Split patch series into smaller patch files Freed-memory guard is a new feauture used to detect UAF (Use-After-Free) memory issue. Jian J Wang (5): MdeModulePkg/MdeModulePkg.dec: update PCD description for new feature UefiCpuPkg/CpuDxe: fix an infinite loop issue MdeModulePkg/Core: fix a lock issue in GCD memory map dump MdeModulePkg/Core: add freed-memory guard feature MdeModulePkg/Core: fix-up for changes introduced by freed-memory guard MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 140 + MdeModulePkg/Core/Dxe/Mem/HeapGuard.c | 409 +- MdeModulePkg/Core/Dxe/Mem/HeapGuard.h | 63 +++- MdeModulePkg/Core/Dxe/Mem/Page.c | 41 ++- MdeModulePkg/Core/Dxe/Mem/Pool.c | 21 +- MdeModulePkg/Core/Dxe/Misc/MemoryProtection.c | 2 +- MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 16 +- MdeModulePkg/MdeModulePkg.dec | 10 + MdeModulePkg/MdeModulePkg.uni | 6 +- UefiCpuPkg/CpuDxe/CpuDxe.h| 2 +- UefiCpuPkg/CpuDxe/CpuPageTable.c | 19 +- 11 files changed, 640 insertions(+), 89 deletions(-) -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 0/3] Add use-after-free memory detection
UAF (Use-After-Free) memory detection is new feature introduced to detect illegal access to memory which has been freed. Jian J Wang (3): MdeModulePkg/MdeModulePkg.dec: add new PCD for UAF detection feature UefiCpuPkg/CpuDxe: fix an infinite loop issue MdeModulePkg/Core: add use-after-free memory detection MdeModulePkg/Core/Dxe/DxeMain.h | 1 + MdeModulePkg/Core/Dxe/DxeMain.inf| 1 + MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 8 + MdeModulePkg/Core/Dxe/Mem/HeapGuard.c| 393 ++- MdeModulePkg/Core/Dxe/Mem/HeapGuard.h| 66 + MdeModulePkg/Core/Dxe/Mem/Page.c | 39 ++- MdeModulePkg/Core/Dxe/Mem/Pool.c | 21 +- MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 16 +- MdeModulePkg/MdeModulePkg.dec| 6 + UefiCpuPkg/CpuDxe/CpuPageTable.c | 12 + 10 files changed, 537 insertions(+), 26 deletions(-) -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 2/3] UefiCpuPkg/CpuDxe: fix an infinite loop issue
The UAF (Use-After-Free) memory detection feature will cause an infinite calling of InitializePageTablePool(). This is due to a fact that AllocateAlignedPages() is used to allocate page table pool memory. This function will most likely call gBS->FreePages to free unaligned pages and then cause another round of page attributes change, like below FreePages() <===| => SetMemoryAttributes() | => | => InitializePageTablePool() | => AllocateAlignedPages() | => FreePages() | The solution is add a lock in page table pool allocation function and fail any other requests if it has not been done. Cc: Laszlo Ersek Cc: Star Zeng Cc: Michael D Kinney Cc: Jiewen Yao Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/CpuDxe/CpuPageTable.c | 12 1 file changed, 12 insertions(+) diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c index 33e8ee2d2c..2145e623fa 100644 --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c @@ -100,6 +100,7 @@ PAGE_ATTRIBUTE_TABLE mPageAttributeTable[] = { }; PAGE_TABLE_POOL *mPageTablePool = NULL; +EFI_LOCK mPageTablePoolLock = EFI_INITIALIZE_LOCK_VARIABLE (TPL_NOTIFY); PAGE_TABLE_LIB_PAGING_CONTEXT mPagingContext; EFI_SMM_BASE2_PROTOCOL*mSmmBase2 = NULL; @@ -1045,6 +1046,12 @@ InitializePageTablePool ( { VOID *Buffer; BOOLEAN IsModified; + EFI_STATUSStatus; + + Status = EfiAcquireLockOrFail (); + if (EFI_ERROR (Status)) { +return FALSE; + } // // Always reserve at least PAGE_TABLE_POOL_UNIT_PAGES, including one page for @@ -1056,7 +1063,10 @@ InitializePageTablePool ( Buffer = AllocateAlignedPages (PoolPages, PAGE_TABLE_POOL_ALIGNMENT); if (Buffer == NULL) { DEBUG ((DEBUG_ERROR, "ERROR: Out of aligned pages\r\n")); +EfiReleaseLock (); return FALSE; + } else { +DEBUG ((DEBUG_INFO, "Paging: added %d pages to page table pool\r\n", PoolPages)); } // @@ -1092,6 +1102,8 @@ InitializePageTablePool ( ); ASSERT (IsModified == TRUE); + EfiReleaseLock (); + return TRUE; } -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 3/3] MdeModulePkg/Core: add use-after-free memory detection
UAF (Use-After-Free) memory detection is new feature introduced to detect illegal access to memory which has been freed. The principle behind is similar to heap guard feature, that is the core 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. To use this feature, one can simply set following PCD to 1 gEfiMdeModulePkgTokenSpaceGuid.PcdUseAfterFreeDetectionPropertyMask Please note this feature cannot be used with heap guard feature controlled by PCD gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask. 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/DxeMain.h | 1 + MdeModulePkg/Core/Dxe/DxeMain.inf| 1 + MdeModulePkg/Core/Dxe/Gcd/Gcd.c | 8 + MdeModulePkg/Core/Dxe/Mem/HeapGuard.c| 393 ++- MdeModulePkg/Core/Dxe/Mem/HeapGuard.h| 66 + MdeModulePkg/Core/Dxe/Mem/Page.c | 39 ++- MdeModulePkg/Core/Dxe/Mem/Pool.c | 21 +- MdeModulePkg/Core/Dxe/Misc/PropertiesTable.c | 16 +- 8 files changed, 519 insertions(+), 26 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/DxeMain.h b/MdeModulePkg/Core/Dxe/DxeMain.h index 2dec9da5e3..ae75cc5b25 100644 --- a/MdeModulePkg/Core/Dxe/DxeMain.h +++ b/MdeModulePkg/Core/Dxe/DxeMain.h @@ -92,6 +92,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include #include #include +#include // diff --git a/MdeModulePkg/Core/Dxe/DxeMain.inf b/MdeModulePkg/Core/Dxe/DxeMain.inf index 10375443c0..d91258c087 100644 --- a/MdeModulePkg/Core/Dxe/DxeMain.inf +++ b/MdeModulePkg/Core/Dxe/DxeMain.inf @@ -198,6 +198,7 @@ gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPoolType ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask ## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdUseAfterFreeDetectionPropertyMask ## CONSUMES # [Hob] # RESOURCE_DESCRIPTOR ## CONSUMES diff --git a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c index d9c65a8045..e43ec74010 100644 --- a/MdeModulePkg/Core/Dxe/Gcd/Gcd.c +++ b/MdeModulePkg/Core/Dxe/Gcd/Gcd.c @@ -160,6 +160,10 @@ CoreDumpGcdMemorySpaceMap ( EFI_GCD_MEMORY_SPACE_DESCRIPTOR *MemorySpaceMap; UINTNIndex; +if ((GetDebugPrintErrorLevel () & DEBUG_GCD) == 0) { + return; +} + Status = CoreGetMemorySpaceMap (, ); ASSERT (Status == EFI_SUCCESS && MemorySpaceMap != NULL); @@ -202,6 +206,10 @@ CoreDumpGcdIoSpaceMap ( EFI_GCD_IO_SPACE_DESCRIPTOR *IoSpaceMap; UINTNIndex; +if ((GetDebugPrintErrorLevel () & DEBUG_GCD) == 0) { + return; +} + Status = CoreGetIoSpaceMap (, ); ASSERT (Status == EFI_SUCCESS && IoSpaceMap != NULL); diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c index 663f969c0d..b120c04f8f 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 Use-After-Free memory detection to promote 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; @@ -1203,6 +1208,373 @@ SetAllGuardPages ( } } +/** + Check to see if the Use-After-Free (UAF) feature is enabled or not. + + @return TRUE/FALSE. +**/ +BOOLEAN +IsUafEnabled ( +
[edk2] [PATCH 1/3] MdeModulePkg/MdeModulePkg.dec: add new PCD for UAF detection feature
UAF (Use-After-Free) memory detection is new feature introduced to detect 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, this patch series add logic 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 freed. To use this feature, one can simply set following PCD to 1 gEfiMdeModulePkgTokenSpaceGuid.PcdUseAfterFreeDetectionPropertyMask Please note this feature cannot be used with heap guard feature controlled by PCD gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask. 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/MdeModulePkg.dec | 6 ++ 1 file changed, 6 insertions(+) diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 6037504fa7..83736cd761 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -1029,6 +1029,12 @@ # @Prompt Enable UEFI Stack Guard. gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard|FALSE|BOOLEAN|0x30001055 + ## This mask is to control Use-After-Free Memory Detection behavior. + # BIT0- Enable Use-After-Free memory detection for UEFI modules. + # BIT1..7 - Reserved for future uses. + # @Prompt The Use-After-Free Memory Detection feature mask + gEfiMdeModulePkgTokenSpaceGuid.PcdUseAfterFreeDetectionPropertyMask|0x0|UINT8|0x30001056 + [PcdsFixedAtBuild, PcdsPatchableInModule] ## Dynamic type PCD can be registered callback function for Pcd setting action. # PcdMaxPeiPcdCallBackNumberPerPcdEntry indicates the maximum number of callback function -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] UefiCpuPkg/CpuExceptionHandlerLib: always clear descriptor data in advance
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1237 Sometimes the memory will be contaminated by random data left in last boot (warm reset). The code should not assume the allocated memory is always filled with zero. This patch add code to clear data structure used for stack switch to prevent such problem from happening. Cc: Eric Dong Cc: Laszlo Ersek Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c | 3 +++ UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c | 3 +++ 2 files changed, 6 insertions(+) diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c index 031d0d35fa..eebd27a25d 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c @@ -214,6 +214,7 @@ ArchSetupExcpetionStack ( // TssBase = (UINTN)Tss; + TssDesc->Uint64 = 0; TssDesc->Bits.LimitLow = sizeof(IA32_TASK_STATE_SEGMENT) - 1; TssDesc->Bits.BaseLow= (UINT16)TssBase; TssDesc->Bits.BaseMid= (UINT8)(TssBase >> 16); @@ -238,6 +239,7 @@ ArchSetupExcpetionStack ( // TssBase = (UINTN)Tss; +TssDesc->Uint64 = 0; TssDesc->Bits.LimitLow = sizeof(IA32_TASK_STATE_SEGMENT) - 1; TssDesc->Bits.BaseLow = (UINT16)TssBase; TssDesc->Bits.BaseMid = (UINT8)(TssBase >> 16); @@ -255,6 +257,7 @@ ArchSetupExcpetionStack ( continue; } +SetMem (Tss, sizeof (IA32_TASK_STATE_SEGMENT), 0); Tss->EIP= (UINT32)(TemplateMap.ExceptionStart + Vector * TemplateMap.ExceptionStubHeaderSize); Tss->EFLAGS = 0x2; diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c index 93ecf5ae5a..6745bc77c0 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c @@ -219,6 +219,8 @@ ArchSetupExcpetionStack ( // TssBase = (UINTN)Tss; + TssDesc->Uint128.Uint64 = 0; + TssDesc->Uint128.Uint64_1= 0; TssDesc->Bits.LimitLow = sizeof(IA32_TASK_STATE_SEGMENT) - 1; TssDesc->Bits.BaseLow= (UINT16)TssBase; TssDesc->Bits.BaseMidl = (UINT8)(TssBase >> 16); @@ -231,6 +233,7 @@ ArchSetupExcpetionStack ( // // Fixup exception task descriptor and task-state segment // + SetMem (Tss, sizeof (IA32_TASK_STATE_SEGMENT), 0); StackTop = StackSwitchData->X64.KnownGoodStackTop - CPU_STACK_ALIGNMENT; StackTop = (UINTN)ALIGN_POINTER (StackTop, CPU_STACK_ALIGNMENT); IdtTable = StackSwitchData->X64.IdtTable; -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain support
This patch adds stack check support for MSFT toolchain, with compiler option /GS and /RTCs. This functionality is similar to the original ones supported by GCC toolchain. Usage example: This is a NULL library instance. Add it under a [LibraryClasses] section in dsc file to let it be built into all modules employed in a platform. [LibraryClasses] NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf Please note all not modules can be built against this library. Most of them are SEC type of modules, such as OvmfPkg/ResetVector/ResetVector.inf In this case, this library should not be added to a common [LibraryClasses] section but to specific ones, like [LibraryClasses.common.PEI_CORE/PEIM/...]. In addition, /GS and/or /RTCs should be added to compiler command line. This can be done by adding something like below under [BuildOptions] section in dsc file. [BuildOptions] MSFT:DEBUG_*_*_CC_FLAGS = /GS /GL- MSFT:DEBUG_*_*_CC_FLAGS = /RTCs /Od Note: /GL- is required for /GS, and /Od is required for /RTCs. Note: The flash layout might be needed to update to accommodate larger image size due to /Od is enforced. Pass tests: a. Overwrite a local buffer variable (in a 32-bit and 64-bit driver)and check if it's caught by new code (on both real platform and virtual platform) b. Boot Windows 10 and Ubuntu 18.04 on real platform with this lib built-in Cc: Michael D Kinney Cc: Liming Gao Cc: Jiewen Yao Cc: Andrew Fish Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- .../BaseStackCheckLib/BaseStackCheckLib.inf| 11 +- .../Library/BaseStackCheckLib/BaseStackCheckMsft.c | 221 + .../Library/BaseStackCheckLib/BaseStackCheckNull.c | 15 -- .../BaseStackCheckLib/Ia32/StackCheckStubAsm.nasm | 76 +++ .../BaseStackCheckLib/X64/StackCheckStubAsm.nasm | 54 + 5 files changed, 360 insertions(+), 17 deletions(-) create mode 100644 MdePkg/Library/BaseStackCheckLib/BaseStackCheckMsft.c delete mode 100644 MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c create mode 100644 MdePkg/Library/BaseStackCheckLib/Ia32/StackCheckStubAsm.nasm create mode 100644 MdePkg/Library/BaseStackCheckLib/X64/StackCheckStubAsm.nasm diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf index e280651b11..1c9e6710c6 100644 --- a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf @@ -4,6 +4,7 @@ # Stack Check Library # # Copyright (c) 2014, ARM Ltd. All rights reserved. +# Copyright (c) 2018, Intel Corporation. All rights reserved. # # This program and the accompanying materials # are licensed and made available under the terms and conditions of the BSD License @@ -26,13 +27,19 @@ # -# VALID_ARCHITECTURES = ARM AARCH64 +# VALID_ARCHITECTURES = ARM AARCH64 IA32 X64 # [Sources] BaseStackCheckGcc.c | GCC BaseStackCheckGcc.c | RVCT - BaseStackCheckNull.c | MSFT + BaseStackCheckMsft.c | MSFT + +[Sources.IA32] + Ia32/StackCheckStubAsm.nasm | MSFT + +[Sources.X64] + X64/StackCheckStubAsm.nasm | MSFT [Packages] MdePkg/MdePkg.dec diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckMsft.c b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckMsft.c new file mode 100644 index 00..951154f0cd --- /dev/null +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckMsft.c @@ -0,0 +1,221 @@ +/** @file + Base Stack Check library for MSFT toolchains compiler options: /GS, RTCs. + +Copyright (c) 2018, Intel Corporation. All rights reserved. +This program and the accompanying materials are licensed and made available under +the terms and conditions of the BSD License that accompanies this distribution. +The full text of the license may be found at +http://opensource.org/licenses/bsd-license.php. + +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON AN "AS IS" BASIS, +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. + +**/ + +#include + +#include +#include +#include + +// +// cookie value that is inserted by the MSFT compiler into the stack frame. +// +extern UINTN __security_cookie; + +// +// Data structure used by MSFT compiler to record local variable information. +// + +typedef struct _RTC_vardesc { + int Addr; + int Size; + char *Name; +} _RTC_vardesc; + +typedef struct _RTC_framedesc { + int VarCount; + _RTC_vardesc *Variables; +} _RTC_framedesc; + +#define RTC_STACK_CHECK_COOKIE 0x + +/** + Function called upon unexpected stack pointer change. + + @param Ip Instruction address where the check happened. + +**/ +VOID +__cdecl +_RTC_Failure ( + VOID*Ip + ) +{ + DEBUG ((DEBUG_ERROR, "\nSTACK FAULT: Suspicious stack pointer (IP:%p).\n\n", Ip)); + + // + // Generate a Breakpoint, DeadLoop, or NOP based
[edk2] [PATCH 1/2] Nt32Pkg/Nt32Pkg.dsc: override PCD default to avoid boot failure
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1215 This issue is caused by commit 5267926134d17e86672b84fd57b438f05ffa68e1. The reason is this commit changed the condition of building page table in DxeIpl. The code before it will only build page table for the sake of PcdSetNxForStack. This commit added PcdImageProtectionPolicy and PcdDxeNxMemoryProtectionPolicy into the logic. But the default value of PcdImageProtectionPolicy is 02, which means the DxeIpl should build page table. Due to the fact that Nt32Pkg doesn't support page table at all, this will cause exception on Windows OS. This patch solves this issue by setting PcdImageProtectionPolicy to 0 explicitly in Nt32Pkg.dsc. Cc: Ruiyu Ni Cc: Hao Wu Cc: Star Zeng Cc: Jiewen Yao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- Nt32Pkg/Nt32Pkg.dsc | 1 + 1 file changed, 1 insertion(+) diff --git a/Nt32Pkg/Nt32Pkg.dsc b/Nt32Pkg/Nt32Pkg.dsc index fcd833858d..58d9f8787d 100644 --- a/Nt32Pkg/Nt32Pkg.dsc +++ b/Nt32Pkg/Nt32Pkg.dsc @@ -265,6 +265,7 @@ gEfiMdeModulePkgTokenSpaceGuid.PcdVariableCollectStatistics|TRUE [PcdsFixedAtBuild] + gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x gEfiMdeModulePkgTokenSpaceGuid.PcdMaxSizeNonPopulateCapsule|0x0 gEfiMdeModulePkgTokenSpaceGuid.PcdMaxSizePopulateCapsule|0x0 gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8040 -- 2.19.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 2/2] EmulatorPkg/EmulatorPkg.dsc: override PCD default to avoid boot failure
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1215 This issue is caused by commit 5267926134d17e86672b84fd57b438f05ffa68e1. The reason is this commit changed the condition of building page table in DxeIpl. The code before it will only build page table for the sake of PcdSetNxForStack. This commit added PcdImageProtectionPolicy and PcdDxeNxMemoryProtectionPolicy into the logic. But the default value of PcdImageProtectionPolicy is 02, which means the DxeIpl should build page table. Due to the fact that EmulatorPkg doesn't support page table at all, this will cause exception on Windows OS. This patch solves this issue by setting PcdImageProtectionPolicy to 0 explicitly in EmulatorPkg.dsc. Cc: Jordan Justen Cc: Andrew Fish Cc: Ruiyu Ni Cc: Star Zeng Cc: Jiewen Yao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- EmulatorPkg/EmulatorPkg.dsc | 1 + 1 file changed, 1 insertion(+) diff --git a/EmulatorPkg/EmulatorPkg.dsc b/EmulatorPkg/EmulatorPkg.dsc index 78e9a3f1cc..c8c444fe1c 100644 --- a/EmulatorPkg/EmulatorPkg.dsc +++ b/EmulatorPkg/EmulatorPkg.dsc @@ -179,6 +179,7 @@ gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplBuildPageTables|FALSE [PcdsFixedAtBuild] + gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x gEfiMdeModulePkgTokenSpaceGuid.PcdResetOnMemoryTypeInformationChange|FALSE gEfiMdePkgTokenSpaceGuid.PcdDebugPrintErrorLevel|0x8040 gEfiMdePkgTokenSpaceGuid.PcdReportStatusCodePropertyMask|0x0f -- 2.19.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 0/2] Fix Nt32Pkg/EmulatorPkg boot failure
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1215 This issue is caused by commit 5267926134d17e86672b84fd57b438f05ffa68e1. Jian J Wang (2): Nt32Pkg/Nt32Pkg.dsc: override PCD default to avoid boot failure EmulatorPkg/EmulatorPkg.dsc: override PCD default to avoid boot failure EmulatorPkg/EmulatorPkg.dsc | 1 + Nt32Pkg/Nt32Pkg.dsc | 1 + 2 files changed, 2 insertions(+) -- 2.19.0.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 2/2] MdeModulePkg/DxeIpl: support more NX related PCDs
> v2 changes: >a. remove macros no longer needed >b. remove DEBUG and ASSERT in ToEnableExecuteDisableFeature() >c. change ToEnableExecuteDisableFeature to EnableNonExec BZ#1116: https://bugzilla.tianocore.org/show_bug.cgi?id=1116 Currently IA32_EFER.NXE is only set against PcdSetNxForStack. This confuses developers because following two other PCDs also need NXE to be set, but actually not. PcdDxeNxMemoryProtectionPolicy PcdImageProtectionPolicy This patch solves this issue by adding logic to enable IA32_EFER.NXE if any of those PCDs have anything enabled. Cc: Star Zeng Cc: Laszlo Ersek Cc: Ard Biesheuvel Cc: Ruiyu Ni Cc: Jiewen Yao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 2 ++ MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 4 ++-- MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 30 +++- MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h | 24 +++ 4 files changed, 57 insertions(+), 3 deletions(-) diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf index fd82657404..068e700074 100644 --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf @@ -117,6 +117,8 @@ [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64] gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack ## SOMETIMES_CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy ## SOMETIMES_CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy ## SOMETIMES_CONSUMES [Depex] gEfiPeiLoadFilePpiGuid AND gEfiPeiMasterBootModePpiGuid diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c index d28baa3615..ccd30f964b 100644 --- a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c +++ b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c @@ -245,7 +245,7 @@ ToBuildPageTable ( return TRUE; } - if (PcdGetBool (PcdSetNxForStack) && IsExecuteDisableBitAvailable ()) { + if (EnableNonExec ()) { return TRUE; } @@ -436,7 +436,7 @@ HandOffToDxeCore ( BuildPageTablesIa32Pae = ToBuildPageTable (); if (BuildPageTablesIa32Pae) { PageTables = Create4GPageTablesIa32Pae (BaseOfStack, STACK_SIZE); - if (IsExecuteDisableBitAvailable ()) { + if (EnableNonExec ()) { EnableExecuteDisableBit(); } } diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c index 496e219913..73b0f67c6b 100644 --- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c +++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c @@ -106,6 +106,31 @@ IsNullDetectionEnabled ( return ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT0) != 0); } +/** + Check if Execute Disable Bit (IA32_EFER.NXE) should be enabled or not. + + @retval TRUEIA32_EFER.NXE should be enabled. + @retval FALSE IA32_EFER.NXE should not be enabled. + +**/ +BOOLEAN +EnableNonExec ( + VOID + ) +{ + if (!IsExecuteDisableBitAvailable ()) { +return FALSE; + } + + // + // XD flag (BIT63) in page table entry is only valid if IA32_EFER.NXE is set. + // Features controlled by Following PCDs need this feature to be enabled. + // + return (PcdGetBool (PcdSetNxForStack) || + PcdGet64 (PcdDxeNxMemoryProtectionPolicy) != 0 || + PcdGet32 (PcdImageProtectionPolicy) != 0); +} + /** Enable Execute Disable Bit. @@ -755,7 +780,10 @@ CreateIdentityMappingPageTables ( // EnablePageTableProtection ((UINTN)PageMap, TRUE); - if (PcdGetBool (PcdSetNxForStack)) { + // + // Set IA32_EFER.NXE if necessary. + // + if (EnableNonExec ()) { EnableExecuteDisableBit (); } diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h index 85457ff937..09085312aa 100644 --- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h +++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h @@ -179,6 +179,30 @@ typedef struct { UINTN FreePages; } PAGE_TABLE_POOL; +/** + Check if Execute Disable Bit (IA32_EFER.NXE) should be enabled or not. + + @retval TRUEIA32_EFER.NXE should be enabled. + @retval FALSE IA32_EFER.NXE should not be enabled. + +**/ +BOOLEAN +EnableNonExec ( + VOID + ); + +/** + The function will check if Execute Disable Bit is available. + + @retval TRUE Execute Disable Bit is available. + @retval FALSE Execute Disable Bit is not available. + +**/ +BOOLEAN +IsExecuteDisableBitAvailable ( + VOID + ); + /** Enable Execute Disable Bit. -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 1/2] MdeModulePkg/MdeModulePkg.dec/.uni: clarify PCDs usage
> v2 changes: >Newly added patch to clarify PCDs usage. BZ#1116: https://bugzilla.tianocore.org/show_bug.cgi?id=1116 The usage of following PCDs described in MdeModulePkg.dec don't match the implementation exactly. This patch updates related description in both .dec and .uni files to avoid confusion in platform configuration. PcdSetNxForStack PcdImageProtectionPolicy PcdDxeNxMemoryProtectionPolicy The main change is at the statement on how to handle the FALSE or 0 setting value in those PCDs. Current statement says the implementation should unset or disable related features but in fact the related code just do nothing (leave it to AS-IS). That means the result might be disabled, or might be not. It depends on other features or platform policy. For example, if one don't want to enforce NX onto stack memory, he/she needs to set PcdSetNxForStack to FALSE as well as to clear BIT4 of PcdDxeNxMemoryProtectionPolicy. Cc: Star Zeng Cc: Laszlo Ersek Cc: Ard Biesheuvel Cc: Ruiyu Ni Cc: Jiewen Yao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/MdeModulePkg.dec | 20 +++- MdeModulePkg/MdeModulePkg.uni | 13 + 2 files changed, 24 insertions(+), 9 deletions(-) diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 74a699cbb7..6566b57fd4 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -1288,17 +1288,23 @@ ## Set image protection policy. The policy is bitwise. # If a bit is set, the image will be protected by DxeCore if it is aligned. # The code section becomes read-only, and the data section becomes non-executable. - # If a bit is clear, the image will not be protected. + # If a bit is clear, nothing will be done to image code/data sections. #BIT0 - Image from unknown device. #BIT1 - Image from firmware volume. + # + # Note: If a bit is cleared, the data section could be still non-executable if + # PcdDxeNxMemoryProtectionPolicy is enabled for EfiLoaderData, EfiBootServicesData + # and/or EfiRuntimeServicesData. + # # @Prompt Set image protection policy. # @ValidRange 0x8002 | 0x - 0x001F gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x0002|UINT32|0x1047 ## Set DXE memory protection policy. The policy is bitwise. # If a bit is set, memory regions of the associated type will be mapped - # non-executable. - # + # non-executable. + # If a bit is cleared, nothing will be done to associated type of memory. + # # Below is bit mask for this PCD: (Order is same as UEFI spec) # EfiReservedMemoryType 0x0001 # EfiLoaderCode 0x0002 @@ -1890,8 +1896,12 @@ # For the DxeIpl and the DxeCore are both X64, set NX for stack feature also require PcdDxeIplBuildPageTables be TRUE. # For the DxeIpl and the DxeCore are both IA32 (PcdDxeIplSwitchToLongMode is FALSE), set NX for stack feature also require # IA32 PAE is supported and Execute Disable Bit is available. - # TRUE - to set NX for stack. - # FALSE - Not to set NX for stack. + # + # Note: If this PCD is set to FALSE, NX could be still applied to stack due to PcdDxeNxMemoryProtectionPolicy enabled for + # EfiBootServicesData. + # + # TRUE - Set NX for stack. + # FALSE - Do nothing for stack. # @Prompt Set NX for stack. gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE|BOOLEAN|0x0001006f diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni index 080b8a62c0..61befdc1e4 100644 --- a/MdeModulePkg/MdeModulePkg.uni +++ b/MdeModulePkg/MdeModulePkg.uni @@ -345,8 +345,9 @@ "For the DxeIpl and the DxeCore are both X64, set NX for stack feature also require PcdDxeIplBuildPageTables be TRUE." "For the DxeIpl and the DxeCore are both IA32 (PcdDxeIplSwitchToLongMode is FALSE), set NX for stack feature also require" "IA32 PAE is supported and Execute Disable Bit is available." - "TRUE - to set NX for stack." - "FALSE - Not to set NX for stack." + "Note: If this PCD is set to FALSE, NX could be still applied to stack due to PcdDxeNxMemoryProtectionPolicy enabled for EfiBootServicesData." + "TRUE - Set NX for stack." +
[edk2] [PATCH v2 0/2] clarify NXE enabling logic
> v2 changes: >Incorporates review comments from Laszlo and Star. BZ#1116: https://bugzilla.tianocore.org/show_bug.cgi?id=1116 Test: a. try all related PCDs combinations and check the page table attributes b. boot to shell on real intel platform with valid PCD setting combinations (IA32/X64) c. boot to fedora26, ubuntu18.04, windows 7 and windows 10 on OVMF emulated platform (X64) Cc: Star Zeng Cc: Laszlo Ersek Cc: Ard Biesheuvel Cc: Ruiyu Ni Cc: Jiewen Yao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang Jian J Wang (2): MdeModulePkg/MdeModulePkg.dec/.uni: clarify PCDs usage MdeModulePkg/DxeIpl: support more NX related PCDs MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 2 ++ MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 4 ++-- MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 30 +++- MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h | 24 +++ MdeModulePkg/MdeModulePkg.dec| 20 MdeModulePkg/MdeModulePkg.uni| 13 ++ 6 files changed, 81 insertions(+), 12 deletions(-) -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] UefiCpuPkg/CpuMpPei: fix unsafe way to get stack pointer
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1186 This patch uses SetJump() to get the stack pointer from esp/rsp register to replace local variable way, which was marked by static code checker as an unsafe way. Cc: Dandan Bi Cc: Hao A Wu Cc: Eric Dong Cc: Laszlo Ersek Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/CpuMpPei/CpuMpPei.h | 8 UefiCpuPkg/CpuMpPei/CpuPaging.c | 9 +++-- 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.h b/UefiCpuPkg/CpuMpPei/CpuMpPei.h index d097a66aa8..fe61f5e3bc 100644 --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.h +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.h @@ -35,6 +35,14 @@ extern EFI_PEI_PPI_DESCRIPTOR mPeiCpuMpPpiDesc; +#if defined (MDE_CPU_IA32) +#define CPU_STACK_POINTER(Context) ((Context).Esp) +#elif defined (MDE_CPU_X64) +#define CPU_STACK_POINTER(Context) ((Context).Rsp) +#else +#error CPU type not supported! +#endif + /** This service retrieves the number of logical processor in the platform and the number of those logical processors that are enabled on this boot. diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c index c7e0822452..997c20c26e 100644 --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c @@ -517,9 +517,14 @@ GetStackBase ( IN OUT VOID *Buffer ) { - EFI_PHYSICAL_ADDRESSStackBase; + EFI_PHYSICAL_ADDRESS StackBase; + BASE_LIBRARY_JUMP_BUFFER Context; - StackBase = (EFI_PHYSICAL_ADDRESS)(UINTN) + // + // Retrieve stack pointer from current processor context. + // + SetJump (); + StackBase = (EFI_PHYSICAL_ADDRESS)CPU_STACK_POINTER (Context); StackBase += BASE_4KB; StackBase &= ~((EFI_PHYSICAL_ADDRESS)BASE_4KB - 1); StackBase -= PcdGet32(PcdCpuApStackSize); -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2] UefiCpuPkg/CpuMpPei: fix vs2012 build error
> v2 changes: > Incorpate Laszlo's reivew comments > a. change title > b. drop unrelated changes > c. remove error message REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1166 Visual Studio 2012 will complain uninitialized variable, StackBase, in the CpuPaging.c. This patch adds code to init it to zero and ASSERT check against 0. This is enough since uninit case will only happen during retrieving stack memory via gEfiHobMemoryAllocStackGuid. But this HOB will always be created in advance. Cc: Dandan Bi Cc: Hao A Wu Cc: Eric Dong Cc: Laszlo Ersek Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/CpuMpPei/CpuPaging.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c index bcb942a8e5..c7e0822452 100644 --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c @@ -554,6 +554,8 @@ SetupStackGuardPage ( MpInitLibGetNumberOfProcessors(, NULL); MpInitLibWhoAmI (); for (Index = 0; Index < NumberOfProcessors; ++Index) { +StackBase = 0; + if (Index == Bsp) { Hob.Raw = GetHobList (); while ((Hob.Raw = GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, Hob.Raw)) != NULL) { @@ -570,6 +572,7 @@ SetupStackGuardPage ( // MpInitLibStartupThisAP(GetStackBase, Index, NULL, 0, (VOID *), NULL); } +ASSERT (StackBase != 0); // // Set Guard page at stack base address. // -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 2/2] UefiCpuPkg/PiSmmCpuDxeSmm: move InitSmmS3Cr3() into else block
> v2: >Move InitSmmS3Cr3() into 'else' block because it needs to access >SmmS3ResumeState BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1165 InitSmmS3Cr3 () will update SmmS3ResumeState so moving the calling of it into else block to keep the logic consistency. Cc: Star Zeng Cc: Benjamin You Cc: Eric Dong Cc: Laszlo Ersek Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 10 +- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c index 0f6b6ef587..9198fa4e4c 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c @@ -750,12 +750,12 @@ InitSmmS3ResumeState ( if (sizeof (UINTN) == sizeof (UINT32)) { SmmS3ResumeState->Signature = SMM_S3_RESUME_SMM_32; } - } - // - // Patch SmmS3ResumeState->SmmS3Cr3 - // - InitSmmS3Cr3 (); +// +// Patch SmmS3ResumeState->SmmS3Cr3 +// +InitSmmS3Cr3 (); + } // // Allocate safe memory in ACPI NVS for AP to execute hlt loop in -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 1/2] UefiCpuPkg/PiSmmCpuDxeSmm: add message for S3 config error
> v2 > a. Refine the error message > b. Use CpuDeadLoop to replace ASSERT(FALSE) for release build BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1165 HOB gEfiAcpiVariableGuid is a must have data for S3 resume if PcdAcpiS3Enable is set to TRUE. Current code in CpuS3.c doesn't embody this strong binding between them. An error message and CpuDeadLoop are added in this patch to warn platform developer about it. Cc: Star Zeng Cc: Benjamin You Cc: Eric Dong Cc: Laszlo Ersek Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 8 +++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c index abd8a5a07b..0f6b6ef587 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c @@ -714,7 +714,13 @@ InitSmmS3ResumeState ( } GuidHob = GetFirstGuidHob (); - if (GuidHob != NULL) { + if (GuidHob == NULL) { +DEBUG ((DEBUG_ERROR, +"ERROR:%a(): HOB(gEfiAcpiVariableGuid=%g) needed by S3 resume doesn't exist!\n", +__FUNCTION__, +)); +CpuDeadLoop (); + } else { SmramDescriptor = (EFI_SMRAM_DESCRIPTOR *) GET_GUID_HOB_DATA (GuidHob); DEBUG ((EFI_D_INFO, "SMM S3 SMRAM Structure = %x\n", SmramDescriptor)); -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH V2 0/2] UefiCpuPkg/PiSmmCpuDxeSmm: add message for S3 config error
> v2 > a. refine the error message to be more useful > b. improve the code logic BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1165 HOB gEfiAcpiVariableGuid is a must have data for S3 resume if PcdAcpiS3Enable is set to TRUE. Current code in CpuS3.c doesn't embody this strong binding between them. This patch series try to fix this problem by a useful message. Jian J Wang (2): UefiCpuPkg/PiSmmCpuDxeSmm: add message for S3 config error UefiCpuPkg/PiSmmCpuDxeSmm: move InitSmmS3Cr3() into else block UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 18 -- 1 file changed, 12 insertions(+), 6 deletions(-) -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] MdeModulePkg/DxeIpl: support more NX related PCDs
BZ#1116: https://bugzilla.tianocore.org/show_bug.cgi?id=1116 Currently IA32_EFER.NXE is only set against PcdSetNxForStack. This confuses developers because following two other PCDs also need NXE to be set, but actually not. PcdDxeNxMemoryProtectionPolicy PcdImageProtectionPolicy This patch solves this issue by adding logic to enable IA32_EFER.NXE if any of those PCDs have anything enabled. Due to the fact that NX memory type of stack (enabled by PcdSetNxForStack) and image data section (enabled by PcdImageProtectionPolicy) are also part of PcdDxeNxMemoryProtectionPolicy, this patch also add more checks to warn (ASSERT) users any unreasonable setting combinations. For example, PcdSetNxForStack == FALSE && (PcdDxeNxMemoryProtectionPolicy & (1 < Cc: Laszlo Ersek Cc: Ard Biesheuvel Cc: Ruiyu Ni Cc: Jiewen Yao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 2 + MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 4 +- MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 55 +++- MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.h | 33 ++ 4 files changed, 91 insertions(+), 3 deletions(-) diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf index fd82657404..068e700074 100644 --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf @@ -117,6 +117,8 @@ [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64] gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack ## SOMETIMES_CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy ## SOMETIMES_CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy ## SOMETIMES_CONSUMES [Depex] gEfiPeiLoadFilePpiGuid AND gEfiPeiMasterBootModePpiGuid diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c index d28baa3615..9a97205ef8 100644 --- a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c +++ b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c @@ -245,7 +245,7 @@ ToBuildPageTable ( return TRUE; } - if (PcdGetBool (PcdSetNxForStack) && IsExecuteDisableBitAvailable ()) { + if (ToEnableExecuteDisableFeature ()) { return TRUE; } @@ -436,7 +436,7 @@ HandOffToDxeCore ( BuildPageTablesIa32Pae = ToBuildPageTable (); if (BuildPageTablesIa32Pae) { PageTables = Create4GPageTablesIa32Pae (BaseOfStack, STACK_SIZE); - if (IsExecuteDisableBitAvailable ()) { + if (ToEnableExecuteDisableFeature ()) { EnableExecuteDisableBit(); } } diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c index 496e219913..253fe84223 100644 --- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c +++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c @@ -106,6 +106,56 @@ IsNullDetectionEnabled ( return ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT0) != 0); } +/** + Check if Execute Disable Bit (IA32_EFER.NXE) should be enabled or not. + + @retval TRUEIA32_EFER.NXE should be enabled. + @retval FALSE IA32_EFER.NXE should not be enabled. + +**/ +BOOLEAN +ToEnableExecuteDisableFeature ( + VOID + ) +{ + if (!IsExecuteDisableBitAvailable ()) { +return FALSE; + } + + // + // Normally stack is type of EfiBootServicesData. Disabling NX for stack + // but enabling NX for EfiBootServicesData doesn't make any sense. + // + if (PcdGetBool (PcdSetNxForStack) == FALSE && + (PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & STACK_MEMORY_TYPE) != 0) { +DEBUG ((DEBUG_ERROR, +"ERROR: NX for stack is disabled but NX for its memory type is enabled!\r\n")); +ASSERT(!(PcdGetBool (PcdSetNxForStack) == FALSE && + (PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & STACK_MEMORY_TYPE) != 0)); + } + + // + // Image data section could be type of EfiLoaderData, EfiBootServicesData + // or EfiRuntimeServicesData. Disabling NX for image data but enabling NX + // for any those memory types doesn't make any sense. + // + if (PcdGet32 (PcdImageProtectionPolicy) == 0 && + (PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & IMAGE_DATA_MEMORY_TYPE) != 0) { +DEBUG ((DEBUG_ERROR, +"ERROR: NX for image data is disabled but NX for its memory type(s) is enabled!\r\n")); +ASSERT (!(PcdGet32 (PcdImageProtectionPolicy) == 0 && + (PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & IMAGE_DATA_MEMORY_TYPE) != 0)); + } + + // + // XD flag (BIT63) in page table entry is only valid if IA32_EFER.NXE is set. + // Features controlled by Following PCDs need this feature to be enabled. + // + return (PcdGetBool (PcdSetNxForStack) || + PcdGet64 (PcdDxeNxMemoryProtectionPolicy) != 0 || + PcdGet32
[edk2] [PATCH 5/5] MdeModulePkg: expire PcdSetNxForStack
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1116 Since the stack memory is allocated as EfiBootServicesData, its NX protection can be covered by BIT4 of PcdDxeNxMemoryProtectionPolicy. To avoid confusing in setting related PCDs, PcdSetNxForStack will be expired. Set BIT4 of PcdDxeNxMemoryProtectionPolicy if NX protection is needed for stack. Cc: Star Zeng Cc: Laszlo Ersek Cc: Ard Biesheuvel Cc: Ruiyu Ni Cc: Jiewen Yao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/MdeModulePkg.dec | 10 +- MdeModulePkg/MdeModulePkg.uni | 10 +- 2 files changed, 2 insertions(+), 18 deletions(-) diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 74a699cbb7..b1f208909c 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -1320,6 +1320,7 @@ # # NOTE: User must NOT set NX protection for EfiLoaderCode / EfiBootServicesCode / EfiRuntimeServicesCode. # User MUST set the same NX protection for EfiBootServicesData and EfiConventionalMemory. + # Stack is allocated as type of EfiBootServicesData. Enable NX protection for it will also enable NX protection for stack. # # e.g. 0x7FD5 can be used for all memory except Code. # e.g. 0x7BD4 can be used for all memory except Code and ACPINVS/Reserved. @@ -1886,15 +1887,6 @@ # @Prompt Default Creator Revision for ACPI table creation. gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiDefaultCreatorRevision|0x0113|UINT32|0x30001038 - ## Indicates if to set NX for stack. - # For the DxeIpl and the DxeCore are both X64, set NX for stack feature also require PcdDxeIplBuildPageTables be TRUE. - # For the DxeIpl and the DxeCore are both IA32 (PcdDxeIplSwitchToLongMode is FALSE), set NX for stack feature also require - # IA32 PAE is supported and Execute Disable Bit is available. - # TRUE - to set NX for stack. - # FALSE - Not to set NX for stack. - # @Prompt Set NX for stack. - gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE|BOOLEAN|0x0001006f - ## This PCD specifies the PCI-based SD/MMC host controller mmio base address. # Define the mmio base address of the pci-based SD/MMC host controller. If there are multiple SD/MMC # host controllers, their mmio base addresses are calculated one by one from this base address. diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni index 080b8a62c0..6b26b21f00 100644 --- a/MdeModulePkg/MdeModulePkg.uni +++ b/MdeModulePkg/MdeModulePkg.uni @@ -339,15 +339,6 @@ #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSerialRegisterStride_HELP #language en-US "The number of bytes between registers in serial device. The default is 1 byte." -#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSetNxForStack_PROMPT #language en-US "Set NX for stack" - -#string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdSetNxForStack_HELP #language en-US "Indicates if to set NX for stack." - "For the DxeIpl and the DxeCore are both X64, set NX for stack feature also require PcdDxeIplBuildPageTables be TRUE." - "For the DxeIpl and the DxeCore are both IA32 (PcdDxeIplSwitchToLongMode is FALSE), set NX for stack feature also require" - "IA32 PAE is supported and Execute Disable Bit is available." - "TRUE - to set NX for stack." - "FALSE - Not to set NX for stack." - #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdAcpiS3Enable_PROMPT #language en-US "ACPI S3 Enable" #string STR_gEfiMdeModulePkgTokenSpaceGuid_PcdAcpiS3Enable_HELP #language en-US "Indicates if ACPI S3 will be enabled." @@ -1129,6 +1120,7 @@ "\n" "NOTE: User must NOT set NX protection for EfiLoaderCode / EfiBootServicesCode / EfiRuntimeServicesCode. \n" "User MUST set the same NX protection for EfiBootServicesData and EfiConventionalMemory. \n" + "Stack is allocated as type of EfiBootServicesData. Enable NX protection for it will also enable NX protection for stack. \n"
[edk2] [PATCH 4/5] ArmVirtPkg/ArmVirt.dsc.inc: expire the use of PcdSetNxForStack
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1116 Since PcdSetNxForStack is expired, remove all references. The default setting of PcdDxeNxMemoryProtectionPolicy has covered PcdSetNxForStack. Cc: Laszlo Ersek Cc: Star Zeng Cc: Ard Biesheuvel Cc: Ruiyu Ni Cc: Jiewen Yao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- ArmVirtPkg/ArmVirt.dsc.inc | 5 - 1 file changed, 5 deletions(-) diff --git a/ArmVirtPkg/ArmVirt.dsc.inc b/ArmVirtPkg/ArmVirt.dsc.inc index 70a0ac4d78..d87ae5a32d 100644 --- a/ArmVirtPkg/ArmVirt.dsc.inc +++ b/ArmVirtPkg/ArmVirt.dsc.inc @@ -383,11 +383,6 @@ # gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC0007FD1 - # - # Enable the non-executable DXE stack. (This gets set up by DxeIpl) - # - gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE - [PcdsFixedAtBuild.ARM] gEmbeddedTokenSpaceGuid.PcdPrePiCpuMemorySize|40 -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 0/5] expire the use of PcdSetNxForStack
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1116 Since the stack memory is allocated as EfiBootServicesData, its NX protection can be covered by BIT4 of PcdDxeNxMemoryProtectionPolicy. To avoid confusing in setting related PCDs, PcdSetNxForStack will be expired. Instead, If BIT4 of PcdDxeNxMemoryProtectionPolicy is set, the DxeIpl will set NX bit in page table entries mapping the stack memory. Jian J Wang (5): MdeModulePkg/DxeIplPeim: expire the use of PcdSetNxForStack OvmfPkg/PlatformPei: expire the use of PcdSetNxForStack OvmfPkg: expire the use of PcdSetNxForStack ArmVirtPkg/ArmVirt.dsc.inc: expire the use of PcdSetNxForStack MdeModulePkg: expire PcdSetNxForStack ArmVirtPkg/ArmVirt.dsc.inc | 5 - MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c | 6 +- MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 2 +- MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 3 ++- MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c | 2 +- MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 14 +++--- MdeModulePkg/MdeModulePkg.dec| 10 +- MdeModulePkg/MdeModulePkg.uni| 10 +- OvmfPkg/OvmfPkgIa32.dsc | 1 - OvmfPkg/OvmfPkgIa32X64.dsc | 1 - OvmfPkg/OvmfPkgX64.dsc | 1 - OvmfPkg/PlatformPei/Platform.c | 1 - OvmfPkg/PlatformPei/PlatformPei.inf | 1 - 13 files changed, 22 insertions(+), 35 deletions(-) -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 1/5] MdeModulePkg/DxeIplPeim: expire the use of PcdSetNxForStack
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1116 Since the stack memory is allocated as EfiBootServicesData, its NX protection can be covered by BIT4 of PcdDxeNxMemoryProtectionPolicy. To avoid confusing in setting related PCDs, PcdSetNxForStack will be expired. Instead, If BIT4 of PcdDxeNxMemoryProtectionPolicy is set, the DxeIpl will set NX bit in page table entries mapping the stack memory. Cc: Star Zeng Cc: Laszlo Ersek Cc: Ard Biesheuvel Cc: Ruiyu Ni Cc: Jiewen Yao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c | 6 +- MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf | 2 +- MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 3 ++- MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c | 2 +- MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c | 14 +++--- 5 files changed, 20 insertions(+), 7 deletions(-) diff --git a/MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c index 176d361f19..d44b845b76 100644 --- a/MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c +++ b/MdeModulePkg/Core/DxeIplPeim/Arm/DxeLoadFunc.c @@ -45,7 +45,11 @@ HandOffToDxeCore ( BaseOfStack = AllocatePages (EFI_SIZE_TO_PAGES (STACK_SIZE)); ASSERT (BaseOfStack != NULL); - if (PcdGetBool (PcdSetNxForStack)) { + // + // Set stack to non-executable, if EfiBootServicesData type of memory is + // set for NX protection. + // + if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & BIT4) != 0) { Status = ArmSetMemoryRegionNoExec ((UINTN)BaseOfStack, STACK_SIZE); ASSERT_EFI_ERROR (Status); } diff --git a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf index fd82657404..44b6ea84ff 100644 --- a/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf +++ b/MdeModulePkg/Core/DxeIplPeim/DxeIpl.inf @@ -116,7 +116,7 @@ gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ## CONSUMES [Pcd.IA32,Pcd.X64,Pcd.ARM,Pcd.AARCH64] - gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack ## SOMETIMES_CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy ## SOMETIMES_CONSUMES [Depex] gEfiPeiLoadFilePpiGuid AND gEfiPeiMasterBootModePpiGuid diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c index d28baa3615..854078e6dd 100644 --- a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c +++ b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c @@ -245,7 +245,8 @@ ToBuildPageTable ( return TRUE; } - if (PcdGetBool (PcdSetNxForStack) && IsExecuteDisableBitAvailable ()) { + if (PcdGet64 (PcdDxeNxMemoryProtectionPolicy) != 0 && + IsExecuteDisableBitAvailable ()) { return TRUE; } diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c index 81efcfe93d..eb53bc9417 100644 --- a/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c +++ b/MdeModulePkg/Core/DxeIplPeim/X64/DxeLoadFunc.c @@ -94,7 +94,7 @@ HandOffToDxeCore ( // Set NX for stack feature also require PcdDxeIplBuildPageTables be TRUE // for the DxeIpl and the DxeCore are both X64. // -ASSERT (PcdGetBool (PcdSetNxForStack) == FALSE); +ASSERT (PcdGet64 (PcdDxeNxMemoryProtectionPolicy) == 0); ASSERT (PcdGetBool (PcdCpuStackGuard) == FALSE); } diff --git a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c index 496e219913..27e9d6955d 100644 --- a/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c +++ b/MdeModulePkg/Core/DxeIplPeim/X64/VirtualMemory.c @@ -152,7 +152,11 @@ ToSplitPageTable ( } } - if (PcdGetBool (PcdSetNxForStack)) { + // + // Set stack to non-executable, if EfiBootServicesData type of memory is + // set for NX protection. + // + if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & BIT4) != 0) { if ((Address < StackBase + StackSize) && ((Address + Size) > StackBase)) { return TRUE; } @@ -314,7 +318,11 @@ Split2MPageTo4K ( PageTableEntry->Bits.Present = 1; } -if (PcdGetBool (PcdSetNxForStack) +// +// Set stack to non-executable, if EfiBootServicesData type of memory is +// set for NX protection. +// +if ((PcdGet64 (PcdDxeNxMemoryProtectionPolicy) & BIT4) != 0 && (PhysicalAddress4K >= StackBase) && (PhysicalAddress4K < StackBase + StackSize)) { // @@ -755,7 +763,7 @@ CreateIdentityMappingPageTables ( // EnablePageTableProtection ((UINTN)PageMap, TRUE); - if (PcdGetBool (PcdSetNxForStack)) { + if (PcdGet64 (PcdDxeNxMemoryProtectionPolicy) != 0) { EnableExecuteDisableBit (); } -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 2/5] OvmfPkg/PlatformPei: expire the use of PcdSetNxForStack
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1116 Since PcdSetNxForStack is expired, remove related config code. PcdDxeNxMemoryProtectionPolicy cannot be used as dynamic PCD. There's no need to add it in code to replace PcdSetNxForStack. Cc: Laszlo Ersek Cc: Star Zeng Cc: Jordan Justen Cc: Ard Biesheuvel Cc: Ruiyu Ni Cc: Jiewen Yao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- OvmfPkg/PlatformPei/Platform.c | 1 - OvmfPkg/PlatformPei/PlatformPei.inf | 1 - 2 files changed, 2 deletions(-) diff --git a/OvmfPkg/PlatformPei/Platform.c b/OvmfPkg/PlatformPei/Platform.c index 5a78668126..6627d236e0 100644 --- a/OvmfPkg/PlatformPei/Platform.c +++ b/OvmfPkg/PlatformPei/Platform.c @@ -340,7 +340,6 @@ NoexecDxeInitialization ( ) { UPDATE_BOOLEAN_PCD_FROM_FW_CFG (PcdPropertiesTableEnable); - UPDATE_BOOLEAN_PCD_FROM_FW_CFG (PcdSetNxForStack); } VOID diff --git a/OvmfPkg/PlatformPei/PlatformPei.inf b/OvmfPkg/PlatformPei/PlatformPei.inf index 9c5ad9961c..ef5dcb7693 100644 --- a/OvmfPkg/PlatformPei/PlatformPei.inf +++ b/OvmfPkg/PlatformPei/PlatformPei.inf @@ -96,7 +96,6 @@ gEfiMdeModulePkgTokenSpaceGuid.PcdPciDisableBusEnumeration gEfiMdeModulePkgTokenSpaceGuid.PcdDxeIplSwitchToLongMode gEfiMdeModulePkgTokenSpaceGuid.PcdUse1GPageTable - gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable gEfiMdeModulePkgTokenSpaceGuid.PcdAcpiS3Enable gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 3/5] OvmfPkg: expire the use of PcdSetNxForStack
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1116 Since PcdSetNxForStack is expired, remove related references in dsc file. Cc: Laszlo Ersek Cc: Star Zeng Cc: Jordan Justen Cc: Ard Biesheuvel Cc: Ruiyu Ni Cc: Jiewen Yao Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- OvmfPkg/OvmfPkgIa32.dsc| 1 - OvmfPkg/OvmfPkgIa32X64.dsc | 1 - OvmfPkg/OvmfPkgX64.dsc | 1 - 3 files changed, 3 deletions(-) diff --git a/OvmfPkg/OvmfPkgIa32.dsc b/OvmfPkg/OvmfPkgIa32.dsc index 9f07e75050..1eaefbfd6e 100644 --- a/OvmfPkg/OvmfPkgIa32.dsc +++ b/OvmfPkg/OvmfPkgIa32.dsc @@ -559,7 +559,6 @@ gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE # Noexec settings for DXE. - gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable|FALSE # UefiCpuPkg PCDs related to initial AP bringup and general AP management. diff --git a/OvmfPkg/OvmfPkgIa32X64.dsc b/OvmfPkg/OvmfPkgIa32X64.dsc index a4eaeb808c..6f0424c862 100644 --- a/OvmfPkg/OvmfPkgIa32X64.dsc +++ b/OvmfPkg/OvmfPkgIa32X64.dsc @@ -567,7 +567,6 @@ gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE # Noexec settings for DXE. - gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable|FALSE # UefiCpuPkg PCDs related to initial AP bringup and general AP management. diff --git a/OvmfPkg/OvmfPkgX64.dsc b/OvmfPkg/OvmfPkgX64.dsc index aa3efc5e73..da6b1293e3 100644 --- a/OvmfPkg/OvmfPkgX64.dsc +++ b/OvmfPkg/OvmfPkgX64.dsc @@ -566,7 +566,6 @@ gUefiOvmfPkgTokenSpaceGuid.PcdQemuSmbiosValidated|FALSE # Noexec settings for DXE. - gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|FALSE gEfiMdeModulePkgTokenSpaceGuid.PcdPropertiesTableEnable|FALSE # UefiCpuPkg PCDs related to initial AP bringup and general AP management. -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] UefiCpuPkg/CpuMpPei: suppress compiler complaining
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1166 Cc: Dandan Bi Cc: Hao A Wu Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/CpuMpPei/CpuPaging.c | 23 --- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/UefiCpuPkg/CpuMpPei/CpuPaging.c b/UefiCpuPkg/CpuMpPei/CpuPaging.c index bcb942a8e5..a63421a1af 100644 --- a/UefiCpuPkg/CpuMpPei/CpuPaging.c +++ b/UefiCpuPkg/CpuMpPei/CpuPaging.c @@ -517,7 +517,7 @@ GetStackBase ( IN OUT VOID *Buffer ) { - EFI_PHYSICAL_ADDRESSStackBase; + volatile EFI_PHYSICAL_ADDRESS StackBase; StackBase = (EFI_PHYSICAL_ADDRESS)(UINTN) StackBase += BASE_4KB; @@ -554,6 +554,8 @@ SetupStackGuardPage ( MpInitLibGetNumberOfProcessors(, NULL); MpInitLibWhoAmI (); for (Index = 0; Index < NumberOfProcessors; ++Index) { +StackBase = 0; + if (Index == Bsp) { Hob.Raw = GetHobList (); while ((Hob.Raw = GetNextHob (EFI_HOB_TYPE_MEMORY_ALLOCATION, Hob.Raw)) != NULL) { @@ -570,12 +572,19 @@ SetupStackGuardPage ( // MpInitLibStartupThisAP(GetStackBase, Index, NULL, 0, (VOID *), NULL); } -// -// Set Guard page at stack base address. -// -ConvertMemoryPageAttributes(StackBase, EFI_PAGE_SIZE, 0); -DEBUG ((DEBUG_INFO, "Stack Guard set at %lx [cpu%lu]!\n", -(UINT64)StackBase, (UINT64)Index)); + +if (StackBase == 0) { + DEBUG ((DEBUG_ERROR, "Stack base address was not found for [cpu%lu]!\n", + (UINT64)Index)); + ASSERT(StackBase != 0); +} else { + // + // Set Guard page at stack base address. + // + ConvertMemoryPageAttributes(StackBase, EFI_PAGE_SIZE, 0); + DEBUG ((DEBUG_INFO, "Stack Guard set at %lx [cpu%lu]!\n", + (UINT64)StackBase, (UINT64)Index)); +} } // -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] UefiCpuPkg/PiSmmCpuDxeSmm: add message for S3 config error
BZ#: https://bugzilla.tianocore.org/show_bug.cgi?id=1165 HOB gEfiAcpiVariableGuid is a must have data for S3 resume if PcdAcpiS3Enable is set to TRUE. Current code in CpuS3.c doesn't embody this strong binding between them. An error message and ASSERT is added by this patch to warn platform developer about it. Cc: Star Zeng Cc: Benjamin You Cc: Eric Dong Cc: Laszlo Ersek Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c index abd8a5a07b..f371667c44 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/CpuS3.c @@ -744,6 +744,9 @@ InitSmmS3ResumeState ( if (sizeof (UINTN) == sizeof (UINT32)) { SmmS3ResumeState->Signature = SMM_S3_RESUME_SMM_32; } + } else { +DEBUG ((DEBUG_ERROR, "ERROR: HOB(gEfiAcpiVariableGuid) needed by S3 resume doesn't exist!\n")); +ASSERT (FALSE); } // -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] UefiCpuPkg/CpuDxe: fix an incorrect bit-wise operation
The left operand is 64-bit but right operand could be 32-bit. A typecast is a must because of '~' op before it. Cc: Hao A Wu Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/CpuDxe/CpuPageTable.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c index ef6e080a07..0a980b9753 100644 --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c @@ -1181,7 +1181,7 @@ DebugExceptionHandler ( for (PFEntry = 0; PFEntry < mPFEntryCount[CpuIndex]; PFEntry++) { if (mLastPFEntryPointer[CpuIndex][PFEntry] != NULL) { - *mLastPFEntryPointer[CpuIndex][PFEntry] &= ~IA32_PG_P; + *mLastPFEntryPointer[CpuIndex][PFEntry] &= ~(UINT64)IA32_PG_P; } } -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] UefiCpuPkg/CpuDxe: fix ECC reported issues
There're two parameters which have different name in comment and prototype. Cc: Dandan Bi Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/CpuDxe/CpuDxe.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.h b/UefiCpuPkg/CpuDxe/CpuDxe.h index 7d65e39e90..064ea05bba 100644 --- a/UefiCpuPkg/CpuDxe/CpuDxe.h +++ b/UefiCpuPkg/CpuDxe/CpuDxe.h @@ -291,7 +291,7 @@ RefreshGcdMemoryAttributesFromPaging ( VOID EFIAPI DebugExceptionHandler ( - IN EFI_EXCEPTION_TYPE InterruptType, + IN EFI_EXCEPTION_TYPE ExceptionType, IN EFI_SYSTEM_CONTEXT SystemContext ); @@ -307,7 +307,7 @@ DebugExceptionHandler ( VOID EFIAPI PageFaultExceptionHandler ( - IN EFI_EXCEPTION_TYPE InterruptType, + IN EFI_EXCEPTION_TYPE ExceptionType, IN EFI_SYSTEM_CONTEXT SystemContext ); -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 1/4] MdeModulePkg/DxeIpl: disable paging before creating new page table
PEI Stack Guard needs to enable paging before DxeIpl. This might cause #GP in the transition from 32-bit PEI to 64-bit DXE due to the code trying to write CR3 register with PML4 page table while the processor is enabled with PAE paging. Simply disabling paging before updating CR3 can solve this conflict. There's no such issue for 64-bit PEI so this change applies only to 32-bit code. Cc: Star Zeng Cc: Ruiyu Ni Cc: Jiewen Yao Cc: "Ware, Ryan R" Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c | 10 ++ 1 file changed, 10 insertions(+) diff --git a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c index 8a939b6c24..d28baa3615 100644 --- a/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c +++ b/MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c @@ -325,6 +325,11 @@ HandOffToDxeCore ( PERF_EVENT_SIGNAL_END (gEndOfPeiSignalPpi.Guid); ASSERT_EFI_ERROR (Status); +// +// Paging might be already enabled. To avoid conflict configuration, +// disable paging first anyway. +// +AsmWriteCr0 (AsmReadCr0 () & (~BIT31)); AsmWriteCr3 (PageTables); // @@ -445,6 +450,11 @@ HandOffToDxeCore ( ASSERT_EFI_ERROR (Status); if (BuildPageTablesIa32Pae) { + // + // Paging might be already enabled. To avoid conflict configuration, + // disable paging first anyway. + // + AsmWriteCr0 (AsmReadCr0 () & (~BIT31)); AsmWriteCr3 (PageTables); // // Set Physical Address Extension (bit 5 of CR4). -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 4/4] UefiCpuPkg/CpuMpPei: support stack guard feature
This feature is the same as Stack Guard enabled in driver CpuDxe but applies to PEI phase. Due to the specialty in PEI module dispatching, this driver is changed to do the actual initialization in notify callback of event gEfiPeiMemoryDiscoveredPpiGuid. This can let the stack guard apply to as most PEI drivers as possible. To let Stack Guard work, some simple page table management code are introduced to setup Guard page at base of stack for each processor. Cc: Eric Dong Cc: Laszlo Ersek Cc: Ruiyu Ni Cc: Jiewen Yao Cc: Star Zeng Cc: "Ware, Ryan R" Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/CpuMpPei/CpuMpPei.c | 269 - UefiCpuPkg/CpuMpPei/CpuMpPei.h | 14 + UefiCpuPkg/CpuMpPei/CpuMpPei.inf | 11 +- UefiCpuPkg/CpuMpPei/CpuPaging.c | 637 +++ 4 files changed, 916 insertions(+), 15 deletions(-) create mode 100644 UefiCpuPkg/CpuMpPei/CpuPaging.c diff --git a/UefiCpuPkg/CpuMpPei/CpuMpPei.c b/UefiCpuPkg/CpuMpPei/CpuMpPei.c index 7c94d5a6d7..e3762daf39 100644 --- a/UefiCpuPkg/CpuMpPei/CpuMpPei.c +++ b/UefiCpuPkg/CpuMpPei/CpuMpPei.c @@ -409,25 +409,225 @@ PeiWhoAmI ( } /** - The Entry point of the MP CPU PEIM. + Get GDT register value. - This function will wakeup APs and collect CPU AP count and install the - Mp Service Ppi. + This function is mainly for AP purpose because AP may have different GDT + table than BSP. - @param FileHandleHandle of the file being invoked. - @param PeiServices Describes the list of possible PEI Services. + @param[in,out] Buffer The pointer to private data buffer. - @retval EFI_SUCCESS MpServicePpi is installed successfully. +**/ +VOID +EFIAPI +GetGdtr ( + IN OUT VOID *Buffer + ) +{ + AsmReadGdtr ((IA32_DESCRIPTOR *)Buffer); +} + +/** + Initializes CPU exceptions handlers for the sake of stack switch requirement. + + This function is a wrapper of InitializeCpuExceptionHandlersEx. It's mainly + for the sake of AP's init because of EFI_AP_PROCEDURE API requirement. + + @param[in,out] Buffer The pointer to private data buffer. **/ -EFI_STATUS +VOID EFIAPI -CpuMpPeimInit ( - IN EFI_PEI_FILE_HANDLE FileHandle, +InitializeExceptionStackSwitchHandlers ( + IN OUT VOID *Buffer + ) +{ + CPU_EXCEPTION_INIT_DATA *EssData; + IA32_DESCRIPTOR Idtr; + EFI_STATUSStatus; + + EssData = Buffer; + // + // We don't plan to replace IDT table with a new one, but we should not assume + // the AP's IDT is the same as BSP's IDT either. + // + AsmReadIdtr (); + EssData->Ia32.IdtTable = (VOID *)Idtr.Base; + EssData->Ia32.IdtTableSize = Idtr.Limit + 1; + Status = InitializeCpuExceptionHandlersEx (NULL, EssData); + ASSERT_EFI_ERROR (Status); +} + +/** + Initializes MP exceptions handlers for the sake of stack switch requirement. + + This function will allocate required resources required to setup stack switch + and pass them through CPU_EXCEPTION_INIT_DATA to each logic processor. + +**/ +VOID +InitializeMpExceptionStackSwitchHandlers ( + VOID + ) +{ + EFI_STATUS Status; + UINTN Index; + UINTN Bsp; + UINTN ExceptionNumber; + UINTN OldGdtSize; + UINTN NewGdtSize; + UINTN NewStackSize; + IA32_DESCRIPTOR Gdtr; + CPU_EXCEPTION_INIT_DATA EssData; + UINT8 *GdtBuffer; + UINT8 *StackTop; + UINTN NumberOfProcessors; + + if (!PcdGetBool (PcdCpuStackGuard)) { +return; + } + + MpInitLibGetNumberOfProcessors(, NULL); + MpInitLibWhoAmI (); + + ExceptionNumber = FixedPcdGetSize (PcdCpuStackSwitchExceptionList); + NewStackSize = FixedPcdGet32 (PcdCpuKnownGoodStackSize) * ExceptionNumber; + + Status = PeiServicesAllocatePool ( + NewStackSize * NumberOfProcessors, + (VOID **) + ); + ASSERT(StackTop != NULL); + if (EFI_ERROR (Status)) { +ASSERT_EFI_ERROR (Status); +return; + } + StackTop += NewStackSize * NumberOfProcessors; + + // + // The default exception handlers must have been initialized. Let's just skip + // it in this method. + // + EssData.Ia32.Revision = CPU_EXCEPTION_INIT_DATA_REV; + EssData.Ia32.InitDefaultHandlers = FALSE; + + EssData.Ia32.StackSwitchExceptions = FixedPcdGetPtr(PcdCpuStackSwitchExceptionList); + EssData.Ia32.StackSwitchExceptionNumber = ExceptionNumber; + EssData.Ia32.KnownGoodStackSize = FixedPcdGet32(PcdCpuKnownGoodStackSize); + + // + // Initialize Gdtr to suppress incorrect compiler/analyzer warnings. + // + Gdtr.Base = 0; + Gdtr.Limit = 0; + for (Index = 0; Index < NumberOfProcessors; ++Index) { +// +// To support stack switch, we need to re-constr
[edk2] [PATCH 2/4] UefiCpuPkg/CpuExceptionHandlerLib: support stack switch for PEI exceptions
Stack Guard needs to setup stack switch capability to allow exception handler to be called with good stack if stack overflow is detected. This patch update InitializeCpuExceptionHandlersEx() to allow pass extra initialization data used to setup exception stack switch for specified exceptions. Cc: Eric Dong Cc: Laszlo Ersek Cc: Ruiyu Ni Cc: Jiewen Yao Cc: Star Zeng Cc: "Ware, Ryan R" Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- .../CpuExceptionHandlerLib/PeiCpuException.c | 27 +- .../PeiCpuExceptionHandlerLib.inf | 4 2 files changed, 30 insertions(+), 1 deletion(-) diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c index 6f271983f2..5687c98c0d 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuException.c @@ -17,6 +17,7 @@ WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, EITHER EXPRESS OR IMPLIED. #include #include #include +#include CONST UINTNmDoFarReturnFlag = 0; @@ -208,5 +209,29 @@ InitializeCpuExceptionHandlersEx ( IN CPU_EXCEPTION_INIT_DATA*InitData OPTIONAL ) { - return InitializeCpuExceptionHandlers (VectorInfo); + EFI_STATUSStatus; + + // + // To avoid repeat initialization of default handlers, the caller should pass + // an extended init data with InitDefaultHandlers set to FALSE. There's no + // need to call this method to just initialize default handlers. Call non-ex + // version instead; or this method must be implemented as a simple wrapper of + // non-ex version of it, if this version has to be called. + // + if (InitData == NULL || InitData->Ia32.InitDefaultHandlers) { +Status = InitializeCpuExceptionHandlers (VectorInfo); + } else { +Status = EFI_SUCCESS; + } + + if (!EFI_ERROR (Status)) { +// +// Initializing stack switch is only necessary for Stack Guard functionality. +// +if (PcdGetBool (PcdCpuStackGuard) && InitData != NULL) { + Status = ArchSetupExcpetionStack (InitData); +} + } + + return Status; } diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf index 783260e39a..0ea44b3052 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf @@ -60,3 +60,7 @@ HobLib MemoryAllocationLib SynchronizationLib + +[Pcd] + gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard + -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 0/4] Add PEI Stack Guard feature
This patch series try to add PEI Stack Guard feature. Please refer to following trackers for details. The machanism behind this feature is the same as Stack Guard for UEFI drivers, and similiar implementation is also employed. REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1126 https://bugzilla.tianocore.org/show_bug.cgi?id=1137 Jian J Wang (4): MdeModulePkg/DxeIpl: disable paging before creating new page table UefiCpuPkg/CpuExceptionHandlerLib: support stack switch for PEI exceptions UefiCpuPkg/MpInitLib: fix register restore issue in AP wakeup UefiCpuPkg/CpuMpPei: support stack guard feature MdeModulePkg/Core/DxeIplPeim/Ia32/DxeLoadFunc.c| 10 + UefiCpuPkg/CpuMpPei/CpuMpPei.c | 269 - UefiCpuPkg/CpuMpPei/CpuMpPei.h | 14 + UefiCpuPkg/CpuMpPei/CpuMpPei.inf | 11 +- UefiCpuPkg/CpuMpPei/CpuPaging.c| 637 + .../CpuExceptionHandlerLib/PeiCpuException.c | 27 +- .../PeiCpuExceptionHandlerLib.inf | 4 + UefiCpuPkg/Library/MpInitLib/MpLib.c | 8 +- 8 files changed, 962 insertions(+), 18 deletions(-) create mode 100644 UefiCpuPkg/CpuMpPei/CpuPaging.c -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 3/4] UefiCpuPkg/MpInitLib: fix register restore issue in AP wakeup
The conflict issues are introduced by Stack Guard feature enabled for PEI. The first is CR0 which should be restored after CR3 and CR4. Another is TR which should not be passed from BSP to AP during init phase. Cc: Eric Dong Cc: Laszlo Ersek Cc: Ruiyu Ni Cc: Jiewen Yao Cc: Star Zeng Cc: "Ware, Ryan R" Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/Library/MpInitLib/MpLib.c | 8 ++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/UefiCpuPkg/Library/MpInitLib/MpLib.c b/UefiCpuPkg/Library/MpInitLib/MpLib.c index 377876643f..709fbc1575 100644 --- a/UefiCpuPkg/Library/MpInitLib/MpLib.c +++ b/UefiCpuPkg/Library/MpInitLib/MpLib.c @@ -217,9 +217,9 @@ RestoreVolatileRegisters ( CPUID_VERSION_INFO_EDXVersionInfoEdx; IA32_TSS_DESCRIPTOR *Tss; - AsmWriteCr0 (VolatileRegisters->Cr0); AsmWriteCr3 (VolatileRegisters->Cr3); AsmWriteCr4 (VolatileRegisters->Cr4); + AsmWriteCr0 (VolatileRegisters->Cr0); if (IsRestoreDr) { AsmCpuid (CPUID_VERSION_INFO, NULL, NULL, NULL, ); @@ -1558,7 +1558,7 @@ MpInitLibInitialize ( ApLoopMode = GetApLoopMode (); // - // Save BSP's Control registers for APs + // Save BSP's Control registers for APs. // SaveVolatileRegisters (); @@ -1656,6 +1656,10 @@ MpInitLibInitialize ( // CopyMem ((VOID *)ApIdtBase, (VOID *)VolatileRegisters.Idtr.Base, VolatileRegisters.Idtr.Limit + 1); VolatileRegisters.Idtr.Base = ApIdtBase; + // + // Don't pass BSP's TR to APs to avoid AP init failure. + // + VolatileRegisters.Tr = 0; CopyMem (>CpuData[0].VolatileRegisters, , sizeof (VolatileRegisters)); // // Set BSP basic information -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] MdeModulePkg/MdeModulePkg.uni: update PCD string per dec file change
PcdNullPointerDetectionPropertyMask and PcdHeapGuardPropertyMask have been updated in dec file to reflect a new feature (non-stop mode). This patch updates corresponding unicode strings in uni file to match those changes. Cc: Star Zeng Cc: Eric Dong Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/MdeModulePkg.uni | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/MdeModulePkg/MdeModulePkg.uni b/MdeModulePkg/MdeModulePkg.uni index 080b8a62c0..8807af801e 100644 --- a/MdeModulePkg/MdeModulePkg.uni +++ b/MdeModulePkg/MdeModulePkg.uni @@ -1146,7 +1146,8 @@ " If enabled, accessing NULL address in UEFI or SMM code can be caught.\n\n" " BIT0- Enable NULL pointer detection for UEFI.\n" " BIT1- Enable NULL pointer detection for SMM.\n" - " BIT2..6 - Reserved for future uses.\n" + " BIT2..5 - Reserved for future uses.\n" + " BIT6- Enable non-stop mode.\n" " BIT7- Disable NULL pointer detection just after EndOfDxe." " This is a workaround for those unsolvable NULL access issues in" " OptionROM, boot loader, etc. It can also help to avoid unnecessary" @@ -1225,6 +1226,7 @@ " BIT1 - Enable UEFI pool guard.\n" " BIT2 - Enable SMM page guard.\n" " BIT3 - Enable SMM pool guard.\n" + " BIT6 - Enable non-stop mode.\n" " BIT7 - The direction of Guard Page for Pool Guard.\n" " 0 - The returned pool is near the tail guard page.\n" " 1 - The returned pool is near the head guard page." -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] UefiCpuPkg/CpuDxe: change level of DEBUG message
It's reported the debug message in CpuDxe driver is quite annoying in boot and shell, and slow down the boot process. To solve this issue, this patch changes the DEBUG_INFO to DEBUG_VERBOSE. On a typical Intel real platform, at least 16s boot time can be saved. Cc: Eric Dong Cc: Laszlo Ersek Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/CpuDxe/CpuDxe.c | 2 +- UefiCpuPkg/CpuDxe/CpuPageTable.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c index cfd4c415ae..76661cbcd9 100644 --- a/UefiCpuPkg/CpuDxe/CpuDxe.c +++ b/UefiCpuPkg/CpuDxe/CpuDxe.c @@ -404,7 +404,7 @@ CpuSetMemoryAttributes ( // to avoid unnecessary computing. // if (mIsFlushingGCD) { -DEBUG((DEBUG_INFO, " Flushing GCD\n")); +DEBUG((DEBUG_VERBOSE, " Flushing GCD\n")); return EFI_SUCCESS; } diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c index df021798c0..609df58e3a 100644 --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c @@ -528,7 +528,7 @@ SplitPage ( ASSERT (SplitAttribute == Page4K); if (SplitAttribute == Page4K) { NewPageEntry = AllocatePagesFunc (1); - DEBUG ((DEBUG_INFO, "Split - 0x%x\n", NewPageEntry)); + DEBUG ((DEBUG_VERBOSE, "Split - 0x%x\n", NewPageEntry)); if (NewPageEntry == NULL) { return RETURN_OUT_OF_RESOURCES; } @@ -549,7 +549,7 @@ SplitPage ( ASSERT (SplitAttribute == Page2M || SplitAttribute == Page4K); if ((SplitAttribute == Page2M || SplitAttribute == Page4K)) { NewPageEntry = AllocatePagesFunc (1); - DEBUG ((DEBUG_INFO, "Split - 0x%x\n", NewPageEntry)); + DEBUG ((DEBUG_VERBOSE, "Split - 0x%x\n", NewPageEntry)); if (NewPageEntry == NULL) { return RETURN_OUT_OF_RESOURCES; } -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 0/4] Support non-stop mode in heap guard and null detection
> v2 changes: >fix GCC build error Background: Heap Guard and NULL Pointer Detection are very useful features to detect code flaw in EDK II. If an issue is detected, #PF exception will be triggered and the BIOS will enter into dead loop, which is the default behavior of exception handling. From QA perspective, this default behavior will block them to collect all tests result in reasonable time. Solution: This patch series update CpuDxe, PiSmmCpuDxeSmm and CpuExceptionHandlerLib to allow the code to continue execution after #PF. The mechanism behind it is the same as SMM Profile feature, in which a special #PF handler is registered to set the page causing #PF to be 'present' and setup single steop trap, then return the control back to the instruction accessing that page. Once the instruction is re-executed, a #DB is triggered and a special handler for it will be called to reset the page back to 'not-present'. Usage: The non-stop mode is enabled/disabled by BIT6 of following PCDs gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask The default setting is 'disable'. BZ Tracker: https://bugzilla.tianocore.org/show_bug.cgi?id=1095 OS Boot Validation: Platform: OVMF OS (x64): Fedora 26, Ubuntu 18.04, Windows 10, Windows 7 Jian J Wang (4): MdeModulePkg/MdeModulePkg.dec: add new settings for PCDs UefiCpuPkg/CpuExceptionHandlerLib: Setup single step in #PF handler UefiCpuPkg/CpuDxe: implement non-stop mode for uefi UefiCpuPkg/PiSmmCpuDxeSmm: implement non-stop mode for SMM MdeModulePkg/MdeModulePkg.dec | 4 +- UefiCpuPkg/CpuDxe/CpuDxe.h | 39 +++ UefiCpuPkg/CpuDxe/CpuDxe.inf | 3 + UefiCpuPkg/CpuDxe/CpuMp.c | 34 ++- UefiCpuPkg/CpuDxe/CpuPageTable.c | 271 + .../Ia32/ExceptionHandlerAsm.nasm | 7 + .../Ia32/ExceptionTssEntryAsm.nasm | 4 +- .../X64/ExceptionHandlerAsm.nasm | 4 + UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 43 ++-- UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm | 3 +- UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 58 - UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h | 15 ++ UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h | 6 + UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c| 43 ++-- 14 files changed, 493 insertions(+), 41 deletions(-) -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 1/4] MdeModulePkg/MdeModulePkg.dec: add new settings for PCDs
> v2 changes: >n/a BIT6 of PcdHeapGuardPropertyMask is used to enable/disable non-stop mode of Heap Guard feature. It applies to both UEFI and SMM heap guard, if any of them is enabled. BIT6 of PcdNullPointerDetectionPropertyMask is used to enable/disable non-stop mode of NULL Pointer Detection feature. It applies to both UEFI and SMM NULL Pointer Detection, if any of them is enabled. Cc: Eric Dong Cc: Laszlo Ersek Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/MdeModulePkg.dec | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 6a6d9660ed..451115030c 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -932,7 +932,8 @@ # If enabled, accessing NULL address in UEFI or SMM code can be caught. #BIT0- Enable NULL pointer detection for UEFI. #BIT1- Enable NULL pointer detection for SMM. - #BIT2..6 - Reserved for future uses. + #BIT2..5 - Reserved for future uses. + #BIT6- Enable non-stop mode. #BIT7- Disable NULL pointer detection just after EndOfDxe. # This is a workaround for those unsolvable NULL access issues in # OptionROM, boot loader, etc. It can also help to avoid unnecessary @@ -1014,6 +1015,7 @@ # BIT1 - Enable UEFI pool guard. # BIT2 - Enable SMM page guard. # BIT3 - Enable SMM pool guard. + # BIT6 - Enable non-stop mode. # BIT7 - The direction of Guard Page for Pool Guard. # 0 - The returned pool is near the tail guard page. # 1 - The returned pool is near the head guard page. -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 3/4] UefiCpuPkg/CpuDxe: implement non-stop mode for uefi
> v2 changes: >n/a Same as SMM profile feature, a special #PF is used to set page attribute to 'present' and a special #DB handler to reset it back to 'not-present', right after the instruction causing #PF got executed. Since the new #PF handler won't enter into dead-loop, the instruction which caused the #PF will get chance to re-execute with accessible pages. The exception message will still be printed out on debug console so that the developer/QA can find that there's potential heap overflow or null pointer access occurred. Cc: Eric Dong Cc: Laszlo Ersek Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/CpuDxe/CpuDxe.h | 39 ++ UefiCpuPkg/CpuDxe/CpuDxe.inf | 3 + UefiCpuPkg/CpuDxe/CpuMp.c| 34 - UefiCpuPkg/CpuDxe/CpuPageTable.c | 271 +++ 4 files changed, 341 insertions(+), 6 deletions(-) diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.h b/UefiCpuPkg/CpuDxe/CpuDxe.h index 540f5f2dbf..7d65e39e90 100644 --- a/UefiCpuPkg/CpuDxe/CpuDxe.h +++ b/UefiCpuPkg/CpuDxe/CpuDxe.h @@ -57,6 +57,12 @@ EFI_MEMORY_RO\ ) +#define HEAP_GUARD_NONSTOP_MODE \ +((PcdGet8 (PcdHeapGuardPropertyMask) & (BIT6|BIT1|BIT0)) > BIT6) + +#define NULL_DETECTION_NONSTOP_MODE \ +((PcdGet8 (PcdNullPointerDetectionPropertyMask) & (BIT6|BIT0)) > BIT6) + /** Flush CPU data cache. If the instruction cache is fully coherent with all DMA operations then function can just return EFI_SUCCESS. @@ -273,7 +279,40 @@ RefreshGcdMemoryAttributesFromPaging ( VOID ); +/** + Special handler for #DB exception, which will restore the page attributes + (not-present). It should work with #PF handler which will set pages to + 'present'. + + @param ExceptionType Exception type. + @param SystemContext Pointer to EFI_SYSTEM_CONTEXT. + +**/ +VOID +EFIAPI +DebugExceptionHandler ( + IN EFI_EXCEPTION_TYPE InterruptType, + IN EFI_SYSTEM_CONTEXT SystemContext + ); + +/** + Special handler for #PF exception, which will set the pages which caused + #PF to be 'present'. The attribute of those pages should be restored in + the subsequent #DB handler. + + @param ExceptionType Exception type. + @param SystemContext Pointer to EFI_SYSTEM_CONTEXT. + +**/ +VOID +EFIAPI +PageFaultExceptionHandler ( + IN EFI_EXCEPTION_TYPE InterruptType, + IN EFI_SYSTEM_CONTEXT SystemContext + ); + extern BOOLEAN mIsAllocatingPageTable; +extern UINTN mNumberOfProcessors; #endif diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.inf b/UefiCpuPkg/CpuDxe/CpuDxe.inf index 6a199b72f7..97a381b046 100644 --- a/UefiCpuPkg/CpuDxe/CpuDxe.inf +++ b/UefiCpuPkg/CpuDxe/CpuDxe.inf @@ -46,6 +46,7 @@ ReportStatusCodeLib MpInitLib TimerLib + PeCoffGetEntryPointLib [Sources] CpuDxe.c @@ -79,6 +80,8 @@ [Pcd] gEfiMdeModulePkgTokenSpaceGuid.PcdPteMemoryEncryptionAddressOrMask## CONSUMES gEfiMdeModulePkgTokenSpaceGuid.PcdCpuStackGuard ## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask ## CONSUMES + gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask## CONSUMES gUefiCpuPkgTokenSpaceGuid.PcdCpuStackSwitchExceptionList ## CONSUMES gUefiCpuPkgTokenSpaceGuid.PcdCpuKnownGoodStackSize## CONSUMES diff --git a/UefiCpuPkg/CpuDxe/CpuMp.c b/UefiCpuPkg/CpuDxe/CpuMp.c index 82145e7624..f8489eb1c3 100644 --- a/UefiCpuPkg/CpuDxe/CpuMp.c +++ b/UefiCpuPkg/CpuDxe/CpuMp.c @@ -673,10 +673,6 @@ InitializeMpExceptionStackSwitchHandlers ( UINT8 *GdtBuffer; UINT8 *StackTop; - if (!PcdGetBool (PcdCpuStackGuard)) { -return; - } - ExceptionNumber = FixedPcdGetSize (PcdCpuStackSwitchExceptionList); NewStackSize = FixedPcdGet32 (PcdCpuKnownGoodStackSize) * ExceptionNumber; @@ -790,6 +786,32 @@ InitializeMpExceptionStackSwitchHandlers ( } } +/** + Initializes MP exceptions handlers for special features, such as Heap Guard + and Stack Guard. +**/ +VOID +InitializeMpExceptionHandlers ( + VOID + ) +{ + // + // Enable non-stop mode for #PF triggered by Heap Guard or NULL Pointer + // Detection. + // + if (HEAP_GUARD_NONSTOP_MODE || NULL_DETECTION_NONSTOP_MODE) { +RegisterCpuInterruptHandler(EXCEPT_IA32_DEBUG, DebugExceptionHandler); +RegisterCpuInterruptHandler(EXCEPT_IA32_PAGE_FAULT, PageFaultExceptionHandler); + } + + // + // Setup stack switch for Stack Guard feature. + // + if (PcdGetBool (PcdCpuStackGuard)) { +InitializeMpExceptionStackSwitchHandlers(); + } +} + /** Initialize Multi-processor support. @@ -814,9 +836,9 @@ InitializeMpSupport ( DEBUG ((DEBUG_INFO, "Detect CPU count: %d\n", mNumberOfProcessors)); // - // Initialize exception stack switch handlers for each log
[edk2] [PATCH v2 2/4] UefiCpuPkg/CpuExceptionHandlerLib: Setup single step in #PF handler
> v2 changes: >n/a Once the #PF handler has set the page to be 'present', there should be a way to reset it to 'not-present'. 'TF' bit in EFLAGS can be used for this purpose. 'TF' bit will be set in interrupted function context so that it can be triggered once the cpu control returns back to the instruction causing #PF and re-execute it. This is an necessary step to implement non-stop mode for Heap Guard and NULL Pointer Detection feature. Cc: Eric Dong Cc: Laszlo Ersek Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- .../Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm | 7 +++ .../Library/CpuExceptionHandlerLib/Ia32/ExceptionTssEntryAsm.nasm | 4 +--- .../Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm| 4 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm index 45d6474091..6fcf5fb23f 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm @@ -383,6 +383,13 @@ ErrorCodeAndVectorOnStack: pop dword [ebp - 4] mov esp, ebp pop ebp + +; Enable TF bit after page fault handler runs +cmp dword [esp], 14 ; #PF? +jne .5 +bts dword [esp + 16], 8 ; EFLAGS + +.5: add esp, 8 cmp dword [esp - 16], 0 ; check EXCEPTION_HANDLER_CONTEXT.OldIdtHandler jz DoReturn diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionTssEntryAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionTssEntryAsm.nasm index 62bcedea1a..7aac29c7e7 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionTssEntryAsm.nasm +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionTssEntryAsm.nasm @@ -355,10 +355,8 @@ o16 mov [ecx + IA32_TSS._SS], ax movzx ebx, word [ecx + IA32_TSS._CS] mov[eax - 0x8], ebx ; create CS in old stack movebx, dword [ecx + IA32_TSS.EFLAGS] -btsebx, 8 +btsebx, 8; Set TF mov[eax - 0x4], ebx ; create eflags in old stack -movdword [ecx + IA32_TSS.EFLAGS], ebx; update eflags in old TSS -moveax, dword [ecx + IA32_TSS._ESP] ; Get old stack pointer subeax, 0xc ; minus 12 byte movdword [ecx + IA32_TSS._ESP], eax ; Set new stack pointer diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm index 7b97810d10..f842af2336 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm @@ -336,6 +336,10 @@ HasErrorCode: pop r15 mov rsp, rbp +cmp qword [rbp + 8], 14 ; #PF? +jne .1 +bts qword [rsp + 40], 8 ; RFLAGS.TF +.1: pop rbp add rsp, 16 cmp qword [rsp - 32], 0 ; check EXCEPTION_HANDLER_CONTEXT.OldIdtHandler -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 2/4] UefiCpuPkg/CpuExceptionHandlerLib: Setup single step in #PF handler
Once the #PF handler has set the page to be 'present', there should be a way to reset it to 'not-present'. 'TF' bit in EFLAGS can be used for this purpose. 'TF' bit will be set in interrupted function context so that it can be triggered once the cpu control returns back to the instruction causing #PF and re-execute it. This is an necessary step to implement non-stop mode for Heap Guard and NULL Pointer Detection feature. Cc: Eric Dong Cc: Laszlo Ersek Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- .../Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm | 7 +++ .../Library/CpuExceptionHandlerLib/Ia32/ExceptionTssEntryAsm.nasm | 4 +--- .../Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm| 4 3 files changed, 12 insertions(+), 3 deletions(-) diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm index 45d6474091..6fcf5fb23f 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionHandlerAsm.nasm @@ -383,6 +383,13 @@ ErrorCodeAndVectorOnStack: pop dword [ebp - 4] mov esp, ebp pop ebp + +; Enable TF bit after page fault handler runs +cmp dword [esp], 14 ; #PF? +jne .5 +bts dword [esp + 16], 8 ; EFLAGS + +.5: add esp, 8 cmp dword [esp - 16], 0 ; check EXCEPTION_HANDLER_CONTEXT.OldIdtHandler jz DoReturn diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionTssEntryAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionTssEntryAsm.nasm index 62bcedea1a..7aac29c7e7 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionTssEntryAsm.nasm +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ExceptionTssEntryAsm.nasm @@ -355,10 +355,8 @@ o16 mov [ecx + IA32_TSS._SS], ax movzx ebx, word [ecx + IA32_TSS._CS] mov[eax - 0x8], ebx ; create CS in old stack movebx, dword [ecx + IA32_TSS.EFLAGS] -btsebx, 8 +btsebx, 8; Set TF mov[eax - 0x4], ebx ; create eflags in old stack -movdword [ecx + IA32_TSS.EFLAGS], ebx; update eflags in old TSS -moveax, dword [ecx + IA32_TSS._ESP] ; Get old stack pointer subeax, 0xc ; minus 12 byte movdword [ecx + IA32_TSS._ESP], eax ; Set new stack pointer diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm index 7b97810d10..f842af2336 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ExceptionHandlerAsm.nasm @@ -336,6 +336,10 @@ HasErrorCode: pop r15 mov rsp, rbp +cmp qword [rbp + 8], 14 ; #PF? +jne .1 +bts qword [rsp + 40], 8 ; RFLAGS.TF +.1: pop rbp add rsp, 16 cmp qword [rsp - 32], 0 ; check EXCEPTION_HANDLER_CONTEXT.OldIdtHandler -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 4/4] UefiCpuPkg/PiSmmCpuDxeSmm: implement non-stop mode for SMM
Since SMM profile feature has already implemented non-stop mode if #PF occurred, this patch just makes use of the existing implementation to accommodate heap guard and NULL pointer detection feature. Cc: Eric Dong Cc: Laszlo Ersek Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 43 +++-- UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm | 3 +- UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 60 +++- UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h | 15 ++ UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h | 6 +++ UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c | 43 +++-- 6 files changed, 139 insertions(+), 31 deletions(-) diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c index 9300a232e4..a32b736089 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c @@ -38,7 +38,9 @@ SmmInitPageTable ( mPhysicalAddressBits = 32; - if (FeaturePcdGet (PcdCpuSmmProfileEnable)) { + if (FeaturePcdGet (PcdCpuSmmProfileEnable) || + HEAP_GUARD_NONSTOP_MODE || + NULL_DETECTION_NONSTOP_MODE) { // // Set own Page Fault entry instead of the default one, because SMM Profile // feature depends on IRET instruction to do Single Step @@ -129,6 +131,11 @@ SmiPFHandler ( DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextIa32->Eip); ); } + + if (HEAP_GUARD_NONSTOP_MODE) { +GuardPagePFHandler (SystemContext.SystemContextIa32->ExceptionData); +goto Exit; + } } CpuDeadLoop (); } @@ -146,6 +153,26 @@ SmiPFHandler ( ); CpuDeadLoop (); } + +// +// If NULL pointer was just accessed +// +if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT1) != 0 && +(PFAddress < EFI_PAGE_SIZE)) { + DumpCpuContext (InterruptType, SystemContext); + DEBUG ((DEBUG_ERROR, "!!! NULL pointer access !!!\n")); + DEBUG_CODE ( +DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextIa32->Eip); + ); + + if (NULL_DETECTION_NONSTOP_MODE) { +GuardPagePFHandler (SystemContext.SystemContextIa32->ExceptionData); +goto Exit; + } + + CpuDeadLoop (); +} + if (IsSmmCommBufferForbiddenAddress (PFAddress)) { DumpCpuContext (InterruptType, SystemContext); DEBUG ((DEBUG_ERROR, "Access SMM communication forbidden address (0x%x)!\n", PFAddress)); @@ -156,19 +183,6 @@ SmiPFHandler ( } } - // - // If NULL pointer was just accessed - // - if ((PcdGet8 (PcdNullPointerDetectionPropertyMask) & BIT1) != 0 && - (PFAddress < EFI_PAGE_SIZE)) { -DumpCpuContext (InterruptType, SystemContext); -DEBUG ((DEBUG_ERROR, "!!! NULL pointer access !!!\n")); -DEBUG_CODE ( - DumpModuleInfoByIp ((UINTN)SystemContext.SystemContextIa32->Eip); -); -CpuDeadLoop (); - } - if (FeaturePcdGet (PcdCpuSmmProfileEnable)) { SmmProfilePFHandler ( SystemContext.SystemContextIa32->Eip, @@ -179,6 +193,7 @@ SmiPFHandler ( SmiDefaultPFHandler (); } +Exit: ReleaseSpinLock (mPFLock); } diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm index fa02c1016c..879fa0ba63 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm @@ -20,6 +20,7 @@ extern ASM_PFX(FeaturePcdGet (PcdCpuSmmProfileEnable)) extern ASM_PFX(SmiPFHandler) +extern ASM_PFX(mSetupDebugTrap) global ASM_PFX(gcSmiIdtr) global ASM_PFX(gcSmiGdtr) @@ -673,7 +674,7 @@ o16 mov [ecx + IA32_TSS._SS], ax mov esp, ebp ; Set single step DB# if SMM profile is enabled and page fault exception happens -cmp byte [dword ASM_PFX(FeaturePcdGet (PcdCpuSmmProfileEnable))], 0 +cmp byte [dword ASM_PFX(mSetupDebugTrap)], 0 jz @Done2 ; Create return context for iretd in stub function diff --git a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c index b4fe0bc23b..2cc46c8c61 100644 --- a/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c +++ b/UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c @@ -51,6 +51,11 @@ BOOLEAN mBtsSupported = TRUE; // BOOLEAN mSmmProfileStart = FALSE; +// +// The flag indicates if #DB will be setup in #PF handler. +// +BOOLEAN mSetupDebugTrap = FALSE; + // // Record the page fault exception count for one instruction execution. // @@ -229,7 +234,9 @@ DebugExceptionHandler ( UINTN CpuIndex; UINTN PFEntry; - if (!mSmmProfileStart) { + if (!mSmmProfileStart && + !HEAP_GUARD_NONSTOP_MODE && + !NULL_DETECTION_NONSTOP_MODE) { return; }
[edk2] [PATCH 0/4] Support non-stop mode in heap guard and null detection
Background: Heap Guard and NULL Pointer Detection are very useful features to detect code flaw in EDK II. If an issue is detected, #PF exception will be triggered and the BIOS will enter into dead loop, which is the default behavior of exception handling. From QA perspective, this default behavior will block them to collect all tests result in reasonable time. Solution: This patch series update CpuDxe, PiSmmCpuDxeSmm and CpuExceptionHandlerLib to allow the code to continue execution after #PF. The mechanism behind it is the same as SMM Profile feature, in which a special #PF handler is registered to set the page causing #PF to be 'present' and setup single steop trap, then return the control back to the instruction accessing that page. Once the instruction is re-executed, a #DB is triggered and a special handler for it will be called to reset the page back to 'not-present'. Usage: The non-stop mode is enabled/disabled by BIT6 of following PCDs gEfiMdeModulePkgTokenSpaceGuid.PcdHeapGuardPropertyMask gEfiMdeModulePkgTokenSpaceGuid.PcdNullPointerDetectionPropertyMask The default setting is 'disable'. BZ Tracker: https://bugzilla.tianocore.org/show_bug.cgi?id=1095 Jian J Wang (4): MdeModulePkg/MdeModulePkg.dec: add new settings for PCDs UefiCpuPkg/CpuExceptionHandlerLib: Setup single step in #PF handler UefiCpuPkg/CpuDxe: implement non-stop mode for uefi UefiCpuPkg/PiSmmCpuDxeSmm: implement non-stop mode for SMM MdeModulePkg/MdeModulePkg.dec | 4 +- UefiCpuPkg/CpuDxe/CpuDxe.h | 39 +++ UefiCpuPkg/CpuDxe/CpuDxe.inf | 3 + UefiCpuPkg/CpuDxe/CpuMp.c | 34 ++- UefiCpuPkg/CpuDxe/CpuPageTable.c | 271 + .../Ia32/ExceptionHandlerAsm.nasm | 7 + .../Ia32/ExceptionTssEntryAsm.nasm | 4 +- .../X64/ExceptionHandlerAsm.nasm | 4 + UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/PageTbl.c | 43 ++-- UefiCpuPkg/PiSmmCpuDxeSmm/Ia32/SmiException.nasm | 3 +- UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.c | 60 - UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfile.h | 15 ++ UefiCpuPkg/PiSmmCpuDxeSmm/SmmProfileInternal.h | 6 + UefiCpuPkg/PiSmmCpuDxeSmm/X64/PageTbl.c| 43 ++-- 14 files changed, 495 insertions(+), 41 deletions(-) -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 1/4] MdeModulePkg/MdeModulePkg.dec: add new settings for PCDs
BIT6 of PcdHeapGuardPropertyMask is used to enable/disable non-stop mode of Heap Guard feature. It applies to both UEFI and SMM heap guard, if any of them is enabled. BIT6 of PcdNullPointerDetectionPropertyMask is used to enable/disable non-stop mode of NULL Pointer Detection feature. It applies to both UEFI and SMM NULL Pointer Detection, if any of them is enabled. Cc: Eric Dong Cc: Laszlo Ersek Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/MdeModulePkg.dec | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/MdeModulePkg/MdeModulePkg.dec b/MdeModulePkg/MdeModulePkg.dec index 6a6d9660ed..451115030c 100644 --- a/MdeModulePkg/MdeModulePkg.dec +++ b/MdeModulePkg/MdeModulePkg.dec @@ -932,7 +932,8 @@ # If enabled, accessing NULL address in UEFI or SMM code can be caught. #BIT0- Enable NULL pointer detection for UEFI. #BIT1- Enable NULL pointer detection for SMM. - #BIT2..6 - Reserved for future uses. + #BIT2..5 - Reserved for future uses. + #BIT6- Enable non-stop mode. #BIT7- Disable NULL pointer detection just after EndOfDxe. # This is a workaround for those unsolvable NULL access issues in # OptionROM, boot loader, etc. It can also help to avoid unnecessary @@ -1014,6 +1015,7 @@ # BIT1 - Enable UEFI pool guard. # BIT2 - Enable SMM page guard. # BIT3 - Enable SMM pool guard. + # BIT6 - Enable non-stop mode. # BIT7 - The direction of Guard Page for Pool Guard. # 0 - The returned pool is near the tail guard page. # 1 - The returned pool is near the head guard page. -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH] UefiCpuPkg/CpuDxe: fix incorrect check of SMM mode
Current IsInSmm() method makes use of gEfiSmmBase2ProtocolGuid.InSmm() to check if current processor is in SMM mode or not. But this is not correct because gEfiSmmBase2ProtocolGuid.InSmm() can only detect if the caller is running in SMRAM or from SMM driver. It cannot guarantee if the caller is running in SMM mode. Because SMM mode will load its own page table, adding an extra check of saved DXE page table base address against current CR3 register value can help to get the correct answer for sure (in SMM mode or not in SMM mode). This is an issue caused by check-in at d106cf71eabaacff63c14626a4a87346b93074dd Cc: Eric Dong Cc: Laszlo Ersek Cc: Jiewen Yao Cc: Star Zeng Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/CpuDxe/CpuPageTable.c | 9 - 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c index 850eed60e7..df021798c0 100644 --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c @@ -136,7 +136,14 @@ IsInSmm ( mSmmBase2->InSmm (mSmmBase2, ); } - return InSmm; + // + // mSmmBase2->InSmm() can only detect if the caller is running in SMRAM + // or from SMM driver. It cannot tell if the caller is running in SMM mode. + // Check page table base address to guarantee that because SMM mode willl + // load its own page table. + // + return (InSmm && + mPagingContext.ContextData.X64.PageTableBase != (UINT64)AsmReadCr3()); } /** -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 0/2] fix DXE memory free issue in SMM mode
> v2: > a. add more specific explanations in commit message > b. add more comments in code > c. remove redundant logic in code > d. fix logic hole in code > e. replace meanless constant macro with meaning ones > f. remove irrelated changes This patch series try to fix an issue caused by trying to free memory allocated in DXE but freed in SMM mode. This happens only if Heap Guard feature is enabled, which needs to update page table. The root cause is that DXE and SMM don't share the same paging configuration but CpuDxe driver still tries to access page table through current processor registers (CR3) in SMM mode, during memory free opration in DXE core. This will cause DXE core got the incorrect paging attributes of memory to be freed, and then fail the free operation. The solution is let CpuDxe store the paging configuration in a global variable and use it to access DXE page table if in SMM mode. Jian J Wang (2): UefiCpuPkg/CpuDxe: allow accessing (DXE) page table in SMM mode MdeModulePkg/Core: remove SMM check for Heap Guard feature detection MdeModulePkg/Core/Dxe/Mem/HeapGuard.c | 10 --- UefiCpuPkg/CpuDxe/CpuDxe.inf | 1 + UefiCpuPkg/CpuDxe/CpuPageTable.c | 159 ++ 3 files changed, 123 insertions(+), 47 deletions(-) -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 2/2] MdeModulePkg/Core: remove SMM check for Heap Guard feature detection
CpuDxe driver is updated to be able to access DXE page table in SMM mode, which means Heap Guard can get correct memory paging attributes in what environment. It's not necessary to exclude SMM from detecting Heap Guard feature support. Cc: Star Zeng Cc: Eric Dong Cc: Jiewen Yao Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- MdeModulePkg/Core/Dxe/Mem/HeapGuard.c | 10 -- 1 file changed, 10 deletions(-) diff --git a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c index 9d765c98f6..447c56bb11 100644 --- a/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c +++ b/MdeModulePkg/Core/Dxe/Mem/HeapGuard.c @@ -667,21 +667,11 @@ IsMemoryTypeToGuard ( { UINT64 TestBit; UINT64 ConfigBit; - BOOLEAN InSmm; if (AllocateType == AllocateAddress) { return FALSE; } - InSmm = FALSE; - if (gSmmBase2 != NULL) { -gSmmBase2->InSmm (gSmmBase2, ); - } - - if (InSmm) { -return FALSE; - } - if ((PcdGet8 (PcdHeapGuardPropertyMask) & PageOrPool) == 0) { return FALSE; } -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH v2 1/2] UefiCpuPkg/CpuDxe: allow accessing (DXE) page table in SMM mode
> v2: > a. add more specific explanations in commit message > b. add more comments in code > c. remove redundant logic in IsInSmm() > d. fix a logic hole in GetCurrentPagingContext() > e. replace meanless constant macro with meaning ones The MdePkg/Library/SmmMemoryAllocationLib, used only by DXE_SMM_DRIVER, allows to free memory allocated in DXE (before EndOfDxe). This is done by checking the memory range and calling gBS services to do real operation if the memory to free is out of SMRAM. If some memory related features, like Heap Guard, are enabled, gBS interface will turn to EFI_CPU_ARCH_PROTOCOL.SetMemoryAttributes(), provided by DXE driver UefiCpuPkg/CpuDxe, to change memory paging attributes. This means we have part of DXE code running in SMM mode in certain circumstances. Because page table in SMM mode is different from DXE mode and CpuDxe always uses current registers (CR0, CR3, etc.) to get memory paging attributes, it cannot get the correct attributes of DXE memory in SMM mode from SMM page table. This will cause incorrect memory manipulations, like fail the releasing of Guard pages if Heap Guard is enabled. The solution in this patch is to store the DXE page table information (e.g. value of CR0, CR3 registers, etc.) in a global variable of CpuDxe driver. If CpuDxe detects it's in SMM mode, it will use this global variable to access page table instead of current processor registers. This can avoid retrieving wrong DXE memory paging attributes and changing SMM page table attributes unexpectedly. Cc: Eric Dong Cc: Laszlo Ersek Cc: Jiewen Yao Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/CpuDxe/CpuDxe.inf | 1 + UefiCpuPkg/CpuDxe/CpuPageTable.c | 159 ++- 2 files changed, 123 insertions(+), 37 deletions(-) diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.inf b/UefiCpuPkg/CpuDxe/CpuDxe.inf index 3c938cee53..ce2bd3627c 100644 --- a/UefiCpuPkg/CpuDxe/CpuDxe.inf +++ b/UefiCpuPkg/CpuDxe/CpuDxe.inf @@ -66,6 +66,7 @@ [Protocols] gEfiCpuArchProtocolGuid ## PRODUCES gEfiMpServiceProtocolGuid ## PRODUCES + gEfiSmmBase2ProtocolGuid ## CONSUMES [Guids] gIdleLoopEventGuid## CONSUMES ## Event diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c index e2595b4d89..b7e75922b6 100644 --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c @@ -23,10 +23,21 @@ #include #include #include +#include +#include +#include #include "CpuDxe.h" #include "CpuPageTable.h" +/// +/// Paging registers +/// +#define CR0_WP BIT16 +#define CR0_PG BIT31 +#define CR4_PSE BIT4 +#define CR4_PAE BIT5 + /// /// Page Table Entry /// @@ -87,7 +98,46 @@ PAGE_ATTRIBUTE_TABLE mPageAttributeTable[] = { {Page1G, SIZE_1GB, PAGING_1G_ADDRESS_MASK_64}, }; -PAGE_TABLE_POOL *mPageTablePool = NULL; +PAGE_TABLE_POOL *mPageTablePool = NULL; +PAGE_TABLE_LIB_PAGING_CONTEXT mPagingContext; +EFI_SMM_BASE2_PROTOCOL*mSmmBase2 = NULL; + +/** + Check if current execution environment is in SMM mode or not, via + EFI_SMM_BASE2_PROTOCOL. + + This is necessary because of the fact that MdePkg\Library\SmmMemoryAllocationLib + supports to free memory outside SMRAM. The library will call gBS->FreePool() or + gBS->FreePages() and then SetMemorySpaceAttributes interface in turn to change + memory paging attributes during free operation, if some memory related features + are enabled (like Heap Guard). + + This means that SetMemorySpaceAttributes() has chance to run in SMM mode. This + will cause incorrect result because SMM mode always loads its own page tables, + which are usually different from DXE. This function can be used to detect such + situation and help to avoid further misoperations. + + @retval TRUEIn SMM mode. + @retval FALSE Not in SMM mode. +**/ +BOOLEAN +IsInSmm ( + VOID + ) +{ + BOOLEAN InSmm; + + InSmm = FALSE; + if (mSmmBase2 == NULL) { +gBS->LocateProtocol (, NULL, (VOID **)); + } + + if (mSmmBase2 != NULL) { +mSmmBase2->InSmm (mSmmBase2, ); + } + + return InSmm; +} /** Return current paging context. @@ -99,45 +149,61 @@ GetCurrentPagingContext ( IN OUT PAGE_TABLE_LIB_PAGING_CONTEXT *PagingContext ) { - UINT32 RegEax; - UINT32 RegEdx; + UINT32 RegEax; + CPUID_EXTENDED_CPU_SIG_EDX RegEdx; + MSR_IA32_EFER_REGISTER MsrEfer; - ZeroMem(PagingContext, sizeof(*PagingContext)); - if (sizeof(UINTN) == sizeof(UINT64)) { -PagingContext->MachineType = IMAGE_FILE_MACHINE_X64; - } else { -PagingContext->MachineType = IMAGE_FIL
[edk2] [PATCH 0/2] fix DXE memory free issue in SMM mode
This patch series try to fix an issue caused by trying to free memory allocated in DXE but freed in SMM mode. This happens only if Heap Guard feature is enabled, which needs to update page table. The root cause is that DXE and SMM don't share the same paging configuration but CpuDxe driver still tries to access page table through current processor registers (CR3) in SMM mode, during memory free opration in DXE core. This will cause DXE core got the incorrect paging attributes of memory to be freed, and then fail the free operation. The solution is let CpuDxe store the paging configuration in a global variable and use it to access DXE page table if in SMM mode. Jian J Wang (2): UefiCpuPkg/CpuDxe: allow accessing (DXE) page table in SMM mode MdeModulePkg/Core: remove SMM check for Heap Guard feature detection MdeModulePkg/Core/Dxe/Mem/HeapGuard.c | 10 UefiCpuPkg/CpuDxe/CpuDxe.c| 2 +- UefiCpuPkg/CpuDxe/CpuDxe.inf | 1 + UefiCpuPkg/CpuDxe/CpuPageTable.c | 108 +++--- 4 files changed, 75 insertions(+), 46 deletions(-) -- 2.16.2.windows.1 ___ edk2-devel mailing list edk2-devel@lists.01.org https://lists.01.org/mailman/listinfo/edk2-devel
[edk2] [PATCH 1/2] UefiCpuPkg/CpuDxe: allow accessing (DXE) page table in SMM mode
The SMM version of MemoryAllocationLib allows to free memory allocated in DXE (before EndOfDxe). This is done by checking the memory range and calling gBS services to do real operation if the memory to free is out of SMRAM. This would cause problem if some memory related features, like Heap Guard, have to update page table to change memory attributes. Because page table in SMM mode is different from DXE mode, gBS memory services cannot get the correct attributes of DXE memory from SMM page table and then cause incorrect memory manipulations. The solution in this patch is to store the DXE page table information (e.g. value of CR0, CR3 registers, etc.) in a global variable of CpuDxe driver. If CpuDxe detects it's in SMM mode, it will use this global variable to access page table instead of current processor registers. Change-Id: If810bb1828160b8bdd8cb616d86df2859c74971f Cc: Eric Dong Cc: Laszlo Ersek Cc: Jiewen Yao Cc: Ruiyu Ni Contributed-under: TianoCore Contribution Agreement 1.1 Signed-off-by: Jian J Wang --- UefiCpuPkg/CpuDxe/CpuDxe.c | 2 +- UefiCpuPkg/CpuDxe/CpuDxe.inf | 1 + UefiCpuPkg/CpuDxe/CpuPageTable.c | 108 ++- 3 files changed, 75 insertions(+), 36 deletions(-) diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.c b/UefiCpuPkg/CpuDxe/CpuDxe.c index 6ae2dcd1c7..1fd996fc3f 100644 --- a/UefiCpuPkg/CpuDxe/CpuDxe.c +++ b/UefiCpuPkg/CpuDxe/CpuDxe.c @@ -404,7 +404,7 @@ CpuSetMemoryAttributes ( // to avoid unnecessary computing. // if (mIsFlushingGCD) { -DEBUG((DEBUG_INFO, " Flushing GCD\n")); +DEBUG((DEBUG_GCD, " Flushing GCD\n")); return EFI_SUCCESS; } diff --git a/UefiCpuPkg/CpuDxe/CpuDxe.inf b/UefiCpuPkg/CpuDxe/CpuDxe.inf index 3c938cee53..8c8773af90 100644 --- a/UefiCpuPkg/CpuDxe/CpuDxe.inf +++ b/UefiCpuPkg/CpuDxe/CpuDxe.inf @@ -66,6 +66,7 @@ [Protocols] gEfiCpuArchProtocolGuid ## PRODUCES gEfiMpServiceProtocolGuid ## PRODUCES + gEfiSmmBase2ProtocolGuid [Guids] gIdleLoopEventGuid## CONSUMES ## Event diff --git a/UefiCpuPkg/CpuDxe/CpuPageTable.c b/UefiCpuPkg/CpuDxe/CpuPageTable.c index e2595b4d89..bf420d3792 100644 --- a/UefiCpuPkg/CpuDxe/CpuPageTable.c +++ b/UefiCpuPkg/CpuDxe/CpuPageTable.c @@ -23,6 +23,7 @@ #include #include #include +#include #include "CpuDxe.h" #include "CpuPageTable.h" @@ -87,7 +88,33 @@ PAGE_ATTRIBUTE_TABLE mPageAttributeTable[] = { {Page1G, SIZE_1GB, PAGING_1G_ADDRESS_MASK_64}, }; -PAGE_TABLE_POOL *mPageTablePool = NULL; +PAGE_TABLE_POOL *mPageTablePool = NULL; +PAGE_TABLE_LIB_PAGING_CONTEXT mPagingContext; +EFI_SMM_BASE2_PROTOCOL*mSmmBase2 = NULL; + +BOOLEAN +IsInSmm ( + VOID + ) +{ + EFI_STATUS Status; + BOOLEAN InSmm; + + InSmm = FALSE; + if (mSmmBase2 == NULL) { +Status = gBS->LocateProtocol (, NULL, + (VOID **)); +if (EFI_ERROR (Status)) { + mSmmBase2 = NULL; +} + } + + if (mSmmBase2 != NULL) { +mSmmBase2->InSmm (mSmmBase2, ); + } + + return InSmm; +} /** Return current paging context. @@ -102,42 +129,45 @@ GetCurrentPagingContext ( UINT32 RegEax; UINT32 RegEdx; - ZeroMem(PagingContext, sizeof(*PagingContext)); - if (sizeof(UINTN) == sizeof(UINT64)) { -PagingContext->MachineType = IMAGE_FILE_MACHINE_X64; - } else { -PagingContext->MachineType = IMAGE_FILE_MACHINE_I386; - } - if ((AsmReadCr0 () & BIT31) != 0) { -PagingContext->ContextData.X64.PageTableBase = (AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64); - } else { -PagingContext->ContextData.X64.PageTableBase = 0; - } + if (!IsInSmm ()) { +if (sizeof(UINTN) == sizeof(UINT64)) { + mPagingContext.MachineType = IMAGE_FILE_MACHINE_X64; +} else { + mPagingContext.MachineType = IMAGE_FILE_MACHINE_I386; +} +if ((AsmReadCr0 () & BIT31) != 0) { + mPagingContext.ContextData.X64.PageTableBase = (AsmReadCr3 () & PAGING_4K_ADDRESS_MASK_64); +} else { + mPagingContext.ContextData.X64.PageTableBase = 0; +} - if ((AsmReadCr4 () & BIT4) != 0) { -PagingContext->ContextData.Ia32.Attributes |= PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PSE; - } - if ((AsmReadCr4 () & BIT5) != 0) { -PagingContext->ContextData.Ia32.Attributes |= PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PAE; - } - if ((AsmReadCr0 () & BIT16) != 0) { -PagingContext->ContextData.Ia32.Attributes |= PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_WP_ENABLE; - } +if ((AsmReadCr4 () & BIT4) != 0) { + mPagingContext.ContextData.Ia32.Attributes |= PAGE_TABLE_LIB_PAGING_CONTEXT_IA32_X64_ATTRIBUTES_PSE; +} +if ((AsmReadCr4 () & BIT5) != 0) { + mPagingContext.ContextData.Ia32.Attri