From e40eff04f4ff6c61d4d027b347e3ad343c26931f Mon Sep 17 00:00:00 2001 From: Joey Vagedes Date: Fri, 16 Feb 2024 10:05:50 -0800 Subject: [PATCH 1/7] remove edk2-basetools (#732) Removes edk2-basetools from pip-requirements.txt and any usage of it in the CISettings.py. The is done as there are changes in the build tools python source code that are available locally in BaseTools (as it is managed by Project Mu) that is not available in edk2-basetools. - [ ] Impacts functionality? - **Functionality** - Does the change ultimately impact how firmware functions? - Examples: Add a new library, publish a new PPI, update an algorithm, ... - [ ] Impacts security? - **Security** - Does the change have a direct security impact on an application, flow, or firmware? - Examples: Crypto algorithm change, buffer overflow fix, parameter validation improvement, ... - [ ] Breaking change? - **Breaking change** - Will anyone consuming this change experience a break in build or boot behavior? - Examples: Add a new library class, move a module to a different repo, call a function in a new library class in a pre-existing module, ... - [ ] Includes tests? - **Tests** - Does the change include any explicit test code? - Examples: Unit tests, integration tests, robot tests, ... - [ ] Includes documentation? - **Documentation** - Does the change contain explicit documentation additions outside direct code modifications (and comments)? - Examples: Update readme file, add feature readme file, link to documentation on an a separate Web page, ... Verified the build system continues to use the local python source N/A - only effects this repository's CI system. --- .pytool/CISettings.py | 26 -------------------------- UefiCpuPkg/UefiCpuPkg.dsc | 2 +- pip-requirements.txt | 1 - 3 files changed, 1 insertion(+), 28 deletions(-) diff --git a/.pytool/CISettings.py b/.pytool/CISettings.py index efdf7f8b0e..585b03343a 100644 --- a/.pytool/CISettings.py +++ b/.pytool/CISettings.py @@ -34,7 +34,6 @@ def __init__(self): self.ActualTargets = [] self.ActualArchitectures = [] self.ActualToolChainTag = "" - self.UseBuiltInBaseTools = None self.ActualScopes = None # ####################################################################################### # @@ -42,10 +41,6 @@ def __init__(self): # ####################################################################################### # def AddCommandLineOptions(self, parserObj): - group = parserObj.add_mutually_exclusive_group() - group.add_argument("-force_piptools", "--fpt", dest="force_piptools", action="store_true", default=False, help="Force the system to use pip tools") - group.add_argument("-no_piptools", "--npt", dest="no_piptools", action="store_true", default=False, help="Force the system to not use pip tools") - try: codeql_helpers.add_command_line_option(parserObj) except NameError: @@ -53,11 +48,6 @@ def AddCommandLineOptions(self, parserObj): def RetrieveCommandLineOptions(self, args): super().RetrieveCommandLineOptions(args) - if args.force_piptools: - self.UseBuiltInBaseTools = True - if args.no_piptools: - self.UseBuiltInBaseTools = False - try: self.codeql = codeql_helpers.is_codeql_enabled_on_command_line(args) except NameError: @@ -158,22 +148,6 @@ def GetActiveScopes(self): is_linux = GetHostInfo().os.upper() == "LINUX" - if self.UseBuiltInBaseTools is None: - is_linux = GetHostInfo().os.upper() == "LINUX" - # try and import the pip module for basetools - try: - import edk2basetools - self.UseBuiltInBaseTools = True - except ImportError: - self.UseBuiltInBaseTools = False - pass - - if self.UseBuiltInBaseTools == True: - scopes += ('pipbuild-unix',) if is_linux else ('pipbuild-win',) - logging.warning("Using Pip Tools based BaseTools") - else: - logging.warning("Falling back to using in-tree BaseTools") - if is_linux and self.ActualToolChainTag.upper().startswith("GCC"): if "AARCH64" in self.ActualArchitectures: scopes += ("gcc_aarch64_linux",) diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc index cf822fe362..cd23b3755e 100644 --- a/UefiCpuPkg/UefiCpuPkg.dsc +++ b/UefiCpuPkg/UefiCpuPkg.dsc @@ -105,7 +105,6 @@ [LibraryClasses.common.PEIM] MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf - LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.inf MpInitLib|UefiCpuPkg/Library/MpInitLib/PeiMpInitLib.inf RegisterCpuFeaturesLib|UefiCpuPkg/Library/RegisterCpuFeaturesLib/PeiRegisterCpuFeaturesLib.inf CpuCacheInfoLib|UefiCpuPkg/Library/CpuCacheInfoLib/PeiCpuCacheInfoLib.inf @@ -113,6 +112,7 @@ [LibraryClasses.IA32.PEIM, LibraryClasses.X64.PEIM] PeiServicesTablePointerLib|MdePkg/Library/PeiServicesTablePointerLibIdt/PeiServicesTablePointerLibIdt.inf CpuExceptionHandlerLib|UefiCpuPkg/Library/CpuExceptionHandlerLib/PeiCpuExceptionHandlerLib.inf + LockBoxLib|MdeModulePkg/Library/SmmLockBoxLib/SmmLockBoxPeiLib.inf # MU_CHANGE [LibraryClasses.common.DXE_DRIVER] MemoryAllocationLib|MdePkg/Library/UefiMemoryAllocationLib/UefiMemoryAllocationLib.inf diff --git a/pip-requirements.txt b/pip-requirements.txt index 2008adc1c3..855ff469ae 100644 --- a/pip-requirements.txt +++ b/pip-requirements.txt @@ -14,7 +14,6 @@ edk2-pytool-library~=0.20.0 # MU_CHANGE edk2-pytool-extensions~=0.27.0 # MU_CHANGE -edk2-basetools==0.1.49 antlr4-python3-runtime==4.13.1 regex lcov-cobertura==2.0.2 From e28ca996dbce685f505e5c51fd7c48cf1a9e44d7 Mon Sep 17 00:00:00 2001 From: Taylor Beebe Date: Tue, 6 Feb 2024 16:51:28 -0800 Subject: [PATCH 2/7] Fix Null Lib Evaluation in Basetools Description When parsing INF files, Basetools treats both libraries and modules the same. When the library dependencies are being collected for a module/library, libraries linked via: `NULL|Path/To/Library` would be included in the list of dependencies for libraries which does not match how these expressions are expected to be interpreted. This update changes the evaluation loop to skip NULL links when collecting dependencies for libraries. - [x] Impacts functionality? - **Functionality** - Does the change ultimately impact how firmware functions? - Examples: Add a new library, publish a new PPI, update an algorithm, ... - [ ] Impacts security? - **Security** - Does the change have a direct security impact on an application, flow, or firmware? - Examples: Crypto algorithm change, buffer overflow fix, parameter validation improvement, ... - [ ] Breaking change? - **Breaking change** - Will anyone consuming this change experience a break in build or boot behavior? - Examples: Add a new library class, move a module to a different repo, call a function in a new library class in a pre-existing module, ... - [ ] Includes tests? - **Tests** - Does the change include any explicit test code? - Examples: Unit tests, integration tests, robot tests, ... - [ ] Includes documentation? - **Documentation** - Does the change contain explicit documentation additions outside direct code modifications (and comments)? - Examples: Update readme file, add feature readme file, link to documentation on an a separate Web page, ... How This Was Tested Tested in pipelines Integration Instructions N/A --- BaseTools/Source/Python/Workspace/WorkspaceCommon.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/BaseTools/Source/Python/Workspace/WorkspaceCommon.py b/BaseTools/Source/Python/Workspace/WorkspaceCommon.py index 9e506fc646..c5754b4738 100644 --- a/BaseTools/Source/Python/Workspace/WorkspaceCommon.py +++ b/BaseTools/Source/Python/Workspace/WorkspaceCommon.py @@ -123,6 +123,10 @@ def GetModuleLibInstances(Module, Platform, BuildDatabase, Arch, Target, Toolcha while len(LibraryConsumerList) > 0: M = LibraryConsumerList.pop() for LibraryClassName in M.LibraryClasses: + # MU_CHANGE [START]: Fix NULL LibraryClass Inclusion Issue + if LibraryClassName.startswith("NULL") and bool(M.LibraryClass): + continue + # MU_CHANGE [END]: Fix NULL LibraryClass Inclusion Issue if LibraryClassName not in LibraryInstance: # override library instance for this module LibraryPath = Platform.Modules[str(Module)].LibraryClasses.get(LibraryClassName,Platform.LibraryClasses[LibraryClassName, ModuleType]) From 659598e47bb3c5e27dfafb0c8b8ee2ae67bc5bcb Mon Sep 17 00:00:00 2001 From: Taylor Beebe Date: Wed, 31 Jan 2024 16:52:10 -0800 Subject: [PATCH 3/7] Update StackCheckLibNull to Fix MSVC IA32 Builds and add GCC ARM Support Description MSVC IA32 requires the __security_check_cookie function to specify byte size (@__security_check_cookie@4). This change also declares __stack_chk_fail() in StackCheckLibNull.c to support GCC. - [x] Impacts functionality? - **Functionality** - Does the change ultimately impact how firmware functions? - Examples: Add a new library, publish a new PPI, update an algorithm, ... - [ ] Impacts security? - **Security** - Does the change have a direct security impact on an application, flow, or firmware? - Examples: Crypto algorithm change, buffer overflow fix, parameter validation improvement, ... - [ ] Breaking change? - **Breaking change** - Will anyone consuming this change experience a break in build or boot behavior? - Examples: Add a new library class, move a module to a different repo, call a function in a new library class in a pre-existing module, ... - [ ] Includes tests? - **Tests** - Does the change include any explicit test code? - Examples: Unit tests, integration tests, robot tests, ... - [ ] Includes documentation? - **Documentation** - Does the change contain explicit documentation additions outside direct code modifications (and comments)? - Examples: Update readme file, add feature readme file, link to documentation on an a separate Web page, ... How This Was Tested Tested on Q35 GCC and MSVC builds and an SBSA GCC build by purposefully overflowing the stack when the NULL library is in use. Integration Instructions N/A --- .../StackCheckFunctionsMsvc.nasm} | 16 ++++++--- .../StackCheckFunctionsMsvc.nasm | 33 ------------------- .../StackCheckLibNull/StackCheckLibNull.c | 13 ++++++-- .../StackCheckLibNull/StackCheckLibNull.inf | 9 +++-- .../X64/StackCheckFunctionsMsvc.nasm | 21 ++++++++++++ 5 files changed, 51 insertions(+), 41 deletions(-) rename MdePkg/Library/StackCheckLibNull/{StackCheckFunctionsGcc.nasm => IA32/StackCheckFunctionsMsvc.nasm} (53%) delete mode 100644 MdePkg/Library/StackCheckLibNull/StackCheckFunctionsMsvc.nasm create mode 100644 MdePkg/Library/StackCheckLibNull/X64/StackCheckFunctionsMsvc.nasm diff --git a/MdePkg/Library/StackCheckLibNull/StackCheckFunctionsGcc.nasm b/MdePkg/Library/StackCheckLibNull/IA32/StackCheckFunctionsMsvc.nasm similarity index 53% rename from MdePkg/Library/StackCheckLibNull/StackCheckFunctionsGcc.nasm rename to MdePkg/Library/StackCheckLibNull/IA32/StackCheckFunctionsMsvc.nasm index 287f80d435..f12f657916 100644 --- a/MdePkg/Library/StackCheckLibNull/StackCheckFunctionsGcc.nasm +++ b/MdePkg/Library/StackCheckLibNull/IA32/StackCheckFunctionsMsvc.nasm @@ -1,13 +1,21 @@ ;------------------------------------------------------------------------------ -; StackCheckFunctionsGcc.nasm +; IA32/StackCheckFunctionsMsvc.nasm ; ; Copyright (c) Microsoft Corporation. All rights reserved. ; SPDX-License-Identifier: BSD-2-Clause-Patent ;------------------------------------------------------------------------------ + DEFAULT REL SECTION .text -; Called when a stack cookie check fails. -global ASM_PFX(__stack_chk_fail) -ASM_PFX(__stack_chk_fail): +global ASM_PFX(__report_rangecheckfailure) +ASM_PFX(__report_rangecheckfailure): + ret + +global ASM_PFX(__GSHandlerCheck) +ASM_PFX(__GSHandlerCheck): ret + +global @__security_check_cookie@4 +@__security_check_cookie@4: + ret \ No newline at end of file diff --git a/MdePkg/Library/StackCheckLibNull/StackCheckFunctionsMsvc.nasm b/MdePkg/Library/StackCheckLibNull/StackCheckFunctionsMsvc.nasm deleted file mode 100644 index b6987c8130..0000000000 --- a/MdePkg/Library/StackCheckLibNull/StackCheckFunctionsMsvc.nasm +++ /dev/null @@ -1,33 +0,0 @@ -;------------------------------------------------------------------------------ -; StackCheckFunctionsMsvc.nasm -; -; Copyright (c) Microsoft Corporation. All rights reserved. -; SPDX-License-Identifier: BSD-2-Clause-Patent -;------------------------------------------------------------------------------ - DEFAULT REL - SECTION .text - -; 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): - 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): - ret - -;------------------------------------------------------------------------------ -; 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): - ret diff --git a/MdePkg/Library/StackCheckLibNull/StackCheckLibNull.c b/MdePkg/Library/StackCheckLibNull/StackCheckLibNull.c index 039ff9b2a2..e335cd697b 100644 --- a/MdePkg/Library/StackCheckLibNull/StackCheckLibNull.c +++ b/MdePkg/Library/StackCheckLibNull/StackCheckLibNull.c @@ -8,7 +8,16 @@ #include #if defined (__GNUC__) || defined (__clang__) -VOID *__stack_chk_guard = (VOID *)0x0; +VOID *__stack_chk_guard = (VOID *)(UINTN)0x0; + +VOID +EFIAPI +__stack_chk_fail ( + VOID + ) +{ +} + #elif defined (_MSC_VER) -UINTN __security_cookie = 0x0; +VOID *__security_cookie = (VOID *)(UINTN)0x0; #endif diff --git a/MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf b/MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf index ebcd173de1..4768165166 100644 --- a/MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf +++ b/MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf @@ -17,12 +17,17 @@ [Sources] StackCheckLibNull.c - StackCheckFunctionsGcc.nasm | GCC - StackCheckFunctionsMsvc.nasm | MSFT + +[Sources.IA32] + IA32/StackCheckFunctionsMsvc.nasm | MSFT + +[Sources.X64] + X64/StackCheckFunctionsMsvc.nasm | MSFT [Packages] MdePkg/MdePkg.dec +# Disable LTO to ensure the linker doesn't optimize out the stack cookie [BuildOptions] GCC:*_*_*_CC_FLAGS = -fno-lto MSFT:*_*_*_CC_FLAGS = /GL- diff --git a/MdePkg/Library/StackCheckLibNull/X64/StackCheckFunctionsMsvc.nasm b/MdePkg/Library/StackCheckLibNull/X64/StackCheckFunctionsMsvc.nasm new file mode 100644 index 0000000000..ce81a002f8 --- /dev/null +++ b/MdePkg/Library/StackCheckLibNull/X64/StackCheckFunctionsMsvc.nasm @@ -0,0 +1,21 @@ +;------------------------------------------------------------------------------ +; X64/StackCheckFunctionsMsvc.nasm +; +; Copyright (c) Microsoft Corporation. All rights reserved. +; SPDX-License-Identifier: BSD-2-Clause-Patent +;------------------------------------------------------------------------------ + + DEFAULT REL + SECTION .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 \ No newline at end of file From 4e5a2a9dd620d3851e6045641ef045d4a98f2a69 Mon Sep 17 00:00:00 2001 From: Taylor Beebe Date: Wed, 31 Jan 2024 16:54:58 -0800 Subject: [PATCH 4/7] Generate Random Stack Cookie Value at Build Time Description This PR updates the GenC logic to generate a random stack cookie value for the stack check libraries. These random values improve security for modules which cannot update the global intrinsics. - [x] Impacts functionality? - **Functionality** - Does the change ultimately impact how firmware functions? - Examples: Add a new library, publish a new PPI, update an algorithm, ... - [ ] Impacts security? - **Security** - Does the change have a direct security impact on an application, flow, or firmware? - Examples: Crypto algorithm change, buffer overflow fix, parameter validation improvement, ... - [ ] Breaking change? - **Breaking change** - Will anyone consuming this change experience a break in build or boot behavior? - Examples: Add a new library class, move a module to a different repo, call a function in a new library class in a pre-existing module, ... - [ ] Includes tests? - **Tests** - Does the change include any explicit test code? - Examples: Unit tests, integration tests, robot tests, ... - [ ] Includes documentation? - **Documentation** - Does the change contain explicit documentation additions outside direct code modifications (and comments)? - Examples: Update readme file, add feature readme file, link to documentation on an a separate Web page, ... How This Was Tested Tested on MSVC and GCC builds of Q35 and a GCC build of SBSA by purposefully overflowing the stack. Integration Instructions N/A --- BaseTools/Source/Python/AutoGen/GenC.py | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/BaseTools/Source/Python/AutoGen/GenC.py b/BaseTools/Source/Python/AutoGen/GenC.py index 2bdfb65a5c..5bf694c425 100755 --- a/BaseTools/Source/Python/AutoGen/GenC.py +++ b/BaseTools/Source/Python/AutoGen/GenC.py @@ -21,6 +21,8 @@ from .GenPcdDb import CreatePcdDatabaseCode from .IdfClassObject import * +import secrets # MU_CHANGE: Add Stack Cookie Support + ## PCD type string gItemTypeStringDatabase = { TAB_PCDS_FEATURE_FLAG : TAB_PCDS_FIXED_AT_BUILD, @@ -2042,6 +2044,22 @@ def CreateFooterCode(Info, AutoGenC, AutoGenH): def CreateCode(Info, AutoGenC, AutoGenH, StringH, UniGenCFlag, UniGenBinBuffer, StringIdf, IdfGenCFlag, IdfGenBinBuffer): CreateHeaderCode(Info, AutoGenC, AutoGenH) + # MU_CHANGE [START]: Add Stack Cookie Support + if Info.ModuleType != SUP_MODULE_HOST_APPLICATION: + if Info.Arch not in ['X64', 'IA32', 'ARM', 'AARCH64']: + EdkLogger.error("build", AUTOGEN_ERROR, "Unsupported Arch %s" % Info.Arch, ExtraData="[%s]" % str(Info)) + else: + Bitwidth = 64 if Info.Arch == 'X64' or Info.Arch == 'AARCH64' else 32 + + CookieValue = secrets.randbelow(0xFFFFFFFFFFFFFFFF if Bitwidth == 64 else 0xFFFFFFFF) + + AutoGenH.Append(( + '#define STACK_COOKIE_VALUE 0x%XULL\n' % CookieValue + if Bitwidth == 64 else + '#define STACK_COOKIE_VALUE 0x%X\n' % CookieValue + )) + # MU_CHANGE [END]: Add Stack Cookie Support + CreateGuidDefinitionCode(Info, AutoGenC, AutoGenH) CreateProtocolDefinitionCode(Info, AutoGenC, AutoGenH) CreatePpiDefinitionCode(Info, AutoGenC, AutoGenH) From 8336989d004ff1908eab5c505337d08eb567e64d Mon Sep 17 00:00:00 2001 From: Taylor Beebe Date: Tue, 6 Feb 2024 18:30:41 -0800 Subject: [PATCH 5/7] Add Stack Cookie Support for IA32, ARM, and AARCH64 Description This update replaces StackCheckLib with StackCheckLibStaticInit and StackCheckLibDynamicInit. The new libraries have GCC support for ARM, AARCH64, IA32 and X64 builds. The libraries have MSVC support for IA32 and X64 builds. StackCheckLibStaticInit does not have a library constructor and should be used whenever the stack cookie value cannot be updated during driver execution (i.e. when the stack cookie is not in a writable or no RNG library is available). StackCheckLibDynamicInit has a library constructor and should be used whenever the stack cookie value can be updated at runtime (i.e. for DXE modules and shadowed PEIMs). This update also removes the stack cookie library definitions from MdeLibs.dsc.inc due to GCC build issues when the instanced versions are used during CI builds. The instanced versions will need to be explicitly added to the platform DSC files, and this is acceptable because platforms will likely want to mix and match the static and dynamic versions of the library for each module type. - [x] Impacts functionality? - **Functionality** - Does the change ultimately impact how firmware functions? - Examples: Add a new library, publish a new PPI, update an algorithm, ... - [x] Impacts security? - **Security** - Does the change have a direct security impact on an application, flow, or firmware? - Examples: Crypto algorithm change, buffer overflow fix, parameter validation improvement, ... - [x] Breaking change? - **Breaking change** - Will anyone consuming this change experience a break in build or boot behavior? - Examples: Add a new library class, move a module to a different repo, call a function in a new library class in a pre-existing module, ... - [ ] Includes tests? - **Tests** - Does the change include any explicit test code? - Examples: Unit tests, integration tests, robot tests, ... - [ ] Includes documentation? - **Documentation** - Does the change contain explicit documentation additions outside direct code modifications (and comments)? - Examples: Update readme file, add feature readme file, link to documentation on an a separate Web page, ... How This Was Tested Tested on a Q35 GCC and MSVC builds, and on an SBSA GCC build by purposefully performing a stack overflow. Integration Instructions Platforms will need to explicitly declare the StackCheckLib and StackCheckFailureLib instances for their platforms. EXAMPLE: ``` StackCheckFailureLib|MdePkg/Library/StackCheckFailureLibNull/StackCheckFailureLibNull.inf [LibraryClasses.common.SEC, LibraryClasses.common.PEI_CORE] NULL|MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf [LibraryClasses.common.PEIM, LibraryClasses.common.MM_CORE_STANDALONE, LibraryClasses.common.MM_STANDALONE] NULL|MdePkg/Library/StackCheckLib/StackCheckLibStaticInit.inf [LibraryClasses.common.DXE_CORE, LibraryClasses.common.SMM_CORE, LibraryClasses.common.DXE_SMM_DRIVER, LibraryClasses.common.DXE_DRIVER, LibraryClasses.common.DXE_RUNTIME_DRIVER, LibraryClasses.common.DXE_SAL_DRIVER, LibraryClasses.common.UEFI_DRIVER, LibraryClasses.common.UEFI_APPLICATION] NULL|MdePkg/Library/StackCheckLib/StackCheckLibDynamicInit.inf ``` --- BaseTools/Source/Python/AutoGen/GenC.py | 7 ++- CryptoPkg/CryptoPkg.dsc | 4 +- MdeModulePkg/MdeModulePkg.dsc | 4 +- .../AArch64/StackCookieInterrupt.S | 21 +++++++ .../StackCheckLib/Arm/StackCookieInterrupt.S | 21 +++++++ ...unctionsMsvc.nasm => CheckCookieMsvc.nasm} | 29 ++------- .../IA32/StackCookieInterrupt.nasm | 23 +++++++ .../StackCheckLib/StackCheckFunctionsGcc.nasm | 17 ------ .../StackCheckLib/StackCheckLibCommon.c | 61 +++++++++++++++++++ ...kCheckLib.c => StackCheckLibDynamicInit.c} | 14 ++--- ...ckLib.inf => StackCheckLibDynamicInit.inf} | 24 +++++--- .../StackCheckLib/StackCheckLibStaticInit.inf | 43 +++++++++++++ ...unctionsMsvc.nasm => CheckCookieMsvc.nasm} | 28 ++------- MdePkg/MdeLibs.dsc.inc | 7 --- MdePkg/MdePkg.dsc | 15 ++--- NetworkPkg/NetworkPkg.dsc | 7 +-- .../SharedNetworking/SharedNetworkPkg.dsc | 15 +---- PcAtChipsetPkg/PcAtChipsetPkg.dsc | 1 + PolicyServicePkg/PolicyServicePkg.dsc | 9 +-- ShellPkg/ShellPkg.dsc | 3 +- StandaloneMmPkg/StandaloneMmPkg.dsc | 3 +- UefiCpuPkg/UefiCpuPkg.dsc | 3 +- .../UnitTestFrameworkPkgTarget.dsc.inc | 9 +-- 23 files changed, 230 insertions(+), 138 deletions(-) create mode 100644 MdePkg/Library/StackCheckLib/AArch64/StackCookieInterrupt.S create mode 100644 MdePkg/Library/StackCheckLib/Arm/StackCookieInterrupt.S rename MdePkg/Library/StackCheckLib/IA32/{StackCheckFunctionsMsvc.nasm => CheckCookieMsvc.nasm} (51%) create mode 100644 MdePkg/Library/StackCheckLib/IA32/StackCookieInterrupt.nasm delete mode 100644 MdePkg/Library/StackCheckLib/StackCheckFunctionsGcc.nasm create mode 100644 MdePkg/Library/StackCheckLib/StackCheckLibCommon.c rename MdePkg/Library/StackCheckLib/{StackCheckLib.c => StackCheckLibDynamicInit.c} (69%) rename MdePkg/Library/StackCheckLib/{StackCheckLib.inf => StackCheckLibDynamicInit.inf} (61%) create mode 100644 MdePkg/Library/StackCheckLib/StackCheckLibStaticInit.inf rename MdePkg/Library/StackCheckLib/X64/{StackCheckFunctionsMsvc.nasm => CheckCookieMsvc.nasm} (52%) diff --git a/BaseTools/Source/Python/AutoGen/GenC.py b/BaseTools/Source/Python/AutoGen/GenC.py index 5bf694c425..97e5ea8490 100755 --- a/BaseTools/Source/Python/AutoGen/GenC.py +++ b/BaseTools/Source/Python/AutoGen/GenC.py @@ -700,10 +700,12 @@ } ## Library Constructor and Destructor Templates +# MU_CHANGE [BEGIN]: Add StackCookieSupport marker for stack cookie support. gLibraryString = { SUP_MODULE_BASE : TemplateString(""" ${BEGIN}${FunctionPrototype}${END} +${StackCookieSupport} VOID EFIAPI ProcessLibrary${Type}List ( @@ -714,10 +716,10 @@ ${FunctionCall}${END} } """), -# MU_CHANGE [BEGIN]: Add StackCookieSupport marker for stack cookie support. 'PEI' : TemplateString(""" ${BEGIN}${FunctionPrototype}${END} +${StackCookieSupport} VOID EFIAPI ProcessLibrary${Type}List ( @@ -745,10 +747,10 @@ ${FunctionCall}${END} } """), -# MU_CHANGE [END]: Add StackCookieSupport marker for stack cookie support. 'MM' : TemplateString(""" ${BEGIN}${FunctionPrototype}${END} +${StackCookieSupport} VOID EFIAPI ProcessLibrary${Type}List ( @@ -761,6 +763,7 @@ } """), } +# MU_CHANGE [END]: Add StackCookieSupport marker for stack cookie support. gBasicHeaderFile = "Base.h" diff --git a/CryptoPkg/CryptoPkg.dsc b/CryptoPkg/CryptoPkg.dsc index a83da89bfa..04524ec3ee 100644 --- a/CryptoPkg/CryptoPkg.dsc +++ b/CryptoPkg/CryptoPkg.dsc @@ -48,6 +48,8 @@ TlsLib|CryptoPkg/Library/TlsLibNull/TlsLibNull.inf HashApiLib|CryptoPkg/Library/BaseHashApiLib/BaseHashApiLib.inf RngLib|MdePkg/Library/BaseRngLibNull/BaseRngLibNull.inf + NULL|MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf # MU_CHANGE: /GS and -fstack-protector support + ##MSCHANGE Begin FltUsedLib|MdePkg/Library/FltUsedLib/FltUsedLib.inf @@ -95,7 +97,7 @@ NULL|MdePkg/Library/CompilerIntrinsicsLib/ArmCompilerIntrinsicsLib.inf # Add support for stack protector - NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf + # NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf # MU_CHANGE: Use Project Mu StackCheckLib [LibraryClasses.common.PEIM] PeimEntryPoint|MdePkg/Library/PeimEntryPoint/PeimEntryPoint.inf diff --git a/MdeModulePkg/MdeModulePkg.dsc b/MdeModulePkg/MdeModulePkg.dsc index 847e2f4133..cdb52e05e0 100644 --- a/MdeModulePkg/MdeModulePkg.dsc +++ b/MdeModulePkg/MdeModulePkg.dsc @@ -122,6 +122,8 @@ PanicLib|MdePkg/Library/BasePanicLibNull/BasePanicLibNull.inf # MU_CHANGE + NULL|MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf # MU_CHANGE: /GS and -fstack-protector support + # MU_CHANGE START Include MemoryProtectionHobLib [LibraryClasses.common.DXE_DRIVER, LibraryClasses.common.DXE_CORE, LibraryClasses.common.UEFI_APPLICATION] DxeMemoryProtectionHobLib|MdeModulePkg/Library/MemoryProtectionHobLib/DxeMemoryProtectionHobLib.inf @@ -235,7 +237,7 @@ # Since software stack checking may be heuristically enabled by the compiler # include BaseStackCheckLib unconditionally. # - NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf + # NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf # MU_CHANGE: Use Project Mu StackCheckLib [LibraryClasses.EBC, LibraryClasses.RISCV64, LibraryClasses.LOONGARCH64] LockBoxLib|MdeModulePkg/Library/LockBoxNullLib/LockBoxNullLib.inf diff --git a/MdePkg/Library/StackCheckLib/AArch64/StackCookieInterrupt.S b/MdePkg/Library/StackCheckLib/AArch64/StackCookieInterrupt.S new file mode 100644 index 0000000000..5d32ff7050 --- /dev/null +++ b/MdePkg/Library/StackCheckLib/AArch64/StackCookieInterrupt.S @@ -0,0 +1,21 @@ +//------------------------------------------------------------------------------ +// AArch64/StackCookieInterrupt.S +// +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: BSD-2-Clause-Patent +//------------------------------------------------------------------------------ + + .text + +//------------------------------------------------------------------------------ +// Calls an interrupt using the vector specified by PcdStackCookieExceptionVector +// +// VOID +// TriggerStackCookieInterrupt ( +// VOID +// ); +//------------------------------------------------------------------------------ +.global ASM_PFX(TriggerStackCookieInterrupt) +ASM_PFX(TriggerStackCookieInterrupt): + smc FixedPcdGet8 (PcdStackCookieExceptionVector) + ret diff --git a/MdePkg/Library/StackCheckLib/Arm/StackCookieInterrupt.S b/MdePkg/Library/StackCheckLib/Arm/StackCookieInterrupt.S new file mode 100644 index 0000000000..93be6de55d --- /dev/null +++ b/MdePkg/Library/StackCheckLib/Arm/StackCookieInterrupt.S @@ -0,0 +1,21 @@ +//------------------------------------------------------------------------------ +// Arm/StackCookieInterrupt.S +// +// Copyright (c) Microsoft Corporation. All rights reserved. +// SPDX-License-Identifier: BSD-2-Clause-Patent +//------------------------------------------------------------------------------ + + .text + +//------------------------------------------------------------------------------ +// Calls an interrupt using the vector specified by PcdStackCookieExceptionVector +// +// VOID +// TriggerStackCookieInterrupt ( +// VOID +// ); +//------------------------------------------------------------------------------ +.global ASM_PFX(TriggerStackCookieInterrupt) +ASM_PFX(TriggerStackCookieInterrupt): + swi FixedPcdGet8 (PcdStackCookieExceptionVector) + bx lr diff --git a/MdePkg/Library/StackCheckLib/IA32/StackCheckFunctionsMsvc.nasm b/MdePkg/Library/StackCheckLib/IA32/CheckCookieMsvc.nasm similarity index 51% rename from MdePkg/Library/StackCheckLib/IA32/StackCheckFunctionsMsvc.nasm rename to MdePkg/Library/StackCheckLib/IA32/CheckCookieMsvc.nasm index 1bb382459e..3592b50eeb 100644 --- a/MdePkg/Library/StackCheckLib/IA32/StackCheckFunctionsMsvc.nasm +++ b/MdePkg/Library/StackCheckLib/IA32/CheckCookieMsvc.nasm @@ -1,5 +1,5 @@ ;------------------------------------------------------------------------------ -; IA32/StackCheckFunctionsMsvc.nasm +; IA32/CheckCookieMsvc.nasm ; ; Copyright (c) Microsoft Corporation. All rights reserved. ; SPDX-License-Identifier: BSD-2-Clause-Patent @@ -8,23 +8,8 @@ DEFAULT REL SECTION .text -extern ASM_PFX(__security_cookie) extern ASM_PFX(StackCheckFailure) -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) - 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) - ret +extern ASM_PFX(__security_cookie) ;------------------------------------------------------------------------------ ; Checks the stack cookie value against __security_cookie and calls the @@ -33,15 +18,11 @@ ASM_PFX(__GSHandlerCheck): ; VOID ; EFIAPI ; __security_check_cookie ( -; IN UINTN CheckValue); +; IN UINTN CheckValue +; ); ;------------------------------------------------------------------------------ global @__security_check_cookie@4 @__security_check_cookie@4: cmp ecx, [ASM_PFX(__security_cookie)] - jne security_check_cookie_failure - ret - -security_check_cookie_failure: - call ASM_PFX(StackCheckFailure) - int FixedPcdGet8 (PcdStackCookieExceptionVector) + jne ASM_PFX(StackCheckFailure) ret diff --git a/MdePkg/Library/StackCheckLib/IA32/StackCookieInterrupt.nasm b/MdePkg/Library/StackCheckLib/IA32/StackCookieInterrupt.nasm new file mode 100644 index 0000000000..22301aacb1 --- /dev/null +++ b/MdePkg/Library/StackCheckLib/IA32/StackCookieInterrupt.nasm @@ -0,0 +1,23 @@ +;------------------------------------------------------------------------------ +; IA32/StackCookieInterrupt.nasm +; +; Copyright (c) Microsoft Corporation. All rights reserved. +; SPDX-License-Identifier: BSD-2-Clause-Patent +;------------------------------------------------------------------------------ + + DEFAULT REL + SECTION .text + +;------------------------------------------------------------------------------ +; Checks the stack cookie value against __security_cookie and calls the +; stack cookie failure handler if there is a mismatch. +; +; VOID +; TriggerStackCookieInterrupt ( +; VOID +; ); +;------------------------------------------------------------------------------ +global ASM_PFX(TriggerStackCookieInterrupt) +ASM_PFX(TriggerStackCookieInterrupt): + int FixedPcdGet8 (PcdStackCookieExceptionVector) + ret diff --git a/MdePkg/Library/StackCheckLib/StackCheckFunctionsGcc.nasm b/MdePkg/Library/StackCheckLib/StackCheckFunctionsGcc.nasm deleted file mode 100644 index 917b97d8b3..0000000000 --- a/MdePkg/Library/StackCheckLib/StackCheckFunctionsGcc.nasm +++ /dev/null @@ -1,17 +0,0 @@ -;------------------------------------------------------------------------------ -; StackCheckFunctionsGcc.nasm -; -; Copyright (c) Microsoft Corporation. All rights reserved. -; SPDX-License-Identifier: BSD-2-Clause-Patent -;------------------------------------------------------------------------------ - DEFAULT REL - SECTION .text - -extern ASM_PFX(StackCheckFailure) - -; Called when a stack cookie check fails. -global ASM_PFX(__stack_chk_fail) -ASM_PFX(__stack_chk_fail): - call StackCheckFailure - int FixedPcdGet8 (PcdStackCookieExceptionVector) - ret diff --git a/MdePkg/Library/StackCheckLib/StackCheckLibCommon.c b/MdePkg/Library/StackCheckLib/StackCheckLibCommon.c new file mode 100644 index 0000000000..256e6003e9 --- /dev/null +++ b/MdePkg/Library/StackCheckLib/StackCheckLibCommon.c @@ -0,0 +1,61 @@ +/** @file + Provides the required functionality for handling stack + cookie check failures. + + Copyright (c) Microsoft Corporation. All rights reserved. + SPDX-License-Identifier: BSD-2-Clause-Patent +**/ + +#include + +#include +#include + +/** + Calls an interrupt using the vector specified by PcdStackCookieExceptionVector +**/ +VOID +TriggerStackCookieInterrupt ( + VOID + ); + +#if defined (__GNUC__) || defined (__clang__) + +VOID *__stack_chk_guard = (VOID *)(UINTN)STACK_COOKIE_VALUE; + +VOID +__stack_chk_fail ( + VOID + ) +{ + TriggerStackCookieInterrupt (); +} + +#elif defined (_MSC_VER) +VOID *__security_cookie = (VOID *)(UINTN)STACK_COOKIE_VALUE; + +NORETURN VOID __cdecl +__report_rangecheckfailure ( + VOID + ) +{ + CpuDeadLoop (); +} + +NORETURN VOID __cdecl +__GSHandlerCheck ( + VOID + ) +{ + CpuDeadLoop (); +} + +VOID +StackCheckFailure ( + VOID *ActualCookieValue + ) +{ + TriggerStackCookieInterrupt (); +} + +#endif diff --git a/MdePkg/Library/StackCheckLib/StackCheckLib.c b/MdePkg/Library/StackCheckLib/StackCheckLibDynamicInit.c similarity index 69% rename from MdePkg/Library/StackCheckLib/StackCheckLib.c rename to MdePkg/Library/StackCheckLib/StackCheckLibDynamicInit.c index c62cae7df6..3c82d77bdf 100644 --- a/MdePkg/Library/StackCheckLib/StackCheckLib.c +++ b/MdePkg/Library/StackCheckLib/StackCheckLibDynamicInit.c @@ -1,6 +1,6 @@ /** @file - Provides the required functionality for initializing and - checking the stack cookie. + Provides the required functionality for initializing + the stack cookie value. Copyright (c) Microsoft Corporation. All rights reserved. SPDX-License-Identifier: BSD-2-Clause-Patent @@ -8,17 +8,13 @@ #include #include -#include -#include -#include -#include + #include -#include #if defined (__GNUC__) || defined (__clang__) -VOID *__stack_chk_guard = (VOID *)0xBEEBE; +extern VOID *__stack_chk_guard; #elif defined (_MSC_VER) -UINTN __security_cookie = 0xBEEBE; +extern VOID *__security_cookie; #endif /** diff --git a/MdePkg/Library/StackCheckLib/StackCheckLib.inf b/MdePkg/Library/StackCheckLib/StackCheckLibDynamicInit.inf similarity index 61% rename from MdePkg/Library/StackCheckLib/StackCheckLib.inf rename to MdePkg/Library/StackCheckLib/StackCheckLibDynamicInit.inf index 2d945af9d3..8861424a59 100644 --- a/MdePkg/Library/StackCheckLib/StackCheckLib.inf +++ b/MdePkg/Library/StackCheckLib/StackCheckLibDynamicInit.inf @@ -8,7 +8,7 @@ [Defines] INF_VERSION = 0x00010005 - BASE_NAME = StackCheckLib + BASE_NAME = StackCheckLibDynamicInit FILE_GUID = 495b10c8-7148-4b62-92b0-7cf4551df83d MODULE_TYPE = BASE VERSION_STRING = 1.0 @@ -16,28 +16,34 @@ CONSTRUCTOR = InitializeSecurityCookie [Sources] - StackCheckLib.c - StackCheckFunctionsGcc.nasm |GCC + StackCheckLibCommon.c + StackCheckLibDynamicInit.c [Sources.IA32] - IA32/StackCheckFunctionsMsvc.nasm | MSFT + IA32/CheckCookieMsvc.nasm | MSFT [Sources.X64] - X64/StackCheckFunctionsMsvc.nasm | MSFT + X64/CheckCookieMsvc.nasm | MSFT + +[Sources.IA32, Sources.X64] + IA32/StackCookieInterrupt.nasm + +[Sources.ARM] + Arm/StackCookieInterrupt.S |GCC + +[Sources.AARCH64] + AArch64/StackCookieInterrupt.S |GCC [Packages] MdePkg/MdePkg.dec [LibraryClasses] - BaseLib - BaseMemoryLib - DebugLib RngLib - StackCheckFailureLib [FixedPcd] gEfiMdePkgTokenSpaceGuid.PcdStackCookieExceptionVector +# Disable LTO to ensure the linker doesn't optimize out the stack cookie [BuildOptions] GCC:*_*_*_CC_FLAGS = -fno-lto MSFT:*_*_*_CC_FLAGS = /GL- diff --git a/MdePkg/Library/StackCheckLib/StackCheckLibStaticInit.inf b/MdePkg/Library/StackCheckLib/StackCheckLibStaticInit.inf new file mode 100644 index 0000000000..5fdf63d70b --- /dev/null +++ b/MdePkg/Library/StackCheckLib/StackCheckLibStaticInit.inf @@ -0,0 +1,43 @@ +## @file +# Provides the required functionality for checking the stack cookie. +# +# Copyright (c) Microsoft Corporation. All rights reserved. +# SPDX-License-Identifier: BSD-2-Clause-Patent +## + +[Defines] + INF_VERSION = 0x00010005 + BASE_NAME = StackCheckLibStaticInit + FILE_GUID = 2b24dc50-e33d-4c9f-8b62-e826f06e483f + MODULE_TYPE = BASE + VERSION_STRING = 1.0 + LIBRARY_CLASS = NULL + +[Sources] + StackCheckLibCommon.c + +[Sources.IA32] + IA32/CheckCookieMsvc.nasm | MSFT + +[Sources.X64] + X64/CheckCookieMsvc.nasm | MSFT + +[Sources.IA32, Sources.X64] + IA32/StackCookieInterrupt.nasm + +[Sources.ARM] + Arm/StackCookieInterrupt.S |GCC + +[Sources.AARCH64] + AArch64/StackCookieInterrupt.S |GCC + +[Packages] + MdePkg/MdePkg.dec + +[FixedPcd] + gEfiMdePkgTokenSpaceGuid.PcdStackCookieExceptionVector + +# Disable LTO to ensure the linker doesn't optimize out the stack cookie +[BuildOptions] + GCC:*_*_*_CC_FLAGS = -fno-lto + MSFT:*_*_*_CC_FLAGS = /GL- diff --git a/MdePkg/Library/StackCheckLib/X64/StackCheckFunctionsMsvc.nasm b/MdePkg/Library/StackCheckLib/X64/CheckCookieMsvc.nasm similarity index 52% rename from MdePkg/Library/StackCheckLib/X64/StackCheckFunctionsMsvc.nasm rename to MdePkg/Library/StackCheckLib/X64/CheckCookieMsvc.nasm index a562737a3f..89d53c6518 100644 --- a/MdePkg/Library/StackCheckLib/X64/StackCheckFunctionsMsvc.nasm +++ b/MdePkg/Library/StackCheckLib/X64/CheckCookieMsvc.nasm @@ -1,5 +1,5 @@ ;------------------------------------------------------------------------------ -; X64/StackCheckFunctionsMsvc.nasm +; X64/CheckCookieMsvc.nasm ; ; Copyright (c) Microsoft Corporation. All rights reserved. ; SPDX-License-Identifier: BSD-2-Clause-Patent @@ -10,22 +10,6 @@ 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) - 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) - ret - ;------------------------------------------------------------------------------ ; Checks the stack cookie value against __security_cookie and calls the @@ -34,15 +18,11 @@ ASM_PFX(__GSHandlerCheck): ; VOID ; EFIAPI ; __security_check_cookie ( -; IN UINTN CheckValue); +; IN UINTN CheckValue +; ); ;------------------------------------------------------------------------------ global ASM_PFX(__security_check_cookie) ASM_PFX(__security_check_cookie): cmp rcx, [ASM_PFX(__security_cookie)] - jne security_check_cookie_failure - ret - -security_check_cookie_failure: - call StackCheckFailure - int FixedPcdGet8 (PcdStackCookieExceptionVector) + jne ASM_PFX(StackCheckFailure) ret diff --git a/MdePkg/MdeLibs.dsc.inc b/MdePkg/MdeLibs.dsc.inc index 0faf3b91d1..4580481cb5 100644 --- a/MdePkg/MdeLibs.dsc.inc +++ b/MdePkg/MdeLibs.dsc.inc @@ -16,10 +16,3 @@ RegisterFilterLib|MdePkg/Library/RegisterFilterLibNull/RegisterFilterLibNull.inf CpuLib|MdePkg/Library/BaseCpuLib/BaseCpuLib.inf SmmCpuRendezvousLib|MdePkg/Library/SmmCpuRendezvousLibNull/SmmCpuRendezvousLibNull.inf - -# MU_CHANGE [BEGIN] - Add Stack Cookie Support -[LibraryClasses.X64.SEC, LibraryClasses.X64.PEIM, LibraryClasses.X64.PEI_CORE, LibraryClasses.X64.SMM_CORE, LibraryClasses.X64.DXE_SMM_DRIVER, LibraryClasses.X64.MM_CORE_STANDALONE, LibraryClasses.X64.MM_STANDALONE, LibraryClasses.X64.DXE_CORE, LibraryClasses.X64.DXE_DRIVER, LibraryClasses.X64.DXE_RUNTIME_DRIVER, LibraryClasses.X64.DXE_SAL_DRIVER, LibraryClasses.X64.UEFI_DRIVER, LibraryClasses.X64.UEFI_APPLICATION] - RngLib|MdePkg/Library/BaseRngLib/BaseRngLib.inf - NULL|MdePkg/Library/StackCheckLib/StackCheckLib.inf - StackCheckFailureLib|MdePkg/Library/StackCheckFailureLibNull/StackCheckFailureLibNull.inf -# MU_CHANGE [END] - Add Stack Cookie Support \ No newline at end of file diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc index 509f2f87d7..3072fdbab6 100644 --- a/MdePkg/MdePkg.dsc +++ b/MdePkg/MdePkg.dsc @@ -34,6 +34,7 @@ [LibraryClasses] SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf + NULL|MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf # MU_CHANGE: /GS and -fstack-protector support [Components] MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf @@ -139,6 +140,11 @@ MdePkg/Library/SmmCpuRendezvousLibNull/SmmCpuRendezvousLibNull.inf MdePkg/Library/BaseMmuLibNull/BaseMmuLibNull.inf ## MU_CHANGE + MdePkg/Library/StackCheckFailureLibNull/StackCheckFailureLibNull.inf ## MU_CHANGE + MdePkg/Library/StackCheckLib/StackCheckLibStaticInit.inf ## MU_CHANGE + MdePkg/Library/StackCheckLib/StackCheckLibDynamicInit.inf ## MU_CHANGE + MdePkg/Library/StackCheckFailureLib/StackCheckFailureLib.inf ## MU_CHANGE + MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf ## MU_CHANGE [Components.IA32, Components.X64, Components.ARM, Components.AARCH64] # @@ -186,15 +192,6 @@ MdePkg/Library/MmUnblockMemoryLib/MmUnblockMemoryLibNull.inf MdePkg/Library/TdxLib/TdxLib.inf -# MU_CHANGE [BEGIN] - Stack Cookie Support -[Components.X64] - MdePkg/Library/StackCheckLib/StackCheckLib.inf - MdePkg/Library/StackCheckFailureLibNull/StackCheckFailureLibNull.inf - MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf -# MU_CHANGE [END] - Add Stack Cookie Support - - - # MS_CHANGE Begin !if $(TOOLCHAIN) == VS2017 or $(TOOLCHAIN) == VS2019 or $(TOOLCHAIN) == VS2022 [Components.IA32] diff --git a/NetworkPkg/NetworkPkg.dsc b/NetworkPkg/NetworkPkg.dsc index dcb1842113..55cf058051 100644 --- a/NetworkPkg/NetworkPkg.dsc +++ b/NetworkPkg/NetworkPkg.dsc @@ -22,10 +22,6 @@ DEFINE NETWORK_ISCSI_ENABLE = TRUE !include MdePkg/MdeLibs.dsc.inc -## MU_CHANGE Begin -[LibraryClasses.ARM, LibraryClasses.AARCH64] - NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf -## MU_CHANGE End [LibraryClasses] DebugLib|MdePkg/Library/BaseDebugLibNull/BaseDebugLibNull.inf @@ -58,6 +54,7 @@ FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf SortLib|MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf + NULL|MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf # MU_CHANGE: /GS and -fstack-protector support [LibraryClasses.common.UEFI_DRIVER] HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf @@ -80,7 +77,7 @@ NULL|MdePkg/Library/CompilerIntrinsicsLib/ArmCompilerIntrinsicsLib.inf !endif # MU_CHANGE End - NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf + # NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf # MU_CHANGE: Use Project Mu StackCheckLib #ArmSoftFloatLib|ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf # MU_CHANGE [LibraryClasses.ARM] diff --git a/NetworkPkg/SharedNetworking/SharedNetworkPkg.dsc b/NetworkPkg/SharedNetworking/SharedNetworkPkg.dsc index e5f098dd31..1881a501e7 100644 --- a/NetworkPkg/SharedNetworking/SharedNetworkPkg.dsc +++ b/NetworkPkg/SharedNetworking/SharedNetworkPkg.dsc @@ -30,12 +30,6 @@ NETWORK_SNP_ENABLE = TRUE !include NetworkPkg/NetworkDefines.dsc.inc - -## MU_CHANGE Begin -[LibraryClasses.ARM, LibraryClasses.AARCH64] - NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf -## MU_CHANGE End - [LibraryClasses] !include NetworkPkg/NetworkLibs.dsc.inc DebugLib|MdePkg/Library/UefiDebugLibDebugPortProtocol/UefiDebugLibDebugPortProtocol.inf @@ -66,6 +60,7 @@ FileHandleLib|MdePkg/Library/UefiFileHandleLib/UefiFileHandleLib.inf FileExplorerLib|MdeModulePkg/Library/FileExplorerLib/FileExplorerLib.inf SortLib|MdeModulePkg/Library/UefiSortLib/UefiSortLib.inf + NULL|MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf # MU_CHANGE: /GS and -fstack-protector support [LibraryClasses.common.DXE_CORE, LibraryClasses.common.DXE_DRIVER, LibraryClasses.common.DXE_RUNTIME_DRIVER, LibraryClasses.common.UEFI_DRIVER] # Common\MU_TIANO\CryptoPkg\Library\BaseCryptLibOnProtocolPpi\DxeCryptLib.inf @@ -80,12 +75,6 @@ BaseCryptLib|CryptoPkg/Library/BaseCryptLibOnProtocolPpi/SmmCryptLib.inf TlsLib|CryptoPkg/Library/BaseCryptLibOnProtocolPpi/SmmCryptLib.inf -# MU_CHANGE [BEGIN] - Add Stack Cookie Support -[LibraryClasses.X64] - NULL|MdePkg/Library/StackCheckLib/StackCheckLib.inf - StackCheckFailureLib|MdePkg/Library/StackCheckFailureLibNull/StackCheckFailureLibNull.inf -# MU_CHANGE [END] - Add Stack Cookie Support - [LibraryClasses.DXE_RUNTIME_DRIVER, LibraryClasses.DXE_CORE] DebugLib|MdePkg/Library/UefiDebugLibDebugPortProtocol/UefiDebugLibDebugPortProtocol.inf ReportStatusCodeLib|MdeModulePkg/Library/DxeReportStatusCodeLib/DxeReportStatusCodeLib.inf @@ -126,7 +115,7 @@ NULL|MdePkg/Library/CompilerIntrinsicsLib/ArmCompilerIntrinsicsLib.inf # MU_CHANGE BaseMemoryLib|MdePkg/Library/BaseMemoryLib/BaseMemoryLib.inf # while building with MSVC, we can't process the s files !endif - NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf + # NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf # MU_CHANGE: Use Project Mu StackCheckLib # ArmSoftFloatLib|ArmPkg/Library/ArmSoftFloatLib/ArmSoftFloatLib.inf # MU_CHANGE diff --git a/PcAtChipsetPkg/PcAtChipsetPkg.dsc b/PcAtChipsetPkg/PcAtChipsetPkg.dsc index c3e52bbea3..587e3135c9 100644 --- a/PcAtChipsetPkg/PcAtChipsetPkg.dsc +++ b/PcAtChipsetPkg/PcAtChipsetPkg.dsc @@ -45,6 +45,7 @@ ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf HobLib|MdePkg/Library/DxeHobLib/DxeHobLib.inf VariablePolicyHelperLib|MdeModulePkg/Library/VariablePolicyHelperLib/VariablePolicyHelperLib.inf # MU_CHANGE + NULL|MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf # MU_CHANGE: /GS and -fstack-protector support [Components] PcAtChipsetPkg/HpetTimerDxe/HpetTimerDxe.inf diff --git a/PolicyServicePkg/PolicyServicePkg.dsc b/PolicyServicePkg/PolicyServicePkg.dsc index 5db47220c7..25ef6cc4ed 100644 --- a/PolicyServicePkg/PolicyServicePkg.dsc +++ b/PolicyServicePkg/PolicyServicePkg.dsc @@ -34,16 +34,11 @@ UnitTestPersistenceLib|UnitTestFrameworkPkg/Library/UnitTestPersistenceLibNull/UnitTestPersistenceLibNull.inf UnitTestResultReportLib|UnitTestFrameworkPkg/Library/UnitTestResultReportLib/UnitTestResultReportLibDebugLib.inf -# MU_CHANGE [BEGIN] - Add Stack Cookie Support -[LibraryClasses.X64] - RngLib|MdePkg/Library/BaseRngLib/BaseRngLib.inf - NULL|MdePkg/Library/StackCheckLib/StackCheckLib.inf - StackCheckFailureLib|MdePkg/Library/StackCheckFailureLibNull/StackCheckFailureLibNull.inf -# MU_CHANGE [END] - Add Stack Cookie Support + NULL|MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf # MU_CHANGE: /GS and -fstack-protector support [LibraryClasses.ARM, LibraryClasses.AARCH64] NULL|MdePkg/Library/CompilerIntrinsicsLib/ArmCompilerIntrinsicsLib.inf - NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf + # NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf # MU_CHANGE: Use Project Mu StackCheckLib [LibraryClasses.common.PEIM] MemoryAllocationLib|MdePkg/Library/PeiMemoryAllocationLib/PeiMemoryAllocationLib.inf diff --git a/ShellPkg/ShellPkg.dsc b/ShellPkg/ShellPkg.dsc index ccd16155fd..32b8851ea3 100644 --- a/ShellPkg/ShellPkg.dsc +++ b/ShellPkg/ShellPkg.dsc @@ -68,6 +68,7 @@ ReportStatusCodeLib|MdePkg/Library/BaseReportStatusCodeLibNull/BaseReportStatusCodeLibNull.inf SafeIntLib|MdePkg/Library/BaseSafeIntLib/BaseSafeIntLib.inf # MU_CHANGE - CodeQL change + NULL|MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf # MU_CHANGE: /GS and -fstack-protector support [LibraryClasses.ARM,LibraryClasses.AARCH64] # @@ -79,7 +80,7 @@ NULL|MdePkg/Library/CompilerIntrinsicsLib/ArmCompilerIntrinsicsLib.inf # MU_CHANGE # Add support for GCC stack protector - NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf + # NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf # MU_CHANGE: Use Project Mu StackCheckLib [PcdsFixedAtBuild] gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask|0xFF diff --git a/StandaloneMmPkg/StandaloneMmPkg.dsc b/StandaloneMmPkg/StandaloneMmPkg.dsc index 55c769b82f..5c657d65db 100644 --- a/StandaloneMmPkg/StandaloneMmPkg.dsc +++ b/StandaloneMmPkg/StandaloneMmPkg.dsc @@ -69,6 +69,7 @@ StandaloneMmDriverEntryPoint|MdePkg/Library/StandaloneMmDriverEntryPoint/StandaloneMmDriverEntryPoint.inf VariableMmDependency|StandaloneMmPkg/Library/VariableMmDependency/VariableMmDependency.inf MmuLib|MdePkg/Library/BaseMmuLibNull/BaseMmuLibNull.inf # MU_CHANGE + NULL|MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf # MU_CHANGE: /GS and -fstack-protector support [LibraryClasses.X64] # MU_CHANGE StandaloneMmCoreEntryPoint|StandaloneMmPkg/Library/StandaloneMmCoreEntryPoint/StandaloneMmCoreEntryPoint.inf # MU_CHANGE @@ -86,7 +87,7 @@ #NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf NULL|MdePkg/Library/CompilerIntrinsicsLib/ArmCompilerIntrinsicsLib.inf # MU_CHANGE [END] - NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf + # NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf # MU_CHANGE: Use Project Mu StackCheckLib [LibraryClasses.common.MM_CORE_STANDALONE] HobLib|StandaloneMmPkg/Library/StandaloneMmCoreHobLib/StandaloneMmCoreHobLib.inf diff --git a/UefiCpuPkg/UefiCpuPkg.dsc b/UefiCpuPkg/UefiCpuPkg.dsc index cd23b3755e..df9ea1e583 100644 --- a/UefiCpuPkg/UefiCpuPkg.dsc +++ b/UefiCpuPkg/UefiCpuPkg.dsc @@ -34,7 +34,7 @@ # #NULL|ArmPkg/Library/CompilerIntrinsicsLib/CompilerIntrinsicsLib.inf NULL|MdePkg/Library/CompilerIntrinsicsLib/ArmCompilerIntrinsicsLib.inf - NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf + # NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf # MU_CHANGE: Use Project Mu StackCheckLib ## MU_CHANGE End [LibraryClasses] @@ -87,6 +87,7 @@ DeviceStateLib|MdeModulePkg/Library/DeviceStateLib/DeviceStateLib.inf PanicLib|MdePkg/Library/BasePanicLibNull/BasePanicLibNull.inf # MU_CHANGE + NULL|MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf # MU_CHANGE: /GS and -fstack-protector support [LibraryClasses.X64, LibraryClasses.IA32] HwResetSystemLib|MdeModulePkg/Library/BaseResetSystemLibNull/BaseResetSystemLibNull.inf ##MSCHANGE End diff --git a/UnitTestFrameworkPkg/UnitTestFrameworkPkgTarget.dsc.inc b/UnitTestFrameworkPkg/UnitTestFrameworkPkgTarget.dsc.inc index fa89dcd5ec..0f479d96f9 100644 --- a/UnitTestFrameworkPkg/UnitTestFrameworkPkgTarget.dsc.inc +++ b/UnitTestFrameworkPkg/UnitTestFrameworkPkgTarget.dsc.inc @@ -27,12 +27,7 @@ UnitTestLib|UnitTestFrameworkPkg/Library/UnitTestLib/UnitTestLib.inf -# MU_CHANGE [BEGIN] - Add Stack Cookie Support -[LibraryClasses.X64] - RngLib|MdePkg/Library/BaseRngLib/BaseRngLib.inf - NULL|MdePkg/Library/StackCheckLib/StackCheckLib.inf - StackCheckFailureLib|MdePkg/Library/StackCheckFailureLibNull/StackCheckFailureLibNull.inf -# MU_CHANGE [END] - Add Stack Cookie Support + NULL|MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf # MU_CHANGE: /GS and -fstack-protector support [LibraryClasses.ARM, LibraryClasses.AARCH64] # @@ -49,7 +44,7 @@ # Since software stack checking may be heuristically enabled by the compiler # include BaseStackCheckLib unconditionally. # - NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf + # NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf # MU_CHANGE: Use Project Mu StackCheckLib [LibraryClasses.common.PEIM] HobLib|MdePkg/Library/PeiHobLib/PeiHobLib.inf From 03cb1dc4c18b19af7754c5b7dc01bd90b41ee257 Mon Sep 17 00:00:00 2001 From: Taylor Beebe Date: Wed, 21 Feb 2024 15:31:21 -0800 Subject: [PATCH 6/7] Update StackCheckFailureLib to StackCheckFailureHookLib, Add Failure Address as Argument Description To clarify the purpose of StackCheckFailureLib, this PR renames it to StackCheckFailureHookLib. Also, the failure address is passed as an argument to the hook function to allow the hook to trace the fault. An interrupt will still be called after the hook returns. - [x] Impacts functionality? - **Functionality** - Does the change ultimately impact how firmware functions? - Examples: Add a new library, publish a new PPI, update an algorithm, ... - [x] Impacts security? - **Security** - Does the change have a direct security impact on an application, flow, or firmware? - Examples: Crypto algorithm change, buffer overflow fix, parameter validation improvement, ... - [x] Breaking change? - **Breaking change** - Will anyone consuming this change experience a break in build or boot behavior? - Examples: Add a new library class, move a module to a different repo, call a function in a new library class in a pre-existing module, ... - [ ] Includes tests? - **Tests** - Does the change include any explicit test code? - Examples: Unit tests, integration tests, robot tests, ... - [ ] Includes documentation? - **Documentation** - Does the change contain explicit documentation additions outside direct code modifications (and comments)? - Examples: Update readme file, add feature readme file, link to documentation on an a separate Web page, ... How This Was Tested Tested on MSVC and GCC builds of Q35 and a GCC build of SBSA by purposefully corrupting the stack. Integration Instructions Platforms will need to update their StackCheckFailureLib instance to the new StackCheckFailureHookLib. EXAMPLE: ``` StackCheckFailureHookLib|MdePkg/Library/StackCheckFailureHookLibNull/StackCheckFailureHookLibNull.inf ``` --- .../Library/StackCheckFailureHookLib.h | 20 ++++++++++++++++ MdePkg/Include/Library/StackCheckFailureLib.h | 19 --------------- .../StackCheckFailureHook.c | 22 +++++++++++++++++ .../StackCheckFailureHookLibNull.inf | 20 ++++++++++++++++ .../StackCheckFailureLibNull.c | 19 --------------- .../StackCheckFailureLibNull.inf | 24 ------------------- .../StackCheckLib/StackCheckLibCommon.c | 3 +++ .../StackCheckLibDynamicInit.inf | 1 + .../StackCheckLib/StackCheckLibStaticInit.inf | 3 +++ MdePkg/MdePkg.dec | 2 +- MdePkg/MdePkg.dsc | 3 +-- 11 files changed, 71 insertions(+), 65 deletions(-) create mode 100644 MdePkg/Include/Library/StackCheckFailureHookLib.h delete mode 100644 MdePkg/Include/Library/StackCheckFailureLib.h create mode 100644 MdePkg/Library/StackCheckFailureHookLibNull/StackCheckFailureHook.c create mode 100644 MdePkg/Library/StackCheckFailureHookLibNull/StackCheckFailureHookLibNull.inf delete mode 100644 MdePkg/Library/StackCheckFailureLibNull/StackCheckFailureLibNull.c delete mode 100644 MdePkg/Library/StackCheckFailureLibNull/StackCheckFailureLibNull.inf diff --git a/MdePkg/Include/Library/StackCheckFailureHookLib.h b/MdePkg/Include/Library/StackCheckFailureHookLib.h new file mode 100644 index 0000000000..44c71aa1b1 --- /dev/null +++ b/MdePkg/Include/Library/StackCheckFailureHookLib.h @@ -0,0 +1,20 @@ +/** @file + Library provides a hook called when a stack cookie check fails. + + Copyright (c) Microsoft Corporation. All rights reserved. + SPDX-License-Identifier: BSD-2-Clause-Patent +**/ + +#ifndef STACK_COOKIE_FAILURE_HOOK_LIB_H_ +#define STACK_COOKIE_FAILURE_HOOK_LIB_H_ + +#include + +NO_STACK_COOKIE +VOID +EFIAPI +StackCheckFailureHook ( + VOID *FailureAddress + ); + +#endif diff --git a/MdePkg/Include/Library/StackCheckFailureLib.h b/MdePkg/Include/Library/StackCheckFailureLib.h deleted file mode 100644 index fc621ee39e..0000000000 --- a/MdePkg/Include/Library/StackCheckFailureLib.h +++ /dev/null @@ -1,19 +0,0 @@ -/** @file - Library provides the handler function for stack cookie check failures. - - Copyright (c) Microsoft Corporation. All rights reserved. - SPDX-License-Identifier: BSD-2-Clause-Patent -**/ - -#ifndef STACK_COOKIE_FAILURE_HANDLER_LIB_H_ -#define STACK_COOKIE_FAILURE_HANDLER_LIB_H_ - -#include - -VOID -EFIAPI -StackCheckFailure ( - VOID - ); - -#endif diff --git a/MdePkg/Library/StackCheckFailureHookLibNull/StackCheckFailureHook.c b/MdePkg/Library/StackCheckFailureHookLibNull/StackCheckFailureHook.c new file mode 100644 index 0000000000..f159e0c2b5 --- /dev/null +++ b/MdePkg/Library/StackCheckFailureHookLibNull/StackCheckFailureHook.c @@ -0,0 +1,22 @@ +/** @file + Library provides a hook called when a stack cookie check fails. + + Copyright (c) Microsoft Corporation. All rights reserved. + SPDX-License-Identifier: BSD-2-Clause-Patent +**/ + +#include +#include + +/** + Initialize the security cookie. +**/ +NO_STACK_COOKIE +VOID +EFIAPI +StackCheckFailureHook ( + VOID *FailureAddress + ) +{ + return; +} diff --git a/MdePkg/Library/StackCheckFailureHookLibNull/StackCheckFailureHookLibNull.inf b/MdePkg/Library/StackCheckFailureHookLibNull/StackCheckFailureHookLibNull.inf new file mode 100644 index 0000000000..dfc74fcf11 --- /dev/null +++ b/MdePkg/Library/StackCheckFailureHookLibNull/StackCheckFailureHookLibNull.inf @@ -0,0 +1,20 @@ +## @file +# Library provides a hook called when a stack cookie check fails. +# +# Copyright (c) Microsoft Corporation. All rights reserved. +# SPDX-License-Identifier: BSD-2-Clause-Patent +## + +[Defines] + INF_VERSION = 0x00010005 + 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/StackCheckFailureLibNull/StackCheckFailureLibNull.c b/MdePkg/Library/StackCheckFailureLibNull/StackCheckFailureLibNull.c deleted file mode 100644 index 35e41fc1a8..0000000000 --- a/MdePkg/Library/StackCheckFailureLibNull/StackCheckFailureLibNull.c +++ /dev/null @@ -1,19 +0,0 @@ -/** @file - Provides the handler when a stack cookie check failure occurs. - - Copyright (c) Microsoft Corporation. All rights reserved. - SPDX-License-Identifier: BSD-2-Clause-Patent -**/ - -#include -#include -#include - -VOID -EFIAPI -StackCheckFailure ( - VOID - ) -{ - return; -} diff --git a/MdePkg/Library/StackCheckFailureLibNull/StackCheckFailureLibNull.inf b/MdePkg/Library/StackCheckFailureLibNull/StackCheckFailureLibNull.inf deleted file mode 100644 index 6522358f83..0000000000 --- a/MdePkg/Library/StackCheckFailureLibNull/StackCheckFailureLibNull.inf +++ /dev/null @@ -1,24 +0,0 @@ -## @file -# Provides the handler when a stack cookie check failure occurs. -# -# Copyright (c) Microsoft Corporation. All rights reserved. -# SPDX-License-Identifier: BSD-2-Clause-Patent -## - -[Defines] - INF_VERSION = 0x00010005 - BASE_NAME = StackCheckFailureLibNull - FILE_GUID = 0a4f7cde-a567-485d-8c9a-98ea271ba322 - MODULE_TYPE = BASE - VERSION_STRING = 1.0 - LIBRARY_CLASS = StackCheckFailureLib - -[Sources] - StackCheckFailureLibNull.c - -[Packages] - MdePkg/MdePkg.dec - -[LibraryClasses] - BaseLib - DebugLib diff --git a/MdePkg/Library/StackCheckLib/StackCheckLibCommon.c b/MdePkg/Library/StackCheckLib/StackCheckLibCommon.c index 256e6003e9..d9fa3a563e 100644 --- a/MdePkg/Library/StackCheckLib/StackCheckLibCommon.c +++ b/MdePkg/Library/StackCheckLib/StackCheckLibCommon.c @@ -10,6 +10,7 @@ #include #include +#include /** Calls an interrupt using the vector specified by PcdStackCookieExceptionVector @@ -28,6 +29,7 @@ __stack_chk_fail ( VOID ) { + StackCheckFailureHook (RETURN_ADDRESS (0)); TriggerStackCookieInterrupt (); } @@ -55,6 +57,7 @@ StackCheckFailure ( VOID *ActualCookieValue ) { + StackCheckFailureHook (RETURN_ADDRESS (0)); TriggerStackCookieInterrupt (); } diff --git a/MdePkg/Library/StackCheckLib/StackCheckLibDynamicInit.inf b/MdePkg/Library/StackCheckLib/StackCheckLibDynamicInit.inf index 8861424a59..a2cb4c7858 100644 --- a/MdePkg/Library/StackCheckLib/StackCheckLibDynamicInit.inf +++ b/MdePkg/Library/StackCheckLib/StackCheckLibDynamicInit.inf @@ -39,6 +39,7 @@ [LibraryClasses] RngLib + StackCheckFailureHookLib [FixedPcd] gEfiMdePkgTokenSpaceGuid.PcdStackCookieExceptionVector diff --git a/MdePkg/Library/StackCheckLib/StackCheckLibStaticInit.inf b/MdePkg/Library/StackCheckLib/StackCheckLibStaticInit.inf index 5fdf63d70b..ba356ad724 100644 --- a/MdePkg/Library/StackCheckLib/StackCheckLibStaticInit.inf +++ b/MdePkg/Library/StackCheckLib/StackCheckLibStaticInit.inf @@ -34,6 +34,9 @@ [Packages] MdePkg/MdePkg.dec +[LibraryClasses] + StackCheckFailureHookLib + [FixedPcd] gEfiMdePkgTokenSpaceGuid.PcdStackCookieExceptionVector diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index b92990453e..e23798672c 100644 --- a/MdePkg/MdePkg.dec +++ b/MdePkg/MdePkg.dec @@ -104,7 +104,7 @@ ## MU_CHANGE [BEGIN] ## @libraryclass Provides a hook called when a stack cookie check fails. - StackCheckFailureLib|Include/Library/StackCheckFailureLib.h + StackCheckFailureHookLib|Include/Library/StackCheckFailureHookLib.h ## MU_CHANGE [END] ## @libraryclass Provides an ordered collection data structure. diff --git a/MdePkg/MdePkg.dsc b/MdePkg/MdePkg.dsc index 3072fdbab6..d83727c52e 100644 --- a/MdePkg/MdePkg.dsc +++ b/MdePkg/MdePkg.dsc @@ -140,10 +140,9 @@ MdePkg/Library/SmmCpuRendezvousLibNull/SmmCpuRendezvousLibNull.inf MdePkg/Library/BaseMmuLibNull/BaseMmuLibNull.inf ## MU_CHANGE - MdePkg/Library/StackCheckFailureLibNull/StackCheckFailureLibNull.inf ## MU_CHANGE MdePkg/Library/StackCheckLib/StackCheckLibStaticInit.inf ## MU_CHANGE MdePkg/Library/StackCheckLib/StackCheckLibDynamicInit.inf ## MU_CHANGE - MdePkg/Library/StackCheckFailureLib/StackCheckFailureLib.inf ## MU_CHANGE + MdePkg/Library/StackCheckFailureHookLibNull/StackCheckFailureHookLibNull.inf ## MU_CHANGE MdePkg/Library/StackCheckLibNull/StackCheckLibNull.inf ## MU_CHANGE [Components.IA32, Components.X64, Components.ARM, Components.AARCH64] From c0fe77901034c8790432ab9862fcb88d0ce0fb25 Mon Sep 17 00:00:00 2001 From: Taylor Beebe Date: Wed, 31 Jan 2024 17:03:51 -0800 Subject: [PATCH 7/7] Update Toolsdef to Add Stack Cookies for MSVC IA32, and GCC Arm and AARCH64 Description This change adds stack cookies to the build commands for IA32 modules built with VS2019 and VS2022. It also adds stack cookies to GCC5 AARCH64 builds. - [x] Impacts functionality? - **Functionality** - Does the change ultimately impact how firmware functions? - Examples: Add a new library, publish a new PPI, update an algorithm, ... - [ ] Impacts security? - **Security** - Does the change have a direct security impact on an application, flow, or firmware? - Examples: Crypto algorithm change, buffer overflow fix, parameter validation improvement, ... - [x] Breaking change? - **Breaking change** - Will anyone consuming this change experience a break in build or boot behavior? - Examples: Add a new library class, move a module to a different repo, call a function in a new library class in a pre-existing module, ... - [ ] Includes tests? - **Tests** - Does the change include any explicit test code? - Examples: Unit tests, integration tests, robot tests, ... - [ ] Includes documentation? - **Documentation** - Does the change contain explicit documentation additions outside direct code modifications (and comments)? - Examples: Update readme file, add feature readme file, link to documentation on an a separate Web page, ... How This Was Tested Tested by on Q35 GCC and MSVC builds and an SBSA GCC build by purposefully overflowing the stack. Integration Instructions Platforms will need to update to ensure that all modules have a StackCheckLib instanced linked. The instances are: StackCheckLibNull: Used to provide the cookie definitions but not actually check the stack. StackCheckLibBuildInit: Used to provide the cookie definitions where the stack cookie value is initialized at build time. StackCheckLibRuntimeInit: Used to provide the cookie definitions where the stack cookie value is initialized at runtime. --- BaseTools/Conf/tools_def.template | 21 +++++++++++---------- 1 file changed, 11 insertions(+), 10 deletions(-) diff --git a/BaseTools/Conf/tools_def.template b/BaseTools/Conf/tools_def.template index 3a979d4834..d34aefa964 100755 --- a/BaseTools/Conf/tools_def.template +++ b/BaseTools/Conf/tools_def.template @@ -25,8 +25,9 @@ # 2.43 - Enable stack cookies to VS2019 and VS20222 X64 builds via /GS flag # 2.44 - Add Rust build support # 2.45 - Enable stack cookies to GCC X64 builds via -fstack-protector flag +# 2.46 - Enable stack cookies for IA32, ARM, and AARCH64 builds for GCC and MSVC # -#!VERSION=2.45 +#!VERSION=2.46 IDENTIFIER = Default TOOL_CHAIN_CONF @@ -1740,9 +1741,9 @@ NOOPT_VS2017_AARCH64_DLINK_FLAGS = /NOLOGO /NODEFAULTLIB /IGNORE:4001 /OPT:REF *_VS2019_IA32_PP_PATH = DEF(VS2019_BIN_IA32)\cl.exe *_VS2019_IA32_ASM_PATH = DEF(VS2019_BIN_IA32)\ml.exe - DEBUG_VS2019_IA32_CC_FLAGS = /nologo /arch:IA32 /c /WX /GS- /W4 /Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Z7 /Gw -RELEASE_VS2019_IA32_CC_FLAGS = /nologo /arch:IA32 /c /WX /GS- /W4 /Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gw -NOOPT_VS2019_IA32_CC_FLAGS = /nologo /arch:IA32 /c /WX /GS- /W4 /Gs32768 /D UNICODE /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Z7 /Od + DEBUG_VS2019_IA32_CC_FLAGS = /nologo /arch:IA32 /c /WX /GS /W4 /Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Z7 /Gw +RELEASE_VS2019_IA32_CC_FLAGS = /nologo /arch:IA32 /c /WX /GS /W4 /Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gw +NOOPT_VS2019_IA32_CC_FLAGS = /nologo /arch:IA32 /c /WX /GS /W4 /Gs32768 /D UNICODE /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Z7 /Od DEBUG_VS2019_IA32_ASM_FLAGS = /nologo /c /WX /W3 /Cx /coff /Zd /Zi RELEASE_VS2019_IA32_ASM_FLAGS = /nologo /c /WX /W3 /Cx /coff /Zd @@ -1899,9 +1900,9 @@ NOOPT_VS2019_AARCH64_DLINK_FLAGS = /NOLOGO /NODEFAULTLIB /IGNORE:4001 /OPT:REF *_VS2022_IA32_ASM_PATH = DEF(VS2022_BIN_IA32)\ml.exe *_VS2022_IA32_MAKE_FLAGS = /nologo - DEBUG_VS2022_IA32_CC_FLAGS = /nologo /arch:IA32 /c /WX /GS- /W4 /Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Z7 /Gw -RELEASE_VS2022_IA32_CC_FLAGS = /nologo /arch:IA32 /c /WX /GS- /W4 /Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gw -NOOPT_VS2022_IA32_CC_FLAGS = /nologo /arch:IA32 /c /WX /GS- /W4 /Gs32768 /D UNICODE /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Z7 /Od + DEBUG_VS2022_IA32_CC_FLAGS = /nologo /arch:IA32 /c /WX /GS /W4 /Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Z7 /Gw +RELEASE_VS2022_IA32_CC_FLAGS = /nologo /arch:IA32 /c /WX /GS /W4 /Gs32768 /D UNICODE /O1b2 /GL /FIAutoGen.h /EHs-c- /GR- /GF /Gw +NOOPT_VS2022_IA32_CC_FLAGS = /nologo /arch:IA32 /c /WX /GS /W4 /Gs32768 /D UNICODE /FIAutoGen.h /EHs-c- /GR- /GF /Gy /Z7 /Od DEBUG_VS2022_IA32_ASM_FLAGS = /nologo /c /WX /W3 /Cx /coff /Zd /Zi RELEASE_VS2022_IA32_ASM_FLAGS = /nologo /c /WX /W3 /Cx /coff /Zd @@ -2033,7 +2034,7 @@ NOOPT_*_*_OBJCOPY_ADDDEBUGFLAG = --add-gnu-debuglink=$(DEBUG_DIR)/$(MODULE_N *_*_*_DTCPP_PATH = DEF(DTCPP_BIN) *_*_*_DTC_PATH = DEF(DTC_BIN) -DEFINE GCC_ALL_CC_FLAGS = -g -Os -fshort-wchar -fno-builtin -fno-strict-aliasing -Wall -Werror -Wno-array-bounds -include AutoGen.h -fno-common +DEFINE GCC_ALL_CC_FLAGS = -g -Os -fshort-wchar -fno-builtin -fno-strict-aliasing -Wall -Werror -Wno-array-bounds -include AutoGen.h -fno-common -fstack-protector -mstack-protector-guard=global DEFINE GCC_IA32_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -m32 -malign-double -freorder-blocks -freorder-blocks-and-partition -O2 -mno-stack-arg-probe DEFINE GCC_X64_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -mno-red-zone -Wno-address -mno-stack-arg-probe DEFINE GCC_ARM_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -mlittle-endian -mabi=aapcs -fno-short-enums -funsigned-char -ffunction-sections -fdata-sections -fomit-frame-pointer -Wno-address -mthumb -fno-pic -fno-pie @@ -2074,8 +2075,8 @@ DEFINE GCC_DEPS_FLAGS = -MMD -MF $@.deps DEFINE GCC48_ALL_CC_FLAGS = DEF(GCC_ALL_CC_FLAGS) -ffunction-sections -fdata-sections -DSTRING_ARRAY_NAME=$(BASE_NAME)Strings DEFINE GCC48_IA32_X64_DLINK_COMMON = -nostdlib -Wl,-n,-q,--gc-sections -z common-page-size=0x20 -DEFINE GCC48_IA32_CC_FLAGS = DEF(GCC48_ALL_CC_FLAGS) -m32 -march=i586 -malign-double -fno-stack-protector -D EFI32 -fno-asynchronous-unwind-tables -Wno-address -fno-omit-frame-pointer -DEFINE GCC48_X64_CC_FLAGS = DEF(GCC48_ALL_CC_FLAGS) -m64 -fstack-protector -mstack-protector-guard=global "-DEFIAPI=__attribute__((ms_abi))" -maccumulate-outgoing-args -mno-red-zone -Wno-address -mcmodel=small -fpie -fno-asynchronous-unwind-tables -Wno-address -fno-omit-frame-pointer +DEFINE GCC48_IA32_CC_FLAGS = DEF(GCC48_ALL_CC_FLAGS) -m32 -march=i586 -malign-double -D EFI32 -fno-asynchronous-unwind-tables -Wno-address -fno-omit-frame-pointer +DEFINE GCC48_X64_CC_FLAGS = DEF(GCC48_ALL_CC_FLAGS) -m64 "-DEFIAPI=__attribute__((ms_abi))" -maccumulate-outgoing-args -mno-red-zone -Wno-address -mcmodel=small -fpie -fno-asynchronous-unwind-tables -Wno-address -fno-omit-frame-pointer DEFINE GCC48_IA32_X64_ASLDLINK_FLAGS = DEF(GCC48_IA32_X64_DLINK_COMMON) -Wl,--entry,ReferenceAcpiTable -u ReferenceAcpiTable DEFINE GCC48_IA32_X64_DLINK_FLAGS = DEF(GCC48_IA32_X64_DLINK_COMMON) -Wl,--entry,$(IMAGE_ENTRY_POINT) -u $(IMAGE_ENTRY_POINT) -Wl,-Map,$(DEST_DIR_DEBUG)/$(BASE_NAME).map,--whole-archive DEFINE GCC48_IA32_DLINK2_FLAGS = -Wl,--defsym=PECOFF_HEADER_SIZE=0x220 DEF(GCC_DLINK2_FLAGS_COMMON)