diff --git a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c index 5f9bcf79669..87ae555020c 100644 --- a/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c +++ b/ArmPkg/Library/DefaultExceptionHandlerLib/AArch64/DefaultExceptionHandler.c @@ -15,6 +15,7 @@ #include #include #include +#include // MU_CHANGE: CLANGPDB Stack Cookies #include #include @@ -194,6 +195,10 @@ DefaultExceptionHandler ( CHAR16 UnicodeBuffer[MAX_PRINT_CHARS]; UINTN CharCount; INT32 Offset; + // MU_CHANGE BEGIN: CLANGPDB Stack Cookies + UINT8 Ec; + UINT32 Iss; + // MU_CHANGE END: CLANGPDB Stack Cookies if (mRecursiveException) { STATIC CHAR8 CONST Message[] = "\nRecursive exception occurred while dumping the CPU state\n"; @@ -202,8 +207,19 @@ DefaultExceptionHandler ( } mRecursiveException = TRUE; + // MU_CHANGE BEGIN: CLANGPDB Stack Cookies + Ec = (SystemContext.SystemContextAArch64->ESR & 0xFC000000) >> 26; + Iss = SystemContext.SystemContextAArch64->ESR & 0x1FFFFFF; + + // Check if this is a stack cookie violation + if ((Ec == 0x15) && (Iss == STACK_CHECK_ERROR_HANDLER_VECTOR)) { + // The address that caused the stack check exception is in X0 + CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "\n\n%a Stack Check Exception at 0x%016lx\n", gExceptionTypeString[ExceptionType], SystemContext.SystemContextAArch64->X0); + } else { + CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "\n\n%a Exception at 0x%016lx\n", gExceptionTypeString[ExceptionType], SystemContext.SystemContextAArch64->ELR); + } + // MU_CHANGE END: CLANGPDB Stack Cookies - CharCount = AsciiSPrint (Buffer, sizeof (Buffer), "\n\n%a Exception at 0x%016lx\n", gExceptionTypeString[ExceptionType], SystemContext.SystemContextAArch64->ELR); SerialPortWrite ((UINT8 *)Buffer, CharCount); // Prepare a unicode buffer for ConOut, if applicable, in case the buffer diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template index a0f265647f8..b930bda0ce8 100755 --- a/BaseTools/Conf/tools_def.template +++ b/BaseTools/Conf/tools_def.template @@ -27,7 +27,8 @@ # 4.02 - Add ARCHCC_FLAGS for GCC AARCH64 # 4.03 - Clang AARCH64 Keep optional header for NX_COMPAT flag # 4.05 - Add CLANGPDB AARCH64 Support -#!VERSION=4.05 +# 4.06 - Enable Stack Cookies for CLANGPDB # MU_CHANGE: CLANGPDB Stack Cookies +#!VERSION=4.06 # MU_CHANGE: CLANGPDB Stack Cookies IDENTIFIER = Default TOOL_CHAIN_CONF @@ -1674,7 +1675,8 @@ DEFINE CLANGPDB_X64_TARGET = -target x86_64-pc-windows-msvc DEFINE CLANGPDB_AARCH64_TARGET = -target aarch64-unknown-windows-msvc DEFINE CLANGPDB_WARNING_OVERRIDES = -Wno-parentheses-equality -Wno-tautological-compare -Wno-tautological-constant-out-of-range-compare -Wno-empty-body -Wno-unused-const-variable -Wno-varargs -Wno-unknown-warning-option -Wno-unused-but-set-variable -Wno-unused-const-variable -Wno-unaligned-access -Wno-microsoft-enum-forward-reference -DEFINE CLANGPDB_ALL_CC_FLAGS = DEF(GCC48_ALL_CC_FLAGS) DEF(CLANGPDB_WARNING_OVERRIDES) -fno-stack-protector -funsigned-char -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -Wno-address -Wno-shift-negative-value -Wno-unknown-pragmas -Wno-incompatible-library-redeclaration -Wno-null-dereference -mno-implicit-float -mms-bitfields -mno-stack-arg-probe -fno-omit-frame-pointer -U _MSC_VER -D __GNUC__=4 -D __GNUC_MINOR__=2 -D __GNUC_PATCHLEVEL__=1 -D __MINGW32__=1 +# MU_CHANGE: CLANGPDB Stack Cookies +DEFINE CLANGPDB_ALL_CC_FLAGS = DEF(GCC48_ALL_CC_FLAGS) DEF(CLANGPDB_WARNING_OVERRIDES) -fstack-protector -funsigned-char -ftrap-function=undefined_behavior_has_been_optimized_away_by_clang -Wno-address -Wno-shift-negative-value -Wno-unknown-pragmas -Wno-incompatible-library-redeclaration -Wno-null-dereference -mno-implicit-float -mms-bitfields -mno-stack-arg-probe -fno-omit-frame-pointer -U _MSC_VER -D __GNUC__=4 -D __GNUC_MINOR__=2 -D __GNUC_PATCHLEVEL__=1 -D __MINGW32__=1 ########################### # CLANGPDB IA32 definitions diff --git a/MdePkg/Include/Library/StackCheckFailureHookLib.h b/MdePkg/Include/Library/StackCheckFailureHookLib.h deleted file mode 100644 index f0657ddfa08..00000000000 --- a/MdePkg/Include/Library/StackCheckFailureHookLib.h +++ /dev/null @@ -1,26 +0,0 @@ -/** @file - Library provides a hook called when a stack cookie check fails. - - Copyright (c) Microsoft Corporation. - SPDX-License-Identifier: BSD-2-Clause-Patent -**/ - -#ifndef STACK_COOKIE_FAILURE_HOOK_LIB_H_ -#define STACK_COOKIE_FAILURE_HOOK_LIB_H_ - -#include - -/** - This function gets called when a compiler generated stack cookie fails. This allows a platform to hook this - call and perform any required actions/telemetry at that time. - - @param FailureAddress The address of the function that failed the stack cookie check. - -**/ -VOID -EFIAPI -StackCheckFailureHook ( - VOID *FailureAddress - ); - -#endif diff --git a/MdePkg/Include/Library/StackCheckLib.h b/MdePkg/Include/Library/StackCheckLib.h index 5773caafa51..c5aa9592bcf 100644 --- a/MdePkg/Include/Library/StackCheckLib.h +++ b/MdePkg/Include/Library/StackCheckLib.h @@ -12,6 +12,12 @@ #include +// MU_CHANGE BEGIN: CLANGPDB Stack Cookies +// The stack cookie error handler vector, in the user defined range for IA32, X64, and AARCH64. This is the +// exception/interrupt number that is signalled on a stack cookie corruption. +#define STACK_CHECK_ERROR_HANDLER_VECTOR 0x42 +// MU_CHANGE END: CLANGPDB Stack Cookies + #if defined (__GNUC__) || defined (__clang__) // The __stack_chk_guard is a random value placed on the stack between the stack variables diff --git a/MdePkg/Library/DynamicStackCookieEntryPointLib/AArch64/DynamicCookieGcc.S b/MdePkg/Library/DynamicStackCookieEntryPointLib/AArch64/DynamicCookieGcc.S index d33c02a7c4b..744f254ac31 100644 --- a/MdePkg/Library/DynamicStackCookieEntryPointLib/AArch64/DynamicCookieGcc.S +++ b/MdePkg/Library/DynamicStackCookieEntryPointLib/AArch64/DynamicCookieGcc.S @@ -5,7 +5,7 @@ # # Module Name: # -# DynamicCookie.S +# DynamicCookieGcc.S # MU_CHANGE: CLANGPDB Stack Cookies # # Abstract: # diff --git a/MdePkg/Library/DynamicStackCookieEntryPointLib/AArch64/DynamicCookieMsvc.S b/MdePkg/Library/DynamicStackCookieEntryPointLib/AArch64/DynamicCookieMsvc.S new file mode 100644 index 00000000000..8741356e795 --- /dev/null +++ b/MdePkg/Library/DynamicStackCookieEntryPointLib/AArch64/DynamicCookieMsvc.S @@ -0,0 +1,58 @@ +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies - new file +#------------------------------------------------------------------------------ +# +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: BSD-2-Clause-Patent +# +# Module Name: +# +# DynamicCookieMsvc.S +# +# Abstract: +# +# Generates random number through the RNDR instruction on a 64-bit AARCH64 platform +# to store a random value in the MSVC __security_cookie stack cookie. +# The first byte is 0'd to prevent string copy functions from clobbering +# the stack cookie. +# +# Notes: +# +# If RNDR fails, the build time static stack cookie value will be used instead. +# +#------------------------------------------------------------------------------ + +#include + +.text +.p2align 2 + +GCC_ASM_IMPORT(__security_cookie) +GCC_ASM_IMPORT(_CModuleEntryPoint) +GCC_ASM_EXPORT(_ModuleEntryPoint) + +#------------------------------------------------------------------------------ +# VOID +# EFIAPI +# _ModuleEntryPoint ( +# Parameters are passed through. +# ) +#------------------------------------------------------------------------------ +ASM_PFX(_ModuleEntryPoint): + AARCH64_BTI(c) + + mrs x9, ID_AA64ISAR0_EL1 // Read the AArch64 Instruction Set Attribute Register 0 + ubfx x9, x9, #60, #4 // Extract the RNDR bit field (bits 60-63) + cbz x9, c_entry // If RNDR is not supported, jump to c_entry + + mrs x9, RNDR // Generate a random number + b.eq c_entry // RNDR sets NZCV to 0b0100 on failure + // So if the zero flag is set, use the static stack guard + + and x9, x9, #0xFFFFFFFFFFFFFF00 // Zero the first byte of the random value + + adrp x8, ASM_PFX(__security_cookie) // Load the page address of __security_cookie + str x9, [x8, :lo12:ASM_PFX(__security_cookie)] // Store the random value in __security_cookie + +c_entry: + b ASM_PFX(_CModuleEntryPoint) // Jump to the C module entry point +# MU_CHANGE END: CLANGPDB Stack Cookies diff --git a/MdePkg/Library/DynamicStackCookieEntryPointLib/DxeCoreEntryPoint.inf b/MdePkg/Library/DynamicStackCookieEntryPointLib/DxeCoreEntryPoint.inf index ea849876780..da9a50c3e87 100644 --- a/MdePkg/Library/DynamicStackCookieEntryPointLib/DxeCoreEntryPoint.inf +++ b/MdePkg/Library/DynamicStackCookieEntryPointLib/DxeCoreEntryPoint.inf @@ -25,16 +25,19 @@ [Sources] DxeCore/DxeCoreEntryPoint.c +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies [Sources.IA32] - IA32/DynamicCookieGcc.nasm | GCC - IA32/DynamicCookieMsvc.nasm | MSFT + IA32/DynamicCookieGcc.nasm ||||!gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking + IA32/DynamicCookieMsvc.nasm ||||gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking [Sources.X64] - X64/DynamicCookieGcc.nasm | GCC - X64/DynamicCookieMsvc.nasm | MSFT + X64/DynamicCookieGcc.nasm ||||!gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking + X64/DynamicCookieMsvc.nasm ||||gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking [Sources.AARCH64] - AArch64/DynamicCookieGcc.S | GCC + AArch64/DynamicCookieGcc.S ||||!gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking + AArch64/DynamicCookieMsvc.S ||||gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking +# MU_CHANGE END: CLANGPDB Stack Cookies [Packages] MdePkg/MdePkg.dec @@ -43,3 +46,8 @@ BaseLib DebugLib StackCheckLib + +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies +[FeaturePcd] + gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking +# MU_CHANGE END: CLANGPDB Stack Cookies diff --git a/MdePkg/Library/DynamicStackCookieEntryPointLib/StandaloneMmCoreEntryPoint.inf b/MdePkg/Library/DynamicStackCookieEntryPointLib/StandaloneMmCoreEntryPoint.inf index 41361dde53b..c956a66dbd2 100644 --- a/MdePkg/Library/DynamicStackCookieEntryPointLib/StandaloneMmCoreEntryPoint.inf +++ b/MdePkg/Library/DynamicStackCookieEntryPointLib/StandaloneMmCoreEntryPoint.inf @@ -25,9 +25,11 @@ [Sources.X64] StandaloneMmCore/X64/StandaloneMmCoreEntryPoint.c +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies [Sources.X64] - X64/DynamicCookieGcc.nasm | GCC - X64/DynamicCookieMsvc.nasm | MSFT + X64/DynamicCookieGcc.nasm ||||!gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking + X64/DynamicCookieMsvc.nasm ||||gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking +# MU_CHANGE END: CLANGPDB Stack Cookies [Packages] MdePkg/MdePkg.dec @@ -36,3 +38,8 @@ BaseLib DebugLib StackCheckLib + +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies +[FeaturePcd] + gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking +# MU_CHANGE END: CLANGPDB Stack Cookies diff --git a/MdePkg/Library/DynamicStackCookieEntryPointLib/StandaloneMmDriverEntryPoint.inf b/MdePkg/Library/DynamicStackCookieEntryPointLib/StandaloneMmDriverEntryPoint.inf index 1d43ce9345c..43a9ce001c3 100644 --- a/MdePkg/Library/DynamicStackCookieEntryPointLib/StandaloneMmDriverEntryPoint.inf +++ b/MdePkg/Library/DynamicStackCookieEntryPointLib/StandaloneMmDriverEntryPoint.inf @@ -29,16 +29,19 @@ [Sources] StandaloneMmDriver/StandaloneMmDriverEntryPoint.c +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies [Sources.IA32] - IA32/DynamicCookieGcc.nasm | GCC - IA32/DynamicCookieMsvc.nasm | MSFT + IA32/DynamicCookieGcc.nasm ||||!gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking + IA32/DynamicCookieMsvc.nasm ||||gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking [Sources.X64] - X64/DynamicCookieGcc.nasm | GCC - X64/DynamicCookieMsvc.nasm | MSFT + X64/DynamicCookieGcc.nasm ||||!gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking + X64/DynamicCookieMsvc.nasm ||||gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking [Sources.AARCH64] - AArch64/DynamicCookieGcc.S | GCC + AArch64/DynamicCookieGcc.S ||||!gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking + AArch64/DynamicCookieMsvc.S ||||gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking +# MU_CHANGE END: CLANGPDB Stack Cookies [Packages] MdePkg/MdePkg.dec @@ -51,3 +54,8 @@ [Protocols] gEfiLoadedImageProtocolGuid ## SOMETIMES_CONSUMES + +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies +[FeaturePcd] + gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking +# MU_CHANGE END: CLANGPDB Stack Cookies diff --git a/MdePkg/Library/DynamicStackCookieEntryPointLib/UefiApplicationEntryPoint.inf b/MdePkg/Library/DynamicStackCookieEntryPointLib/UefiApplicationEntryPoint.inf index 596303e0017..4dc9e3c414f 100644 --- a/MdePkg/Library/DynamicStackCookieEntryPointLib/UefiApplicationEntryPoint.inf +++ b/MdePkg/Library/DynamicStackCookieEntryPointLib/UefiApplicationEntryPoint.inf @@ -24,16 +24,19 @@ [Sources] UefiApplication/ApplicationEntryPoint.c +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies [Sources.IA32] - IA32/DynamicCookieGcc.nasm | GCC - IA32/DynamicCookieMsvc.nasm | MSFT + IA32/DynamicCookieGcc.nasm ||||!gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking + IA32/DynamicCookieMsvc.nasm ||||gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking [Sources.X64] - X64/DynamicCookieGcc.nasm | GCC - X64/DynamicCookieMsvc.nasm | MSFT + X64/DynamicCookieGcc.nasm ||||!gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking + X64/DynamicCookieMsvc.nasm ||||gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking [Sources.AARCH64] - AArch64/DynamicCookieGcc.S | GCC + AArch64/DynamicCookieGcc.S ||||!gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking + AArch64/DynamicCookieMsvc.S ||||gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking +# MU_CHANGE END: CLANGPDB Stack Cookies [Packages] MdePkg/MdePkg.dec @@ -43,3 +46,8 @@ DebugLib BaseLib StackCheckLib + +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies +[FeaturePcd] + gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking +# MU_CHANGE END: CLANGPDB Stack Cookies diff --git a/MdePkg/Library/DynamicStackCookieEntryPointLib/UefiDriverEntryPoint.inf b/MdePkg/Library/DynamicStackCookieEntryPointLib/UefiDriverEntryPoint.inf index d624c3e1a36..3e6ab49ba74 100644 --- a/MdePkg/Library/DynamicStackCookieEntryPointLib/UefiDriverEntryPoint.inf +++ b/MdePkg/Library/DynamicStackCookieEntryPointLib/UefiDriverEntryPoint.inf @@ -29,16 +29,19 @@ [Packages] MdePkg/MdePkg.dec +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies [Sources.IA32] - IA32/DynamicCookieGcc.nasm | GCC - IA32/DynamicCookieMsvc.nasm | MSFT + IA32/DynamicCookieGcc.nasm ||||!gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking + IA32/DynamicCookieMsvc.nasm ||||gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking [Sources.X64] - X64/DynamicCookieGcc.nasm | GCC - X64/DynamicCookieMsvc.nasm | MSFT + X64/DynamicCookieGcc.nasm ||||!gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking + X64/DynamicCookieMsvc.nasm ||||gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking [Sources.AARCH64] - AArch64/DynamicCookieGcc.S | GCC + AArch64/DynamicCookieGcc.S ||||!gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking + AArch64/DynamicCookieMsvc.S ||||gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking +# MU_CHANGE END: CLANGPDB Stack Cookies [LibraryClasses] UefiBootServicesTableLib @@ -66,3 +69,8 @@ gEfiVariableWriteArchProtocolGuid AND gEfiVariableArchProtocolGuid AND gEfiWatchdogTimerArchProtocolGuid + +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies +[FeaturePcd] + gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking +# MU_CHANGE END: CLANGPDB Stack Cookies diff --git a/MdePkg/Library/StackCheckFailureHookLibNull/StackCheckFailureHook.c b/MdePkg/Library/StackCheckFailureHookLibNull/StackCheckFailureHook.c deleted file mode 100644 index 0a258e4773e..00000000000 --- a/MdePkg/Library/StackCheckFailureHookLibNull/StackCheckFailureHook.c +++ /dev/null @@ -1,25 +0,0 @@ -/** @file - Library provides a hook called when a stack cookie check fails. - - Copyright (c) Microsoft Corporation. - SPDX-License-Identifier: BSD-2-Clause-Patent -**/ - -#include -#include - -/** - This function gets called when a compiler generated stack cookie fails. This allows a platform to hook this - call and perform any required actions/telemetry at that time. - - @param FailureAddress The address of the function that failed the stack cookie check. - -**/ -VOID -EFIAPI -StackCheckFailureHook ( - VOID *FailureAddress - ) -{ - return; -} diff --git a/MdePkg/Library/StackCheckFailureHookLibNull/StackCheckFailureHookLibNull.inf b/MdePkg/Library/StackCheckFailureHookLibNull/StackCheckFailureHookLibNull.inf deleted file mode 100644 index 300073a7e7b..00000000000 --- a/MdePkg/Library/StackCheckFailureHookLibNull/StackCheckFailureHookLibNull.inf +++ /dev/null @@ -1,20 +0,0 @@ -## @file -# Library provides a hook called when a stack cookie check fails. -# -# Copyright (c) Microsoft Corporation. -# SPDX-License-Identifier: BSD-2-Clause-Patent -## - -[Defines] - INF_VERSION = 1.29 - BASE_NAME = StackCheckFailureHookLibNull - FILE_GUID = 9ca2587c-d1f2-451a-989a-d49a9a0a613e - MODULE_TYPE = BASE - VERSION_STRING = 1.0 - LIBRARY_CLASS = StackCheckFailureHookLib - -[Sources] - StackCheckFailureHook.c - -[Packages] - MdePkg/MdePkg.dec diff --git a/MdePkg/Library/StackCheckLib/AArch64/CheckCookieMsvc.S b/MdePkg/Library/StackCheckLib/AArch64/CheckCookieMsvc.S new file mode 100644 index 00000000000..d171b55160d --- /dev/null +++ b/MdePkg/Library/StackCheckLib/AArch64/CheckCookieMsvc.S @@ -0,0 +1,30 @@ +// MU_CHANGE BEGIN: CLANGPDB Stack Cookies - new file +//------------------------------------------------------------------------------ +// AArch64/CheckCookieMsvc.S +// +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: BSD-2-Clause-Patent +//------------------------------------------------------------------------------ + +#include + + .text + +//------------------------------------------------------------------------------ +// Checks the stack cookie value against __security_cookie and calls the +// stack cookie failure handler if there is a mismatch. +// +// VOID +// EFIAPI +// __security_check_cookie ( +// IN UINTN CheckValue +// ); +//------------------------------------------------------------------------------ +.global ASM_PFX(__security_check_cookie) +ASM_PFX(__security_check_cookie): + AARCH64_BTI(c) + LDR_LIT(x9, ASM_PFX(__security_cookie)) + cmp x0, x9 + b.ne ASM_PFX(StackCheckFailure) + ret +// MU_CHANGE END: CLANGPDB Stack Cookies diff --git a/MdePkg/Library/StackCheckLib/AArch64/StackCookieInterrupt.S b/MdePkg/Library/StackCheckLib/AArch64/StackCookieInterrupt.S index bce13643e3d..5e83edb56ad 100644 --- a/MdePkg/Library/StackCheckLib/AArch64/StackCookieInterrupt.S +++ b/MdePkg/Library/StackCheckLib/AArch64/StackCookieInterrupt.S @@ -5,17 +5,26 @@ // SPDX-License-Identifier: BSD-2-Clause-Patent //------------------------------------------------------------------------------ +#include // MU_CHANGE: CLANGPDB Stack Cookies + .text //------------------------------------------------------------------------------ -// Calls an interrupt using the vector specified by PcdStackCookieExceptionVector +// MU_CHANGE BEGIN: CLANGPDB Stack Cookies +// Calls an interrupt using the stack cookie exception vector +// and passes the ExceptionAddress through in x0. // // VOID +// EFIAPI // TriggerStackCookieInterrupt ( -// VOID +// EFI_PHYSICAL_ADDRESS ExceptionAddress // ); +// MU_CHANGE END: CLANGPDB Stack Cookies //------------------------------------------------------------------------------ .global ASM_PFX(TriggerStackCookieInterrupt) ASM_PFX(TriggerStackCookieInterrupt): - svc FixedPcdGet8 (PcdStackCookieExceptionVector) +// MU_CHANGE BEGIN: CLANGPDB Stack Cookies + AARCH64_BTI(c) + svc STACK_CHECK_ERROR_HANDLER_VECTOR +// MU_CHANGE END: CLANGPDB Stack Cookies ret diff --git a/MdePkg/Library/StackCheckLib/AArch64/StackCookieInterrupt.asm b/MdePkg/Library/StackCheckLib/AArch64/StackCookieInterrupt.asm deleted file mode 100644 index ff5ee35acce..00000000000 --- a/MdePkg/Library/StackCheckLib/AArch64/StackCookieInterrupt.asm +++ /dev/null @@ -1,25 +0,0 @@ -;------------------------------------------------------------------------------ -; AArch64/StackCookieInterrupt.asm -; -; Copyright (c) Microsoft Corporation. -; SPDX-License-Identifier: BSD-2-Clause-Patent -;------------------------------------------------------------------------------ - - EXPORT TriggerStackCookieInterrupt - - AREA |.text|, CODE, READONLY - -;------------------------------------------------------------------------------ -; Calls an interrupt using the vector specified by PcdStackCookieExceptionVector -; -; VOID -; TriggerStackCookieInterrupt ( -; VOID -; ); -;------------------------------------------------------------------------------ -TriggerStackCookieInterrupt PROC - SVC FixedPcdGet8 (PcdStackCookieExceptionVector) - RET -TriggerStackCookieInterrupt ENDP - - END diff --git a/MdePkg/Library/StackCheckLib/IA32/CheckCookieMsvc.nasm b/MdePkg/Library/StackCheckLib/IA32/CheckCookieMsvc.nasm index a52ee90ad9f..ea772c6fcc8 100644 --- a/MdePkg/Library/StackCheckLib/IA32/CheckCookieMsvc.nasm +++ b/MdePkg/Library/StackCheckLib/IA32/CheckCookieMsvc.nasm @@ -10,20 +10,19 @@ extern ASM_PFX(StackCheckFailure) extern ASM_PFX(__security_cookie) -extern ASM_PFX(CpuDeadLoop) ; Called when a buffer check fails. This functionality is dependent on MSVC ; C runtime libraries and so is unsupported in UEFI. global ASM_PFX(__report_rangecheckfailure) ASM_PFX(__report_rangecheckfailure): - jmp ASM_PFX(CpuDeadLoop) + int 3 ret ; The GS handler is for checking the stack cookie during SEH or ; EH exceptions and is unsupported in UEFI. global ASM_PFX(__GSHandlerCheck) ASM_PFX(__GSHandlerCheck): - jmp ASM_PFX(CpuDeadLoop) + int 3 ret ;------------------------------------------------------------------------------ diff --git a/MdePkg/Library/StackCheckLib/IA32/StackCookieInterrupt.nasm b/MdePkg/Library/StackCheckLib/IA32/StackCookieInterrupt.nasm index 83a686ddb30..59836cd1d9c 100644 --- a/MdePkg/Library/StackCheckLib/IA32/StackCookieInterrupt.nasm +++ b/MdePkg/Library/StackCheckLib/IA32/StackCookieInterrupt.nasm @@ -5,19 +5,28 @@ ; SPDX-License-Identifier: BSD-2-Clause-Patent ;------------------------------------------------------------------------------ +#include ; MU_CHANGE: CLANGPDB Stack Cookies + DEFAULT REL SECTION .text ;------------------------------------------------------------------------------ +; MU_CHANGE BEGIN: CLANGPDB Stack Cookies ; Checks the stack cookie value against __security_cookie and calls the -; stack cookie failure handler if there is a mismatch. +; stack cookie failure handler if there is a mismatch, passing along the +; exception address in ecx. ; ; VOID +; EFIAPI ; TriggerStackCookieInterrupt ( -; VOID +; EFI_PHYSICAL_ADDRESS ExceptionAddress ; ); +; MU_CHANGE END: CLANGPDB Stack Cookies ;------------------------------------------------------------------------------ global ASM_PFX(TriggerStackCookieInterrupt) ASM_PFX(TriggerStackCookieInterrupt): - int FixedPcdGet8 (PcdStackCookieExceptionVector) +; MU_CHANGE BEGIN: CLANGPDB Stack Cookies + mov ecx, [esp + 4] ; first parameter, skipping return address + int STACK_CHECK_ERROR_HANDLER_VECTOR +; MU_CHANGE END: CLANGPDB Stack Cookies ret diff --git a/MdePkg/Library/StackCheckLib/Readme.md b/MdePkg/Library/StackCheckLib/Readme.md index 5569ad2e27e..ade6539f9ef 100644 --- a/MdePkg/Library/StackCheckLib/Readme.md +++ b/MdePkg/Library/StackCheckLib/Readme.md @@ -22,9 +22,11 @@ continuously writing past the stack variables will cause the stack cookie to be overwritten. Before the function returns, the stack cookie value will be checked and if there is a mismatch then `StackCheckLib` handles the failure. -Because UEFI doesn't use the C runtime libraries provided by MSVC, the stack -check code is written in assembly within this library. GCC and Clang compilers -have built-in support for stack cookie checking, so this library only handles failures. +Because UEFI doesn't use toolchain provided C runtime libraries, the stack check code is +written in assembly within this library. The GCC compiler has built-in support +for stack cookie checking, so this library only handles failures. CLANGPDB and MSVC both +use MSVC-style stack cookie checking. Below these toolchains are referred to collectively +as MSVC-style. The stack cookie value is initialized at compile time via updates to the AutoGen process. Each module will define `STACK_COOKIE_VALUE` which is used for the module stack cookie @@ -36,23 +38,31 @@ global cannot be written to such as in execute-in-place (XIP) modules and during temporary RAM phase of the boot process. It is always preferable to use one of the dynamic stack cookie entry points when possible. +This document is intended as a user's guide, see the [theory of operations document](./TheoryOfOperations.md) for a detailed breakdown of functionality. + ### StackCheckLib `StackCheckLib` provides the stack cookie checking functionality per architecture and -toolchain. The currently supported pairs are IA32{GCC,MSVC}, X64{GCC, MSVC}, -and AARCH64{GCC, MSVC}. `StackCheckLib` is agnostic as to whether the stack cookie was -updated during build time or run time; it simply checks the cookie in the MSVC case and -in both GCC and MSVC responds to stack cookie checking failures. +toolchain. The currently supported pairs are IA32{CLANGPDB, GCC,MSVC}, X64{CLANGPDB, GCC, MSVC}, +and AARCH64{CLANGPDB, GCC}. `StackCheckLib` is agnostic as to whether the stack cookie was +updated during build time or run time; it simply checks the cookie in the MSVC-style case and +in both GCC and MSVC-style responds to stack cookie checking failures. To add support for other architectures/toolchains, additional assembly files should be added to `StackCheckLib.inf` and scoped to that architecture/toolchain. -Note: Stack cookie failures generate exceptions and SEC and PEI_CORE may not have -exception handlers registered. In order to safely use stack cookie checking in -these phases, a platform should implement exception handlers because unhandled -exceptions may lead to a hung system state. If a platform does not implement -exception handlers in SEC and PEI_CORE, it is recommended to use `StackCheckLibNull` -for these phases, except for development purposes. +`StackCheckLib` must not have any dependencies. Each toolchain has a different process +when it comes to link time optimization (LTO) and stack cookie generation. Link errors +have been observed on CLANGPDB and GCC when `StackCheckLib` has LTO disabled for itself +and its dependencies have LTO enabled. The linking time is incorrect and the required +symbols are gone. + +>**Note**: Stack cookie failures generate exceptions and SEC and PEI_CORE may not have +> exception handlers registered. In order to safely use stack cookie checking in +> these phases, a platform should implement exception handlers because unhandled +> exceptions may lead to a hung system state. If a platform does not implement +> exception handlers in SEC and PEI_CORE, it is recommended to use `StackCheckLibNull` +> for these phases, except for development purposes. ### DynamicStackCookieEntryPointLib @@ -72,8 +82,8 @@ entry point, so there is no dynamic stack cookie entry point lib there. The dynamic stack cookie entry point lib is used in place of the standard entry point lib, e.g. for UefiDriverEntryPoint to have dynamic stack cookies, a platform would remove -MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf from its DSC and instead -include MdePkg/Library/DynamicStackCookieEntryPointLib/UefiDriverEntryPoint.inf. +`MdePkg/Library/UefiDriverEntryPoint/UefiDriverEntryPoint.inf` from its DSC and instead +include `MdePkg/Library/DynamicStackCookieEntryPointLib/UefiDriverEntryPoint.inf`. See the Usage section for other ways of including these libraries. @@ -86,25 +96,22 @@ added at a future date. `StackCheckLibNull` is an instance of `StackCheckLib` which does not perform any stack cookie checks. This is useful for modules which will fail if stack cookie checks are inserted. Of course, this is not recommended for production code outside of -SEC and PEI_CORE. +SEC and PEI. ## How Failures are Handled -When a stack cookie check fails, the `StackCheckLib` library will first call into a hook -function `StackCheckFailureHook()` which only has a NULL implementation in edk2. -The NULL implementation will simply print the failure address and return, but a platform -can implement their own instance of this library which can perform additional actions -before the system triggers an interrupt. +When a stack cookie check fails, the `StackCheckLib` library will trigger an exception with +ID 0x42. -After `StackCheckFailureHook()` returns, the library will trigger an interrupt with -PcdStackCookieExceptionVector. +Platforms may register interrupt handlers for this exception vector to hook into the +process and perform whatever actions they desire. In lieu of this, the default +exception handlers will print that a stack cookie error occured and the address of +the instruction where it was caught. -- On IA32 and X64 platforms, PcdStackCookieExceptionVector is used as an index into the +- On IA32 and X64 platforms, the Exception Vector ID (0x42) is used as an index into the Interrupt Descriptor Table. - On AARCH64 platforms, a supervisor call (`SVC`) is called with the value -of PcdStackCookieExceptionVector. This value can similarly be retrieved by the -handler to determine if the interrupt was triggered by the stack cookie check. Reference: -[Arm A64 Instruction Set Architecture Version 2024-3](https://developer.arm.com/documentation/ddi0602/2024-03/Base-Instructions/SVC--Supervisor-Call-?lang=en) +of 0x42. ## Debugging Stack Cookie Check Failures @@ -113,7 +120,7 @@ printf debugging to determine which function has an overflow only to find that t disappears on the next boot. This curiosity is usually due to the black-box heuristic used by compilers to determine where to put stack cookie checks or compiler optimization features removing the failing check. The address where the failed stack cookie check occurred will -be printed using DebugLib. If .map files are available, the address combined with the image +be printed by the exception handler. If .map files are available, the address combined with the image offset can be used to determine the function which failed. GNU-based compilers have the `-fstack-protector-all` flag to force stack cookie checks on @@ -127,15 +134,20 @@ checks and are useful for determining where the checks are placed. To generate t append `/FAcs` to the build options for each target module. The easiest way to do this is to update the tools_def file so the `___CC_FLAGS` includes `/FAcs`. +The best way to debug this failure is by attaching a debugger to catch the exception live +and be able to probe the stack, reboot, break in on the faulting function and walk through, +etc. + ## Usage -edk2 updated the tools_def to add `/GS` to VS2022 and VS2019 IA32/X64 builds and -`-fstack-protector` to GCC builds. This will cause stack cookie references to be inserted -throughout the code. Every module should have a `StackCheckLib` instance linked to satisfy -these references. So every module doesn't need to add `StackCheckLib` to the LibraryClasses -section of the INF file, `StackCheckLib` is added as a dependency for each entry point lib. -This means that custom entry point libs need to have StackCheckLib added as a dependency. -The only exception to this is MSVC built host-based unit tests as they will be +CLANGPDB, GCC, and MSVC have stack cookies enabled by default. This will cause stack cookie +references to be inserted throughout the code. Every module should have a `StackCheckLib` +instance linked to satisfy these references. So every module doesn't need to add +`StackCheckLib` to the LibraryClasses section of the INF file, `StackCheckLib` is added as +a dependency for each entry point lib. This means that custom entry point libs need to have +StackCheckLib added as a dependency. + +The only exception to this is MSVC-style built host-based unit tests as they will be compiled with the runtime libraries which already contain the stack cookie definitions and will collide with `StackCheckLib`. A `StackCheckLibHostApplication.inf` is linked by `UnitTestFrameworkPkg/UnitTestFrameworkPkgHost.dsc.inc` that provides the stack @@ -208,6 +220,11 @@ Platforms should include this separately, e.g.: StandaloneMmCoreEntryPoint|MdePkg/Library/DynamicStackCookieEntryPointLib/StandaloneMmCoreEntryPoint.inf ``` +> **Note:** Dynamic stack cookies on AArch64 depend on a multiple of 4KB section alignment because we must use ADRP/LDR +> to get the address of the stack cookie to update. CLANGPDB defines all modules to have 4KB section alignment, +> so this is not a concern there. GCC has 4KB section alignment for DXE and Standalone MM, which is where dynamic stack +> cookies may be used. Platforms must maintain this (or a multiple of 4KB) in order to use dynamic stack cookies. + Platforms then must remove any references to these entry point libs in their DSC, so that the `MdeLibs.dsc.inc` versions are chosen. Alternatively, for better DSC readability, a platform can directly reference the dynamic stack cookie entry points. diff --git a/MdePkg/Library/StackCheckLib/StackCheckLib.inf b/MdePkg/Library/StackCheckLib/StackCheckLib.inf index 8ecb9b638ec..e8aa0d6bdba 100644 --- a/MdePkg/Library/StackCheckLib/StackCheckLib.inf +++ b/MdePkg/Library/StackCheckLib/StackCheckLib.inf @@ -14,39 +14,63 @@ LIBRARY_CLASS = StackCheckLib [Sources] - StackCheckLibCommonMsvc.c | MSFT - StackCheckLibCommonGcc.c | GCC + StackCheckLibCommonMsvc.c ||||gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking # MU_CHANGE: CLANGPDB Stack Cookies + StackCheckLibCommonGcc.c ||||!gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking # MU_CHANGE: CLANGPDB Stack Cookies [Sources.IA32] - IA32/CheckCookieMsvc.nasm | MSFT + IA32/CheckCookieMsvc.nasm ||||gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking # MU_CHANGE: CLANGPDB Stack Cookies + IA32/StackCookieInterrupt.nasm # MU_CHANGE: CLANGPDB Stack Cookies [Sources.X64] - X64/CheckCookieMsvc.nasm | MSFT + X64/CheckCookieMsvc.nasm ||||gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking # MU_CHANGE: CLANGPDB Stack Cookies + X64/StackCookieInterrupt.nasm # MU_CHANGE: CLANGPDB Stack Cookies -[Sources.IA32, Sources.X64] - IA32/StackCookieInterrupt.nasm +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies - removed shared [Sources.IA32, Sources.X64] section +# [Sources.IA32, Sources.X64] +# IA32/StackCookieInterrupt.nasm +# MU_CHANGE END: CLANGPDB Stack Cookies [Sources.AARCH64] - AArch64/StackCookieInterrupt.S |GCC - AArch64/StackCookieInterrupt.asm |MSFT + AArch64/StackCookieInterrupt.S # MU_CHANGE: CLANGPDB Stack Cookies + AArch64/CheckCookieMsvc.S ||||gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking # MU_CHANGE: CLANGPDB Stack Cookies + # MU_CHANGE BEGIN: CLANGPDB Stack Cookies - removed MSVC AArch64 file + # AArch64/StackCookieInterrupt.asm |MSFT + # MU_CHANGE END: CLANGPDB Stack Cookies + +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies +[LibraryClasses] + # StackCheckLib must not have any dependencies due to the unique nature of when compilers may + # insert stack cookies and link this library. See the README for more details. +# MU_CHANGE END: CLANGPDB Stack Cookies [Packages] MdePkg/MdePkg.dec -[LibraryClasses] - StackCheckFailureHookLib - -[FixedPcd] - gEfiMdePkgTokenSpaceGuid.PcdStackCookieExceptionVector - -[BuildOptions] - # We cannot build the MSVC version with /GL (whole program optimization) because we run into linker error - # LNK1237, which is a failure to link against a symbol from a library compiled with /GL. The whole program - # optimization tries to do away with references to this symbol. The solution is to not compile the stack - # check libs with /GL - MSFT:*_*_*_CC_FLAGS = /GL- - - # We cannot build the GCC version with LTO (link time optimization) because we run into linker errors where - # the stack cookie variable has been optimized away, as it looks to GCC like the variable is not used, because - # the compiler inserts the usage. - GCC:*_*_*_CC_FLAGS = -fno-lto +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies - removed LibraryClasses section +# [LibraryClasses] +# StackCheckFailureHookLib +# MU_CHANGE END: CLANGPDB Stack Cookies + +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies - removed FixedPcd section +# [FixedPcd] +# gEfiMdePkgTokenSpaceGuid.PcdStackCookieExceptionVector +# MU_CHANGE END: CLANGPDB Stack Cookies + +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies - removed BuildOptions section +# [BuildOptions] +# # We cannot build the MSVC version with /GL (whole program optimization) because we run into linker error +# # LNK1237, which is a failure to link against a symbol from a library compiled with /GL. The whole program +# # optimization tries to do away with references to this symbol. The solution is to not compile the stack +# # check libs with /GL +# MSFT:*_*_*_CC_FLAGS = /GL- +# +# # We cannot build the GCC version with LTO (link time optimization) because we run into linker errors where +# # the stack cookie variable has been optimized away, as it looks to GCC like the variable is not used, because +# # the compiler inserts the usage. +# GCC:*_*_*_CC_FLAGS = -fno-lto +# MU_CHANGE END: CLANGPDB Stack Cookies + +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies +[FeaturePcd] + gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking +# MU_CHANGE END: CLANGPDB Stack Cookies diff --git a/MdePkg/Library/StackCheckLib/StackCheckLibCommonGcc.c b/MdePkg/Library/StackCheckLib/StackCheckLibCommonGcc.c index 7157e0dfe77..d40495e0378 100644 --- a/MdePkg/Library/StackCheckLib/StackCheckLibCommonGcc.c +++ b/MdePkg/Library/StackCheckLib/StackCheckLibCommonGcc.c @@ -7,19 +7,29 @@ **/ #include +#include // MU_CHANGE: CLANGPDB Stack Cookies -#include -#include +// MU_CHANGE BEGIN: CLANGPDB Stack Cookies - removed DebugLib dependency +// #include +// MU_CHANGE END: CLANGPDB Stack Cookies +// MU_CHANGE BEGIN: CLANGPDB Stack Cookies - removed BaseLib dependency +// #include +// MU_CHANGE END: CLANGPDB Stack Cookies #include -#include +// MU_CHANGE BEGIN: CLANGPDB Stack Cookies - removed StackCheckFailureHookLib dependency +// #include +// MU_CHANGE END: CLANGPDB Stack Cookies +// MU_CHANGE BEGIN: CLANGPDB Stack Cookies /** - Triggers an interrupt using the vector specified by PcdStackCookieExceptionVector + Triggers an interrupt using the stack cookie exception vector. **/ VOID +EFIAPI TriggerStackCookieInterrupt ( - VOID + EFI_PHYSICAL_ADDRESS ExceptionAddress ); +// MU_CHANGE END: CLANGPDB Stack Cookies VOID *__stack_chk_guard = (VOID *)(UINTN)STACK_COOKIE_VALUE; @@ -34,7 +44,11 @@ __stack_chk_fail ( VOID ) { - DEBUG ((DEBUG_ERROR, "Stack cookie check failed at address 0x%llx!\n", RETURN_ADDRESS (0))); - StackCheckFailureHook (RETURN_ADDRESS (0)); - TriggerStackCookieInterrupt (); + // MU_CHANGE BEGIN: CLANGPDB Stack Cookies - removed DebugLib dependency + // DEBUG ((DEBUG_ERROR, "Stack cookie check failed at address 0x%llx!\n", RETURN_ADDRESS (0))); + // MU_CHANGE END: CLANGPDB Stack Cookies + // MU_CHANGE BEGIN: CLANGPDB Stack Cookies - removed StackCheckFailureHookLib dependency + // StackCheckFailureHook (RETURN_ADDRESS (0)); + // MU_CHANGE END: CLANGPDB Stack Cookies + TriggerStackCookieInterrupt ((EFI_PHYSICAL_ADDRESS)(UINTN)RETURN_ADDRESS (0)); // MU_CHANGE: CLANGPDB Stack Cookies } diff --git a/MdePkg/Library/StackCheckLib/StackCheckLibCommonMsvc.c b/MdePkg/Library/StackCheckLib/StackCheckLibCommonMsvc.c index d9018ed09eb..b0e9b553488 100644 --- a/MdePkg/Library/StackCheckLib/StackCheckLibCommonMsvc.c +++ b/MdePkg/Library/StackCheckLib/StackCheckLibCommonMsvc.c @@ -8,18 +8,28 @@ #include -#include -#include +// MU_CHANGE BEGIN: CLANGPDB Stack Cookies - removed DebugLib dependency +// #include +// MU_CHANGE END: CLANGPDB Stack Cookies +// MU_CHANGE BEGIN: CLANGPDB Stack Cookies - removed BaseLib dependency +// #include +// MU_CHANGE END: CLANGPDB Stack Cookies +#include // MU_CHANGE: CLANGPDB Stack Cookies #include -#include +// MU_CHANGE BEGIN: CLANGPDB Stack Cookies - removed StackCheckFailureHookLib dependency +// #include +// MU_CHANGE END: CLANGPDB Stack Cookies +// MU_CHANGE BEGIN: CLANGPDB Stack Cookies /** - Triggers an interrupt using the vector specified by PcdStackCookieExceptionVector + Triggers an interrupt using the stack cookie exception vector. **/ VOID +EFIAPI TriggerStackCookieInterrupt ( - VOID + EFI_PHYSICAL_ADDRESS ExceptionAddress ); +// MU_CHANGE END: CLANGPDB Stack Cookies VOID *__security_cookie = (VOID *)(UINTN)STACK_COOKIE_VALUE; @@ -31,11 +41,16 @@ VOID *__security_cookie = (VOID *)(UINTN)STACK_COOKIE_VALUE; **/ VOID +EFIAPI // MU_CHANGE: CLANGPDB Stack Cookies StackCheckFailure ( VOID *ActualCookieValue ) { - DEBUG ((DEBUG_ERROR, "Stack cookie check failed at address 0x%llx!\n", RETURN_ADDRESS (0))); - StackCheckFailureHook (RETURN_ADDRESS (0)); - TriggerStackCookieInterrupt (); + // MU_CHANGE BEGIN: CLANGPDB Stack Cookies - removed DebugLib dependency + // DEBUG ((DEBUG_ERROR, "Stack cookie check failed at address 0x%llx!\n", RETURN_ADDRESS (0))); + // MU_CHANGE END: CLANGPDB Stack Cookies + // MU_CHANGE BEGIN: CLANGPDB Stack Cookies - removed StackCheckFailureHookLib dependency + // StackCheckFailureHook (RETURN_ADDRESS (0)); + // MU_CHANGE END: CLANGPDB Stack Cookies + TriggerStackCookieInterrupt ((EFI_PHYSICAL_ADDRESS)(UINTN)RETURN_ADDRESS (0)); // MU_CHANGE: CLANGPDB Stack Cookies } diff --git a/MdePkg/Library/StackCheckLib/TheoryOfOperations.md b/MdePkg/Library/StackCheckLib/TheoryOfOperations.md new file mode 100644 index 00000000000..19e1810dda8 --- /dev/null +++ b/MdePkg/Library/StackCheckLib/TheoryOfOperations.md @@ -0,0 +1,250 @@ +# Stack Cookie Theory of Operations + +This document details the stack cookie theory of operations in edk2. + +## Overview + +Stack cookies are native width values inserted by the toolchain before the return address +on the stack. They are intended to catch certain classes of overflow attacks as well as +catching certain classes of overflow mistakes. + +The toolchain inserts the cookie and checking functions on each function it determines may +need them (generally because they contain arrays on the stack or flags have been passed to +protect all functions). On return from the function, the stack cookie checking function is +called to validate the stack cookie maintains the same value as before the function was +invoked. + +```text +|-------------------------------| +| Previous Frame | +|-------------------------------| +| Return Address | +| Stack Cookie | +| Locals | +|-------------------------------| +``` + +In this way, if a local array, e.g., is overflowed, the stack cookie must be corrupted in +order for the return address to be corrupted and it can be caught at run time. + +## MSVC-style vs GCC-style Stack Cookies + +There are two predominant forms of stack cookie checking, MSVC-style and GCC-style. MSVC and +CLANGPDB use the former, while GCC and CLANGDWARF use the latter. StackCheckLib supports +<<<<<<< HEAD +both styles, though at the time of writing support is enabled for MSVC and GCC only. +======= +both styles, though at the time of writing support is enabled for MSVC, GCC, and CLANGPDB. +>>>>>>> 55039a1845 (MdePkg: StackCheckLib: Update Theory of Operations) + +### MSVC-style Stack Cookies + +MSVC-style stack cookies use the following APIset: + +```c +// The __security_cookie is a random value placed on the stack between the stack variables +// and the return address so that continuously writing past the stack variables will cause +// the stack cookie to be overwritten. Before the function returns, the stack cookie value +// will be checked and if there is a mismatch then StackCheckLib handles the failure. +extern VOID *__security_cookie; + +/** + Called when a buffer check fails. This functionality is dependent on MSVC + C runtime libraries and so is unsupported in UEFI. + +**/ +VOID +EFIAPI +__report_rangecheckfailure ( + VOID + ); + +/** + The GS handler is for checking the stack cookie during SEH or + EH exceptions and is unsupported in UEFI. + +**/ +VOID +EFIAPI +__GSHandlerCheck ( + VOID + ); + +/** + Checks the stack cookie value against __security_cookie and calls the + stack cookie failure handler if there is a mismatch. + + @param UINTN CheckValue The value to check against __security_cookie + +**/ +VOID +EFIAPI +__security_check_cookie ( + UINTN CheckValue + ); +``` + +`__report_rangecheckfailure()` and `__GSHandlerCheck()` are not invoked in the UEFI context, +but MSVC can still include references to them in the binary, so they are included. + +UEFI populates `__security_cookie` at build time (and optionally at runtime, see [dynamic cookies](#dynamic-cookies)). +The toolchain then inserts this value before the return value on certain functions as described. Upon return from these +functions, `__security_check_cookie()` is called. StackCheckLib will compare against the +expected stack cookie and if the value matches, it will return (returning back to last stack frame) if it fails it will +trigger a [stack cookie failure](#stack-cookie-failures). + +### GCC-style Stack Cookies + +GCC-style stack cookies use the following APIset: + +```c +// The __stack_chk_guard is a random value placed on the stack between the stack variables +// and the return address so that continuously writing past the stack variables will cause +// the stack cookie to be overwritten. Before the function returns, the stack cookie value +// will be checked and if there is a mismatch then StackCheckLib handles the failure. +extern VOID *__stack_chk_guard; + +/** + Called when a stack cookie check fails. The return address is the failing address. + +**/ +VOID +EFIAPI +__stack_chk_fail ( + VOID + ); +``` + +As with MSVC-style checking, `__stack_chk_guard` is the stack cookie value, populating by UEFI at build time (and +optionally [runtime](#dynamic-cookies)). GCC-style stack cookies differ from MSVC-style because the stack cookie +checking code is automatically generated by the toolchain. Only if a [failure](#stack-cookie-failures) occurs is +StackCheckLib invoked, through `__stack_chk_fail()`. + +## Cookie Generation + +At build time, a random static stack cookie value is generated by BaseTools and inserted in `Autogen.h` as +`STACK_COOKIE_VALUE`. StackCheckLib then uses this define to set the static stack cookie value. + +### Dynamic Cookies + +In DXE and MM, dynamic stack cookies can be generated by the entry point libs under +`MdePkg/DynamicStackCookieEntryPointLib/`. Platforms may selectively use any or all of these dynamic entry point libs +to use dynamic stack cookies in some modules and not in others. + +#### IA32/X64 + +On the x86 family of systems, the `RDRAND` instruction is used to generate a random number to use for each module's +stack cookie value dynamically at runtime. + +The sequence of events is: + +- `_ModuleEntryPoint()` is jumped to from the core. +- `RDRAND` support is queried from CPUID leaf 1 +- If `RDRAND` is not supported, the static stack cookie is retained and execution continues in the module +- If `RDRAND` is supported, it is called to generate a random number. If it fails, the static stack cookie is retained + and execution continues in the module +- If `RDRAND` succeeds, the stack cookie is updated with the random number + +#### ARM64 + +For ARM64, the flow is the same as for x86, but `RNDR` is used and support is queried from `ID_AA64ISAR0_EL1`. + +## Platform Configuration + +This section discusses the architecture of how platforms consume this feature; see the [README](./Readme.md) for +platform integration instructions. + +The supported toolchains build modules with stack cookies enabled. That means all modules must link against an instance +of StackCheckLib to satisfy the dependencies. + +The entry point libs (whether static or dynamic) and SEC modules have StackCheckLib as an explicit dependency. This +forces the library to be linked to all modules without having to resort to a `NULL` lib. + +`MdeLibs.dsc.inc` is used to provide `StackCheckLibNull` as the instance for all modules if they do not opt into a +different mechanism. This means that enabling/disabling stack cookie checking is not a breaking change for platforms. + +### StackCheckLibNull + +`StackCheckLibNull` is an instance of `StackCheckLib` that satisfies the symbol dependencies that must be filled with +stack cookies enabled in the toolchain, but it does no stack cookie checking or failures; it simply returns, which lets +execution continue in the event of a failure. This is the same behavior as if stack cookie checking was disabled in the +toolchain: errors are silently ignored. + +### StackCheckLib + +`StackCheckLib` is the library that implements actual stack cookie checking and error generation per toolchain type. +Platforms may use this implementation with either static or dynamic cookies, the checking logic is independent of how +the stack cookie was generated. + +### Stack Cookie Failures + +If a module has opted into stack cookie checking (by using the `StackCheckLib` library instance), stack cookie failures +will have the same behavior on all architectures. + +<<<<<<< HEAD +1. The address of the function that corrupted the stack cookie is printed +2. `StackCheckFailureHookLib` is called to allow platforms to implement any behavior they want on a failure +3. `PcdStackCookieExceptionVector` is signaled to cause a fault (on x86 this is through `int` on ARM64 this is through + `svc`) + +This allows a platform to do specific actions they deem relevant, such as log to an external source (e.g. SEL) or +resetting the system. When the exception is taken, the exception handlers will catch it and hang the system or handle +it in a software debugger, if attached. + +Developers can then debug from this point. See debugging tips in the [README](./Readme.md). +======= +1. The address of the function that corrupted the stack cookie is saved in a register or via the stack + a. On IA32, the stack is used to pass the address. On X64, `rcx` is used. On ARM64, `x0` is used. +2. Interrupt 0x42 is signaled to cause a fault (on x86 this is through `int` on ARM64 this is through + `svc`) + +Platforms may hook into this by registering an exception handler for vector 0x42 or overriding the exception handler +library. This allows a platform to do specific actions they deem relevant, such as log to an external source (e.g. SEL), +reset the system, or ignore the stack cookie violation. + +If the exception is taken by the default exception handlers, they will print out a stack cookie exception occurred at +the given address and hang the system. Alternatively, if a software debugger is attached, it will catch the exception, +allowing it be be debugged. Developers can then debug from this point. See debugging tips in the [README](./Readme.md). +>>>>>>> 55039a1845 (MdePkg: StackCheckLib: Update Theory of Operations) + +## Linking Considerations + +The stack cookie symbols are unique in that the toolchain inserts them while linking a binary, generally fairly late +<<<<<<< HEAD +into the process, when it can decide which functions will need stack cookie checking. As such, toolchains must build +`StackCheckLib` with LTO disabled, because LTO occurs before the stack cookie symbol/function references are added; +the toolchain believes these symbols are unused, even though it will insert the references later on. +======= +into the process, when it can decide which functions will need stack cookie checking. + +One option to solve this issue is to disable LTO for `StackCheckLib` instances. However, this creates additional +complexity with attempting to link LTO objects with non-LTO objects. + +Instead, `MdeLibs.dsc.inc` is used to forcibly include the symbols for every module (using `/INCLUDE` and +`-Wl,--undefined`). This allows `StackCheckLib` instances to be compiled with LTO but still retain the symbols that +will be needed. +>>>>>>> 55039a1845 (MdePkg: StackCheckLib: Update Theory of Operations) + +## Host Applications + +There are two classes of Host Applications that are considered here: Host-Based Unit Tests and EmulatorPkg. + +### Host-Based Unit Tests + +edk2 runs host unit tests with ASAN enabled. This is a better buffer overflow checking strategy than stack cookies and +as such it is recommended that stack cookies are disabled and ASAN used in tests instead. + +However, as stack cookies were enabled in edk2 before ASAN was, a `StackCheckLibNullHostApplication` library instance +exists which allows for running host-based unit tests with stack cookies enabled. + +On Windows, edk2 links against a C runtime that provides stack cookie checking functionality. As such, the MSVC-style +stack cookie support here is a no-op and lets the C runtime handle it. + +On Linux, edk2 does not link against a C runtime that provides stack cookie checking functionality. As such, the +GCC-style stack cookie checking is the same as in `StackCheckLib`. + +### EmulatorPkg + +EmulatorPkg is a host-based application that emulates a physical machine to boot edk2 on. It does not use ASAN. As such, +the `StackCheckLibNullHostApplication` library instance is used on the emulator piece of EmulatorPkg and the firmware +pieces may use either `StackCheckLib` or `StackCheckLibNull`. diff --git a/MdePkg/Library/StackCheckLib/X64/CheckCookieMsvc.nasm b/MdePkg/Library/StackCheckLib/X64/CheckCookieMsvc.nasm index ebc4f75712e..454f663a582 100644 --- a/MdePkg/Library/StackCheckLib/X64/CheckCookieMsvc.nasm +++ b/MdePkg/Library/StackCheckLib/X64/CheckCookieMsvc.nasm @@ -10,20 +10,19 @@ extern ASM_PFX(StackCheckFailure) extern ASM_PFX(__security_cookie) -extern ASM_PFX(CpuDeadLoop) ; Called when a buffer check fails. This functionality is dependent on MSVC ; C runtime libraries and so is unsupported in UEFI. global ASM_PFX(__report_rangecheckfailure) ASM_PFX(__report_rangecheckfailure): - jmp ASM_PFX(CpuDeadLoop) + int 3 ret ; The GS handler is for checking the stack cookie during SEH or ; EH exceptions and is unsupported in UEFI. global ASM_PFX(__GSHandlerCheck) ASM_PFX(__GSHandlerCheck): - jmp ASM_PFX(CpuDeadLoop) + int 3 ret ;------------------------------------------------------------------------------ diff --git a/MdePkg/Library/StackCheckLib/X64/StackCookieInterrupt.nasm b/MdePkg/Library/StackCheckLib/X64/StackCookieInterrupt.nasm new file mode 100644 index 00000000000..52d87e2df76 --- /dev/null +++ b/MdePkg/Library/StackCheckLib/X64/StackCookieInterrupt.nasm @@ -0,0 +1,29 @@ +; MU_CHANGE BEGIN: CLANGPDB Stack Cookies - new file +;------------------------------------------------------------------------------ +; X64/StackCookieInterrupt.nasm +; +; Copyright (c) Microsoft Corporation. +; SPDX-License-Identifier: BSD-2-Clause-Patent +;------------------------------------------------------------------------------ + +#include + + DEFAULT REL + SECTION .text + +;------------------------------------------------------------------------------ +; Checks the stack cookie value against __security_cookie and calls the +; stack cookie failure handler if there is a mismatch, passing along the +; exception address in rcx. +; +; VOID +; EFIAPI +; TriggerStackCookieInterrupt ( +; EFI_PHYSICAL_ADDRESS ExceptionAddress +; ); +;------------------------------------------------------------------------------ +global ASM_PFX(TriggerStackCookieInterrupt) +ASM_PFX(TriggerStackCookieInterrupt): + int STACK_CHECK_ERROR_HANDLER_VECTOR + ret +; MU_CHANGE END: CLANGPDB Stack Cookies diff --git a/MdePkg/Library/StackCheckLibNull/AArch64/StackCheckFunctionsMsvc.S b/MdePkg/Library/StackCheckLibNull/AArch64/StackCheckFunctionsMsvc.S new file mode 100644 index 00000000000..79fae394bfc --- /dev/null +++ b/MdePkg/Library/StackCheckLibNull/AArch64/StackCheckFunctionsMsvc.S @@ -0,0 +1,22 @@ +// MU_CHANGE BEGIN: CLANGPDB Stack Cookies - new file +//------------------------------------------------------------------------------ +// AArch64/StackCheckFunctionsMsvc.S +// +// Copyright (c) Microsoft Corporation. +// SPDX-License-Identifier: BSD-2-Clause-Patent +//------------------------------------------------------------------------------ + + .text + +.global ASM_PFX(__report_rangecheckfailure) +ASM_PFX(__report_rangecheckfailure): + ret + +.global ASM_PFX(__GSHandlerCheck) +ASM_PFX(__GSHandlerCheck): + ret + +.global ASM_PFX(__security_check_cookie) +ASM_PFX(__security_check_cookie): + ret +// MU_CHANGE END: CLANGPDB Stack Cookies diff --git a/MdePkg/Library/StackCheckLibNull/IA32/StackCheckFunctionsMsvc.nasm b/MdePkg/Library/StackCheckLibNull/IA32/StackCheckFunctionsMsvc.nasm index 510d500847a..77fa3885746 100644 --- a/MdePkg/Library/StackCheckLibNull/IA32/StackCheckFunctionsMsvc.nasm +++ b/MdePkg/Library/StackCheckLibNull/IA32/StackCheckFunctionsMsvc.nasm @@ -5,7 +5,9 @@ ; SPDX-License-Identifier: BSD-2-Clause-Patent ;------------------------------------------------------------------------------ - DEFAULT REL +; MU_CHANGE BEGIN: CLANGPDB Stack Cookies - removed DEFAULT REL +; DEFAULT REL +; MU_CHANGE END: CLANGPDB Stack Cookies SECTION .text global ASM_PFX(__report_rangecheckfailure) diff --git a/MdePkg/Library/StackCheckLibNull/StackCheckLibHostApplicationMsvc.c b/MdePkg/Library/StackCheckLibNull/StackCheckLibHostApplicationMsvc.c index 6af9891c390..711377714fa 100644 --- a/MdePkg/Library/StackCheckLibNull/StackCheckLibHostApplicationMsvc.c +++ b/MdePkg/Library/StackCheckLibNull/StackCheckLibHostApplicationMsvc.c @@ -11,3 +11,20 @@ #include extern VOID *__security_cookie; + +// MU_CHANGE BEGIN: CLANGPDB Stack Cookies +/** + This function gets called when an MSVC generated stack cookie fails. This implementation calls into a platform + failure hook lib and then triggers the stack cookie interrupt. + + @param[in] ActualCookieValue The value that was written onto the stack, corrupting the stack cookie. + +**/ +VOID +EFIAPI +StackCheckFailure ( + VOID *ActualCookieValue + ) +{ +} +// MU_CHANGE END: CLANGPDB Stack Cookies diff --git a/MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf b/MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf index bb42833d02d..7ef73f1bec5 100644 --- a/MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf +++ b/MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf @@ -15,27 +15,39 @@ VERSION_STRING = 1.0 LIBRARY_CLASS = StackCheckLib +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies [Sources] - StackCheckLibNullGcc.c | GCC - StackCheckLibNullMsvc.c | MSFT + StackCheckLibNullGcc.c ||||!gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking + StackCheckLibNullMsvc.c ||||gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking [Sources.IA32] - IA32/StackCheckFunctionsMsvc.nasm | MSFT + IA32/StackCheckFunctionsMsvc.nasm ||||gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking [Sources.X64] - X64/StackCheckFunctionsMsvc.nasm | MSFT + X64/StackCheckFunctionsMsvc.nasm ||||gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking + +[Sources.AARCH64] + AArch64/StackCheckFunctionsMsvc.S ||||gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking +# MU_CHANGE END: CLANGPDB Stack Cookies [Packages] MdePkg/MdePkg.dec -[BuildOptions] - # We cannot build the MSVC version with /GL (whole program optimization) because we run into linker error - # LNK1237, which is a failure to link against a symbol from a library compiled with /GL. The whole program - # optimization tries to do away with references to this symbol. The solution is to not compile the stack - # check libs with /GL - MSFT:*_*_*_CC_FLAGS = /GL- - - # We cannot build the GCC version with LTO (link time optimization) because we run into linker errors where - # the stack cookie variable has been optimized away, as it looks to GCC like the variable is not used, because - # the compiler inserts the usage. - GCC:*_*_*_CC_FLAGS = -fno-lto +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies - removed BuildOptions section +# [BuildOptions] +# # We cannot build the MSVC version with /GL (whole program optimization) because we run into linker error +# # LNK1237, which is a failure to link against a symbol from a library compiled with /GL. The whole program +# # optimization tries to do away with references to this symbol. The solution is to not compile the stack +# # check libs with /GL +# MSFT:*_*_*_CC_FLAGS = /GL- +# +# # We cannot build the GCC version with LTO (link time optimization) because we run into linker errors where +# # the stack cookie variable has been optimized away, as it looks to GCC like the variable is not used, because +# # the compiler inserts the usage. +# GCC:*_*_*_CC_FLAGS = -fno-lto +# MU_CHANGE END: CLANGPDB Stack Cookies + +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies +[FeaturePcd] + gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking +# MU_CHANGE END: CLANGPDB Stack Cookies diff --git a/MdePkg/Library/StackCheckLibNull/StackCheckLibNullHostApplication.inf b/MdePkg/Library/StackCheckLibNull/StackCheckLibNullHostApplication.inf index 3e898263c2a..cd27c7e4bd4 100644 --- a/MdePkg/Library/StackCheckLibNull/StackCheckLibNullHostApplication.inf +++ b/MdePkg/Library/StackCheckLibNull/StackCheckLibNullHostApplication.inf @@ -20,15 +20,24 @@ VERSION_STRING = 1.0 LIBRARY_CLASS = StackCheckLib|HOST_APPLICATION +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies [Sources] - StackCheckLibHostApplicationMsvc.c | MSFT - StackCheckLibNullGcc.c | GCC + StackCheckLibHostApplicationMsvc.c ||||gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking + StackCheckLibNullGcc.c ||||!gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking +# MU_CHANGE END: CLANGPDB Stack Cookies [Packages] MdePkg/MdePkg.dec -[BuildOptions] - # We cannot build the GCC version with LTO (link time optimization) because we run into linker errors where - # the stack cookie variable has been optimized away, as it looks to GCC like the variable is not used, because - # the compiler inserts the usage. We do not worry about the MSVC version here as it is a no-op. - GCC:*_*_*_CC_FLAGS = -fno-lto +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies - removed BuildOptions section +# [BuildOptions] +# # We cannot build the GCC version with LTO (link time optimization) because we run into linker errors where +# # the stack cookie variable has been optimized away, as it looks to GCC like the variable is not used, because +# # the compiler inserts the usage. We do not worry about the MSVC version here as it is a no-op. +# GCC:*_GCC_*_CC_FLAGS = -fno-lto +# MU_CHANGE END: CLANGPDB Stack Cookies + +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies +[FeaturePcd] + gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking +# MU_CHANGE END: CLANGPDB Stack Cookies diff --git a/MdePkg/Library/StackCheckLibNull/StackCheckLibNullMsvc.c b/MdePkg/Library/StackCheckLibNull/StackCheckLibNullMsvc.c index ebc2ba21d31..19ded757c86 100644 --- a/MdePkg/Library/StackCheckLibNull/StackCheckLibNullMsvc.c +++ b/MdePkg/Library/StackCheckLibNull/StackCheckLibNullMsvc.c @@ -9,3 +9,20 @@ #include VOID *__security_cookie = (VOID *)(UINTN)0x0; + +// MU_CHANGE BEGIN: CLANGPDB Stack Cookies +/** + This function gets called when an MSVC generated stack cookie fails. This implementation calls into a platform + failure hook lib and then triggers the stack cookie interrupt. + + @param[in] ActualCookieValue The value that was written onto the stack, corrupting the stack cookie. + +**/ +VOID +EFIAPI +StackCheckFailure ( + VOID *ActualCookieValue + ) +{ +} +// MU_CHANGE END: CLANGPDB Stack Cookies diff --git a/MdePkg/MdeLibs.dsc.inc b/MdePkg/MdeLibs.dsc.inc index b063480df72..df0ce8f84f7 100644 --- a/MdePkg/MdeLibs.dsc.inc +++ b/MdePkg/MdeLibs.dsc.inc @@ -28,7 +28,9 @@ SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf SynchronizationLib|MdePkg/Library/BaseSynchronizationLib/BaseSynchronizationLib.inf MmUnblockMemoryLib|MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf - StackCheckFailureHookLib|MdePkg/Library/StackCheckFailureHookLibNull/StackCheckFailureHookLibNull.inf + # MU_CHANGE BEGIN: CLANGPDB Stack Cookies - removed StackCheckFailureHookLib + # StackCheckFailureHookLib|MdePkg/Library/StackCheckFailureHookLibNull/StackCheckFailureHookLibNull.inf + # MU_CHANGE END: CLANGPDB Stack Cookies FvLib|MdePkg/Library/FvLib/FvLib.inf !if $(CUSTOM_STACK_CHECK_LIB) == STATIC @@ -80,3 +82,25 @@ # definitions for the intrinsic functions. # NULL|MdePkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf + +# MU_CHANGE BEGIN: CLANGPDB Stack Cookies +[PcdsFeatureFlag] + !if "$(FAMILY)" == "MSFT" || "$(TOOL_CHAIN_TAG)" == "CLANGPDB" + # By default, GCC style stack checking is used. MSFT FAMILY and CLANGPDB need to set this to MSVC style stack + # checking. + gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking|TRUE + !endif + +[BuildOptions] + # + # Linker flags to ensure that stack cookie symbols are not optimized out when StackCheckLib is included. + # + MSFT:*_*_IA32_DLINK_FLAGS = /INCLUDE:___security_cookie + MSFT:*_*_X64_DLINK_FLAGS = /INCLUDE:__security_cookie + + GCC:*_CLANGPDB_IA32_DLINK_FLAGS = /INCLUDE:___security_cookie /INCLUDE:_StackCheckFailure + GCC:*_CLANGPDB_X64_DLINK_FLAGS = /INCLUDE:__security_cookie /INCLUDE:StackCheckFailure + GCC:*_CLANGPDB_AARCH64_DLINK_FLAGS = /INCLUDE:__security_cookie /INCLUDE:StackCheckFailure + + GCC:*_GCC_*_DLINK_FLAGS = -Wl,--undefined=__stack_chk_fail,--undefined=__stack_chk_guard +# MU_CHANGE END: CLANGPDB Stack Cookies diff --git a/MdePkg/MdePkg.ci.yaml b/MdePkg/MdePkg.ci.yaml index b0172b40c55..e62198e4e2c 100644 --- a/MdePkg/MdePkg.ci.yaml +++ b/MdePkg/MdePkg.ci.yaml @@ -128,7 +128,9 @@ ## options defined ci/Plugin/HostUnitTestDscCompleteCheck "HostUnitTestDscCompleteCheck": { - "IgnoreInf": [""], + "IgnoreInf": [ + "MdePkg/Test/GoogleTest/Library/StackCheckLib/GoogleTestStackCheckLib.inf" # Only tested on GCC + ], "DscPath": "Test/MdePkgHostTest.dsc" }, diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index 1da9ea40271..18111b4584d 100644 --- a/MdePkg/MdePkg.dec +++ b/MdePkg/MdePkg.dec @@ -308,9 +308,11 @@ # TraceHubDebugSysTLib|Include/Library/TraceHubDebugSysTLib.h - ## @libraryclass Provides a hook called when a stack cookie check fails. - # - StackCheckFailureHookLib|Include/Library/StackCheckFailureHookLib.h + # MU_CHANGE BEGIN: CLANGPDB Stack Cookies - removed StackCheckFailureHookLib + # ## @libraryclass Provides a hook called when a stack cookie check fails. + # # + # StackCheckFailureHookLib|Include/Library/StackCheckFailureHookLib.h + # MU_CHANGE END: CLANGPDB Stack Cookies ## @libraryclass Provides stack cookie checking functionality # @@ -2109,6 +2111,14 @@ # @Prompt Validate ORDERED_COLLECTION structure gEfiMdePkgTokenSpaceGuid.PcdValidateOrderedCollection|FALSE|BOOLEAN|0x0000002a + # MU_CHANGE BEGIN: CLANGPDB Stack Cookies + ## Enable the use of MSVC style stack checking in StackCheckLib. By default, this is FALSE, + # which means that StackCheckLib will use GCC style stack checking. MSFT FAMILY and CLANGPDB + # builds are expected to set this to TRUE, while the rest of the GCC FAMILY will leave this as FALSE. + # @Prompt Enable MSVC Style Stack Checking in StackCheckLib. + gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking|FALSE|BOOLEAN|0x0000002f + # MU_CHANGE END: CLANGPDB Stack Cookies + [PcdsFixedAtBuild] ## Status code value for indicating a watchdog timer has expired. # EFI_COMPUTING_UNIT_HOST_PROCESSOR | EFI_CU_HP_EC_TIMER_EXPIRED @@ -2333,8 +2343,10 @@ # @Prompt Speculation Barrier Type. gEfiMdePkgTokenSpaceGuid.PcdSpeculationBarrierType|0x01|UINT8|0x30001018 - ## This PCD specifies the interrupt vector for stack cookie check failures - gEfiMdePkgTokenSpaceGuid.PcdStackCookieExceptionVector|0x42|UINT8|0x30001019 + # MU_CHANGE BEGIN: CLANGPDB Stack Cookies - removed PCD + # ## This PCD specifies the interrupt vector for stack cookie check failures + # gEfiMdePkgTokenSpaceGuid.PcdStackCookieExceptionVector|0x42|UINT8|0x30001019 + # MU_CHANGE END: CLANGPDB Stack Cookies ## Enforces the use of Secure UEFI spec defined RNG algorithms. # TRUE - Enforce the use of Secure UEFI spec defined RNG algorithms. diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc index 042518cf33b..49af97d975a 100644 --- a/MdePkg/MdePkg.dsc +++ b/MdePkg/MdePkg.dsc @@ -138,7 +138,9 @@ MdePkg/Library/BaseFdtLib/BaseFdtLib.inf MdePkg/Library/PeiRngLib/PeiRngLib.inf - MdePkg/Library/StackCheckFailureHookLibNull/StackCheckFailureHookLibNull.inf + # MU_CHANGE BEGIN: CLANGPDB Stack Cookies - removed StackCheckFailureHookLib + # MdePkg/Library/StackCheckFailureHookLibNull/StackCheckFailureHookLibNull.inf + # MU_CHANGE END: CLANGPDB Stack Cookies MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf MdePkg/Library/StackCheckLib/StackCheckLib.inf MdePkg/Library/DynamicStackCookieEntryPointLib/DxeCoreEntryPoint.inf diff --git a/MdePkg/Test/GoogleTest/Library/StackCheckLib/GoogleTestStackCheckLib.inf b/MdePkg/Test/GoogleTest/Library/StackCheckLib/GoogleTestStackCheckLib.inf new file mode 100644 index 00000000000..34a3ae4b5b1 --- /dev/null +++ b/MdePkg/Test/GoogleTest/Library/StackCheckLib/GoogleTestStackCheckLib.inf @@ -0,0 +1,29 @@ +## @file +# Host OS based Application that unit tests StackCheckLib using Google Test +# +# Copyright (c) Microsoft Corporation. +# SPDX-License-Identifier: BSD-2-Clause-Patent +## + +[Defines] + INF_VERSION = 0x00010005 + BASE_NAME = GoogleTestStackCheckLib + FILE_GUID = 2776BF17-D803-4127-8118-44D617C3B5FF + MODULE_TYPE = HOST_APPLICATION + VERSION_STRING = 1.0 + +[Sources] + StackCheckLibGoogleTestGcc.cpp ||||!gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking + +[Packages] + MdePkg/MdePkg.dec + UnitTestFrameworkPkg/UnitTestFrameworkPkg.dec + +[LibraryClasses] + GoogleTestLib + BaseLib + DebugLib + StackCheckLib + +[Pcd] + gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking diff --git a/MdePkg/Test/GoogleTest/Library/StackCheckLib/StackCheckLibGoogleTestGcc.cpp b/MdePkg/Test/GoogleTest/Library/StackCheckLib/StackCheckLibGoogleTestGcc.cpp new file mode 100644 index 00000000000..bbb8b539247 --- /dev/null +++ b/MdePkg/Test/GoogleTest/Library/StackCheckLib/StackCheckLibGoogleTestGcc.cpp @@ -0,0 +1,51 @@ +/** @file + Unit tests for the GCC-style -fstack-protector stack cookie checking + in StackCheckLib. + + This uses the actual StackCheckLib implementation in edk2 to validate + that overflowing a buffer is caught. + + Copyright (c) Microsoft Corporation. + SPDX-License-Identifier: BSD-2-Clause-Patent +**/ + +#include + +extern "C" { + #include + #include +} + +using namespace testing; + +// +// Overflow a stack buffer into the stack cookie. With stack cookies disabled (or StackCheckLibNull used) +// this function returns normally. +// +static +VOID +OverflowStackBuffer ( + VOID + ) +{ + UINTN Index; + volatile CHAR8 Buffer[16]; + + for (Index = 0; Index < 30; Index++) { + ((volatile CHAR8 *)Buffer)[Index] = (CHAR8)Index; + } +} + +TEST (StackCheckGccDeathTest, StackBufferOverflowDetected) { + EXPECT_DEATH (OverflowStackBuffer (), ""); +} + +int +main ( + int argc, + char *argv[] + ) +{ + testing::InitGoogleTest (&argc, argv); + return RUN_ALL_TESTS (); +} diff --git a/MdePkg/Test/MdePkgHostTest.dsc b/MdePkg/Test/MdePkgHostTest.dsc index 9f5075c2c0a..e757e4f89b3 100644 --- a/MdePkg/Test/MdePkgHostTest.dsc +++ b/MdePkg/Test/MdePkgHostTest.dsc @@ -24,6 +24,11 @@ SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf DevicePathLib|MdePkg/Library/UefiDevicePathLib/UefiDevicePathLibBase.inf +[PcdsFeatureFlag] + !if "$(FAMILY)" == "MSFT" || "$(TOOL_CHAIN_TAG)" == "CLANGPDB" + gEfiMdePkgTokenSpaceGuid.PcdEnableMsvcStyleStackChecking|TRUE + !endif + [Components] # # Build HOST_APPLICATION that tests the SafeIntLib @@ -43,6 +48,26 @@ MdePkg/Test/Library/SynchronizationLibHostUnitTest/SynchronizationLibHostUnitTest.inf # MU_CHANGE [END] + # + # StackCheckLib unit test. This is only run on GCC because the Windows C runtime provides + # its own stack cookie mechanism that cannot be overridden and GCC is the only toolchain + # that produces ELF binaries that also has stack cookies enabled. + # + !if "$(TOOL_CHAIN_TAG)" == "GCC" + MdePkg/Test/GoogleTest/Library/StackCheckLib/GoogleTestStackCheckLib.inf { + + StackCheckLib|MdePkg/Library/StackCheckLib/StackCheckLib.inf + + # + # Disable ASAN so only the stack cookie check is tested. + # ASAN catches stack buffer overflows before the stack cookie and would mask the test. + # Enable stack cookie checking for all functions to guarantee we should expect the stack cookie + # to be corrupted. + # + GCC:*_GCC_*_CC_FLAGS = -fno-sanitize=address -fstack-protector-all + } + !endif + # # Build HOST_APPLICATION Libraries # diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c index 903fac927e3..78f762b135d 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.c @@ -71,6 +71,10 @@ GetExceptionNameStr ( { if ((UINTN)ExceptionType < EXCEPTION_KNOWN_NAME_NUM) { return mExceptionNameStr[ExceptionType]; + // MU_CHANGE BEGIN: CLANGPDB Stack Cookies + } else if ((UINTN)ExceptionType == STACK_CHECK_ERROR_HANDLER_VECTOR) { + return "Stack Check Error Interrupt"; + // MU_CHANGE END: CLANGPDB Stack Cookies } else { return mExceptionReservedStr; } diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h index 465e81a0634..12eb099cc8a 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/CpuExceptionCommon.h @@ -17,6 +17,7 @@ #include #include #include +#include // MU_CHANGE: CLANGPDB Stack Cookies #include #include diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c index c9ffb785347..d1c92a96244 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/Ia32/ArchExceptionHandler.c @@ -328,6 +328,15 @@ DumpCpuContext ( InternalPrintMessage ("\n"); } + // MU_CHANGE BEGIN: CLANGPDB Stack Cookies + if (ExceptionType == STACK_CHECK_ERROR_HANDLER_VECTOR) { + InternalPrintMessage ( + "\nStack Check Exception occurred at address 0x%08x\n\n", + SystemContext.SystemContextIa32->Ecx + ); + } + // MU_CHANGE END: CLANGPDB Stack Cookies + InternalPrintMessage ( "EIP - %08x, CS - %08x, EFLAGS - %08x\n", SystemContext.SystemContextIa32->Eip, diff --git a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c index 1da68b9f179..b576f9bdbae 100644 --- a/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c +++ b/UefiCpuPkg/Library/CpuExceptionHandlerLib/X64/ArchExceptionHandler.c @@ -303,6 +303,15 @@ DumpCpuContext ( InternalPrintMessage ("\n"); } + // MU_CHANGE BEGIN: CLANGPDB Stack Cookies + if (ExceptionType == STACK_CHECK_ERROR_HANDLER_VECTOR) { + InternalPrintMessage ( + "\nStack Check Exception occurred at address 0x%016lx\n\n", + SystemContext.SystemContextX64->Rcx + ); + } + // MU_CHANGE END: CLANGPDB Stack Cookies + InternalPrintMessage ( "RIP - %016lx, CS - %016lx, RFLAGS - %016lx\n", SystemContext.SystemContextX64->Rip,