Jian, Is there a link to the documentation for these new functions? We should put the link in the file headers.
The link to docs should also be added to the BZ. Also, which VS compilers was this tested with? Are different implementations required for different VS compilers? Mike > -----Original Message----- > From: Gao, Liming > Sent: Wednesday, October 17, 2018 6:22 PM > To: Wang, Jian J <[email protected]>; edk2-devel > <[email protected]>; edk2- > [email protected] > Cc: Kinney, Michael D <[email protected]>; Yao, > Jiewen <[email protected]>; Andrew Fish > <[email protected]> > Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib: add > MSFT toolchain support > > Jian: > The patch is good to me. Reviewed-by: Liming Gao > <[email protected]> > > Thanks > Liming > >-----Original Message----- > >From: Wang, Jian J > >Sent: Thursday, October 18, 2018 9:19 AM > >To: edk2-devel <[email protected]>; edk2- > [email protected] > >Cc: Kinney, Michael D <[email protected]>; Yao, > Jiewen > ><[email protected]>; Andrew Fish <[email protected]>; > Gao, Liming > ><[email protected]> > >Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib: > add MSFT toolchain > >support > > > >Ping ... :) > > > >Regards, > >Jian > > > > > >> -----Original Message----- > >> From: edk2-devel [mailto:edk2-devel- > [email protected]] > >> Sent: Tuesday, October 16, 2018 8:59 AM > >> To: edk2-devel <[email protected]>; > edk2- > >[email protected] > >> Cc: Kinney, Michael D <[email protected]>; > Yao, Jiewen > >> <[email protected]>; Andrew Fish <[email protected]>; > Gao, Liming > >> <[email protected]> > >> Subject: Re: [edk2] [PATCH] MdePkg/BaseStackCheckLib: > add MSFT > >toolchain > >> support > >> > >> REF: > https://bugzilla.tianocore.org/show_bug.cgi?id=1239 > >> > >> Regards, > >> Jian > >> > >> > >> > -----Original Message----- > >> > From: edk2-devel [mailto:edk2-devel- > [email protected]] > >> > Sent: Tuesday, October 16, 2018 8:55 AM > >> > To: [email protected] > >> > Cc: Kinney, Michael D <[email protected]>; > Yao, Jiewen > >> > <[email protected]>; Andrew Fish > <[email protected]>; Gao, Liming > >> > <[email protected]> > >> > Subject: [edk2] [PATCH] MdePkg/BaseStackCheckLib: > add MSFT toolchain > >> > support > >> > > >> > This patch adds stack check support for MSFT > toolchain, with > >> > compiler option /GS and /RTCs. This functionality is > similar > >> > to the original ones supported by GCC toolchain. > >> > > >> > Usage example: > >> > This is a NULL library instance. Add it under a > [LibraryClasses] > >> > section in dsc file to let it be built into all > modules employed > >> > in a platform. > >> > > >> > [LibraryClasses] > >> > > NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.i > nf > >> > > >> > Please note all not modules can be built against > this library. Most > >> > of them are SEC type of modules, such as > >> > > >> > OvmfPkg/ResetVector/ResetVector.inf > >> > > >> > In this case, this library should not be added to a > common > >> > [LibraryClasses] section but to specific ones, like > >> > [LibraryClasses.common.PEI_CORE/PEIM/...]. > >> > > >> > In addition, /GS and/or /RTCs should be added to > compiler command line. > >> > This can be done by adding something like below > under [BuildOptions] > >> > section in dsc file. > >> > > >> > [BuildOptions] > >> > MSFT:DEBUG_*_*_CC_FLAGS = /GS /GL- > >> > MSFT:DEBUG_*_*_CC_FLAGS = /RTCs /Od > >> > > >> > Note: /GL- is required for /GS, and /Od is required > for /RTCs. > >> > Note: The flash layout might be needed to update to > accommodate larger > >> > image size due to /Od is enforced. > >> > > >> > Pass tests: > >> > a. Overwrite a local buffer variable (in a 32-bit > and 64-bit driver)and > >> > check if it's caught by new code (on both real > platform and virtual > >> > platform) > >> > b. Boot Windows 10 and Ubuntu 18.04 on real platform > with this > >> > lib built-in > >> > > >> > Cc: Michael D Kinney <[email protected]> > >> > Cc: Liming Gao <[email protected]> > >> > Cc: Jiewen Yao <[email protected]> > >> > Cc: Andrew Fish <[email protected]> > >> > Contributed-under: TianoCore Contribution Agreement > 1.1 > >> > Signed-off-by: Jian J Wang <[email protected]> > >> > --- > >> > .../BaseStackCheckLib/BaseStackCheckLib.inf > | 11 +- > >> > .../Library/BaseStackCheckLib/BaseStackCheckMsft.c > | 221 > >> > +++++++++++++++++++++ > >> > .../Library/BaseStackCheckLib/BaseStackCheckNull.c > | 15 -- > >> > .../BaseStackCheckLib/Ia32/StackCheckStubAsm.nasm > | 76 +++++++ > >> > .../BaseStackCheckLib/X64/StackCheckStubAsm.nasm > | 54 +++++ > >> > 5 files changed, 360 insertions(+), 17 deletions(-) > >> > create mode 100644 > >> > > MdePkg/Library/BaseStackCheckLib/BaseStackCheckMsft.c > >> > delete mode 100644 > >> MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c > >> > create mode 100644 > >> > > MdePkg/Library/BaseStackCheckLib/Ia32/StackCheckStubAsm.n > asm > >> > create mode 100644 > >> > > MdePkg/Library/BaseStackCheckLib/X64/StackCheckStubAsm.na > sm > >> > > >> > diff --git > a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > >> > > b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > >> > index e280651b11..1c9e6710c6 100644 > >> > --- > a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > >> > +++ > b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > >> > @@ -4,6 +4,7 @@ > >> > # Stack Check Library > >> > # > >> > # Copyright (c) 2014, ARM Ltd. All rights > reserved.<BR> > >> > +# Copyright (c) 2018, Intel Corporation. All > rights reserved.<BR> > >> > # > >> > # This program and the accompanying materials > >> > # are licensed and made available under the terms > and conditions of the > >BSD > >> > License > >> > @@ -26,13 +27,19 @@ > >> > > >> > > >> > # > >> > -# VALID_ARCHITECTURES = ARM AARCH64 > >> > +# VALID_ARCHITECTURES = ARM AARCH64 IA32 > X64 > >> > # > >> > > >> > [Sources] > >> > BaseStackCheckGcc.c | GCC > >> > BaseStackCheckGcc.c | RVCT > >> > - BaseStackCheckNull.c | MSFT > >> > + BaseStackCheckMsft.c | MSFT > >> > + > >> > +[Sources.IA32] > >> > + Ia32/StackCheckStubAsm.nasm | MSFT > >> > + > >> > +[Sources.X64] > >> > + X64/StackCheckStubAsm.nasm | MSFT > >> > > >> > [Packages] > >> > MdePkg/MdePkg.dec > >> > diff --git > a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckMsft.c > >> > > b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckMsft.c > >> > new file mode 100644 > >> > index 0000000000..951154f0cd > >> > --- /dev/null > >> > +++ > b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckMsft.c > >> > @@ -0,0 +1,221 @@ > >> > +/** @file > >> > + Base Stack Check library for MSFT toolchains > compiler options: /GS, RTCs. > >> > + > >> > +Copyright (c) 2018, Intel Corporation. All rights > reserved.<BR> > >> > +This program and the accompanying materials are > licensed and made > >> available > >> > under > >> > +the terms and conditions of the BSD License that > accompanies this > >> distribution. > >> > +The full text of the license may be found at > >> > +http://opensource.org/licenses/bsd-license.php. > >> > + > >> > +THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE ON > AN "AS IS" > >> BASIS, > >> > +WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, > EITHER > >> EXPRESS > >> > OR IMPLIED. > >> > + > >> > +**/ > >> > + > >> > +#include <Base.h> > >> > + > >> > +#include <Library/BaseLib.h> > >> > +#include <Library/DebugLib.h> > >> > +#include <Library/PcdLib.h> > >> > + > >> > +// > >> > +// cookie value that is inserted by the MSFT > compiler into the stack frame. > >> > +// > >> > +extern UINTN __security_cookie; > >> > + > >> > +// > >> > +// Data structure used by MSFT compiler to record > local variable > >information. > >> > +// > >> > + > >> > +typedef struct _RTC_vardesc { > >> > + int Addr; > >> > + int Size; > >> > + char *Name; > >> > +} _RTC_vardesc; > >> > + > >> > +typedef struct _RTC_framedesc { > >> > + int VarCount; > >> > + _RTC_vardesc *Variables; > >> > +} _RTC_framedesc; > >> > + > >> > +#define RTC_STACK_CHECK_COOKIE 0xCCCCCCCC > >> > + > >> > +/** > >> > + Function called upon unexpected stack pointer > change. > >> > + > >> > + @param Ip Instruction address where the > check happened. > >> > + > >> > +**/ > >> > +VOID > >> > +__cdecl > >> > +_RTC_Failure ( > >> > + VOID *Ip > >> > + ) > >> > +{ > >> > + DEBUG ((DEBUG_ERROR, "\nSTACK FAULT: Suspicious > stack pointer > >> > (IP:%p).\n\n", Ip)); > >> > + > >> > + // > >> > + // Generate a Breakpoint, DeadLoop, or NOP based > on PCD settings > >even if > >> > + // BaseDebugLibNull is in use. > >> > + // > >> > + if ((PcdGet8 (PcdDebugPropertyMask) & > >> > DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) { > >> > + CpuBreakpoint (); > >> > + } else { > >> > + // > >> > + // Usually the boot should stop here if check > failure. Due to the fact > >> > + // that the normal Stack Switch happened in > boot will also fail the stack > >> > + // pointer check. So no dead loop here. > >> > + // > >> > + } > >> > + return; > >> > +} > >> > + > >> > +/** > >> > + Function reporting stack buffer overlow. > >> > + > >> > + @param Name Local varible name. > >> > + @param Ip Instruction address where the > check happened. > >> > + > >> > +**/ > >> > +STATIC > >> > +VOID > >> > +_RTC_StackFailure ( > >> > + CHAR8 *Name, > >> > + VOID *Ip > >> > + ) > >> > +{ > >> > + DEBUG ((DEBUG_ERROR, "\nSTACK FAULT: Local > variable '%a' overflow > >> > (IP:%p).\n\n", Name, Ip)); > >> > + > >> > + // > >> > + // Generate a Breakpoint, DeadLoop, or NOP based > on PCD settings > >even if > >> > + // BaseDebugLibNull is in use. > >> > + // > >> > + if ((PcdGet8 (PcdDebugPropertyMask) & > >> > DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) { > >> > + CpuBreakpoint (); > >> > + } else if ((PcdGet8 (PcdDebugPropertyMask) & > >> > DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) != 0) { > >> > + CpuDeadLoop (); > >> > + } > >> > + return ; > >> > +} > >> > + > >> > +/** > >> > + Function called upon stack buffer overflow. > (/RTCs) > >> > + > >> > + @param _Esp Stack frame pointer. > >> > + @param _Fd Pointer to local variable > information. > >> > + > >> > +**/ > >> > +VOID > >> > +__fastcall > >> > +_RTC_CheckStackVars ( > >> > + VOID *_Esp, > >> > + _RTC_framedesc *_Fd > >> > + ) > >> > +{ > >> > + INTN Index; > >> > + UINT8 *Addr; > >> > + > >> > + for (Index = 0; Index < _Fd->VarCount; Index++) { > >> > + Addr = (UINT8 *)_Esp + _Fd- > >Variables[Index].Addr - sizeof(UINT32); > >> > + if (*(UINT32 *)Addr != RTC_STACK_CHECK_COOKIE) > { > >> > + _RTC_StackFailure (_Fd- > >Variables[Index].Name, > >RETURN_ADDRESS(0)); > >> > + } > >> > + > >> > + Addr = (UINT8 *)_Esp + _Fd- > >Variables[Index].Addr + _Fd- > >> > >Variables[Index].Size; > >> > + if (*(UINT32 *)Addr != RTC_STACK_CHECK_COOKIE) > { > >> > + _RTC_StackFailure (_Fd- > >Variables[Index].Name, > >RETURN_ADDRESS(0)); > >> > + } > >> > + } > >> > +} > >> > + > >> > +/** > >> > + Function required by linker but not implemented > by firmware image > >loader. > >> > (/RTCs) > >> > + > >> > +**/ > >> > +VOID > >> > +__cdecl > >> > +_RTC_Shutdown ( > >> > + VOID > >> > + ) > >> > +{ > >> > + return; > >> > +} > >> > + > >> > +/** > >> > + Function required by linker but not implemented > by firmware image > >loader. > >> > (/RTCs) > >> > + > >> > +**/ > >> > +VOID > >> > +__cdecl > >> > +_RTC_InitBase ( > >> > + VOID > >> > + ) > >> > +{ > >> > + return; > >> > +} > >> > + > >> > + > >> > +/** > >> > + Function called upon stack frame overflow > detected. (/GS) > >> > + > >> > + @param StackCookie Actual cookie value got > from stack boundary. > >> > + @param Ip Instruction address where > the check happened. > >> > + > >> > +**/ > >> > +NORETURN > >> > +VOID > >> > +__cdecl > >> > +__report_gsfailure ( > >> > + UINTN StackCookie, > >> > + VOID *Ip > >> > + ) > >> > +{ > >> > + DEBUG ((DEBUG_ERROR, "\nSTACK FAULT: Stack > overflow check failed > >in > >> > cookie checker (IP:%p).\n\n", Ip)); > >> > + > >> > + // > >> > + // Generate a Breakpoint, DeadLoop, or NOP based > on PCD settings > >even if > >> > + // BaseDebugLibNull is in use. > >> > + // > >> > + if ((PcdGet8 (PcdDebugPropertyMask) & > >> > DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) { > >> > + CpuBreakpoint (); > >> > + } else if ((PcdGet8 (PcdDebugPropertyMask) & > >> > DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) != 0) { > >> > + CpuDeadLoop (); > >> > + } > >> > +} > >> > + > >> > +/** > >> > + Function called upon failure at local array range > check . (/GS) > >> > + > >> > +**/ > >> > +NORETURN > >> > +VOID > >> > +__cdecl > >> > +__report_rangecheckfailure ( > >> > + VOID > >> > + ) > >> > +{ > >> > + DEBUG((DEBUG_ERROR, "\nSTACK FAULT: Range check > check failed in > >> cookie > >> > checker.\n\n")); > >> > + > >> > + // > >> > + // Generate a Breakpoint, DeadLoop, or NOP based > on PCD settings > >even if > >> > + // BaseDebugLibNull is in use. > >> > + // > >> > + if ((PcdGet8 (PcdDebugPropertyMask) & > >> > DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) { > >> > + CpuBreakpoint (); > >> > + } else if ((PcdGet8 (PcdDebugPropertyMask) & > >> > DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) != 0) { > >> > + CpuDeadLoop (); > >> > + } > >> > +} > >> > + > >> > +/** > >> > + Function required by linker but not implemented > by firmware image > >loader. > >> > (/GS) > >> > + > >> > +**/ > >> > +VOID > >> > +__GSHandlerCheck ( > >> > + VOID > >> > + ) > >> > +{ > >> > + return; > >> > +} > >> > + > >> > diff --git > a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c > >> > > b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c > >> > deleted file mode 100644 > >> > index 7c27c73e23..0000000000 > >> > --- > a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c > >> > +++ /dev/null > >> > @@ -1,15 +0,0 @@ > >> > -/** @file > >> > - This file is purely empty as a work around for > BaseStackCheck to pass > >MSVC > >> > build. > >> > - > >> > - Copyright (c) 2018, Intel Corporation. All rights > reserved.<BR> > >> > - This program and the accompanying materials > >> > - are licensed and made available under the terms > and conditions of the > >BSD > >> > License > >> > - which accompanies this distribution. The full > text of the license may be > >> found > >> > at > >> > - http://opensource.org/licenses/bsd-license.php. > >> > - > >> > - THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE > ON AN "AS IS" > >> BASIS, > >> > - WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY KIND, > EITHER > >> EXPRESS > >> > OR IMPLIED. > >> > - > >> > -**/ > >> > - > >> > -extern int __BaseStackCheckNull; > >> > diff --git > >> > a/MdePkg/Library/BaseStackCheckLib/Ia32/StackCheckStubAsm > .nasm > >> > > b/MdePkg/Library/BaseStackCheckLib/Ia32/StackCheckStubAsm > .nasm > >> > new file mode 100644 > >> > index 0000000000..d3c8d32161 > >> > --- /dev/null > >> > +++ > >b/MdePkg/Library/BaseStackCheckLib/Ia32/StackCheckStubAs > m.nasm > >> > @@ -0,0 +1,76 @@ > >> > +;-------------------------------------------------- > ---------------------------- ; > >> > +; Copyright (c) 2018, Intel Corporation. All rights > reserved.<BR> > >> > +; This program and the accompanying materials > >> > +; are licensed and made available under the terms > and conditions of the > >BSD > >> > License > >> > +; which accompanies this distribution. The full > text of the license may be > >> found > >> > at > >> > +; http://opensource.org/licenses/bsd-license.php. > >> > +; > >> > +; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE > ON AN "AS IS" > >> > BASIS, > >> > +; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY > KIND, EITHER > >> > EXPRESS OR IMPLIED. > >> > +; > >> > +; Module Name: > >> > +; > >> > +; StackCheckStubAsm.nasm > >> > +; > >> > +; Abstract: > >> > +; > >> > +; Stub globals and functions for compiler options > /GS, /RTCs > >> > +; > >> > +; Notes: > >> > +; > >> > +;-------------------------------------------------- > ---------------------------- > >> > + > >> > +; > >> > +; __declspec(noreturn) void __cdecl > __report_gsfailure(UINTN cookie, > >void > >> *ip); > >> > +; > >> > +extern ___report_gsfailure > >> > +; > >> > +; void __cdecl _RTC_Failure (void *Ip); > >> > +; > >> > +extern __RTC_Failure > >> > + > >> > +SECTION .data > >> > + > >> > +; > >> > +; UINTN __security_cookie; > >> > +; > >> > +global ___security_cookie > >> > +___security_cookie: > >> > + DW 987974FAh > >> > + > >> > +SECTION .text > >> > + > >> > +; > >> > +; void __fastcall __security_check_cookie(UINTN > cookie) > >> > +; > >> > +; Note: __fastcall calling convention uses > ecx/edx to pass first two > >> parameters > >> > +; > >> > +global @__security_check_cookie@4 > >> > +@__security_check_cookie@4: > >> > + push ebp > >> > + mov ebp, esp > >> > + cmp ecx, [___security_cookie] > >> > + je .1 > >> > + push dword [ebp] ; pass return address > as the second parameter > >> > + push ecx ; cookie value in stack > is the first parameter > >> > + call ___report_gsfailure > >> > +.1: > >> > + mov esp, ebp > >> > + pop ebp > >> > + ret > >> > + > >> > +; > >> > +; void __declspec(naked) __cdecl > _RTC_CheckEsp(void) > >> > +; > >> > +global __RTC_CheckEsp > >> > +__RTC_CheckEsp: > >> > + push ebp > >> > + mov ebp, esp > >> > + je .1 > >> > + push dword [ebp] ; pass return address > to __RTC_Failure > >> > + call __RTC_Failure > >> > +.1: > >> > + mov esp, ebp > >> > + pop ebp > >> > + ret > >> > + > >> > diff --git > >a/MdePkg/Library/BaseStackCheckLib/X64/StackCheckStubAsm > .nasm > >> > > b/MdePkg/Library/BaseStackCheckLib/X64/StackCheckStubAsm. > nasm > >> > new file mode 100644 > >> > index 0000000000..1c8601f09c > >> > --- /dev/null > >> > +++ > >b/MdePkg/Library/BaseStackCheckLib/X64/StackCheckStubAsm > .nasm > >> > @@ -0,0 +1,54 @@ > >> > +;-------------------------------------------------- > ---------------------------- ; > >> > +; Copyright (c) 2018, Intel Corporation. All rights > reserved.<BR> > >> > +; This program and the accompanying materials > >> > +; are licensed and made available under the terms > and conditions of the > >BSD > >> > License > >> > +; which accompanies this distribution. The full > text of the license may be > >> found > >> > at > >> > +; http://opensource.org/licenses/bsd-license.php. > >> > +; > >> > +; THE PROGRAM IS DISTRIBUTED UNDER THE BSD LICENSE > ON AN "AS IS" > >> > BASIS, > >> > +; WITHOUT WARRANTIES OR REPRESENTATIONS OF ANY > KIND, EITHER > >> > EXPRESS OR IMPLIED. > >> > +; > >> > +; Module Name: > >> > +; > >> > +; StackCheckStubAsm.nasm > >> > +; > >> > +; Abstract: > >> > +; > >> > +; Stub globals and functions for compiler options > /GS, /RTCs > >> > +; > >> > +; Notes: > >> > +; > >> > +;-------------------------------------------------- > ---------------------------- > >> > + > >> > +; > >> > +; __declspec(noreturn) void __cdecl > __report_gsfailure(UINTN cookie, > >void > >> *ip); > >> > +; > >> > +extern __report_gsfailure > >> > + > >> > +DEFAULT REL > >> > + > >> > +SECTION .data > >> > + > >> > +; > >> > +; UINTN __security_cookie; > >> > +; > >> > +global __security_cookie > >> > +__security_cookie: > >> > + DQ 0CFE3FE6A3F5C5A88h > >> > + > >> > +SECTION .text > >> > + > >> > +; > >> > +; void __fastcall __security_check_cookie(UINTN > cookie) > >> > +; > >> > +; Note: __fastcall calling convention uses > ecx/edx to pass first two > >> parameters > >> > +; > >> > +global __security_check_cookie > >> > +__security_check_cookie: > >> > + cmp rcx, qword [__security_cookie] > >> > + je .1 > >> > + mov rdx, [esp] ; pass return address > as the second parameter > >> > + call __report_gsfailure > >> > +.1 > >> > + ret > >> > + > >> > -- > >> > 2.16.2.windows.1 > >> > > >> > _______________________________________________ > >> > edk2-devel mailing list > >> > [email protected] > >> > https://lists.01.org/mailman/listinfo/edk2-devel > >> _______________________________________________ > >> edk2-devel mailing list > >> [email protected] > >> https://lists.01.org/mailman/listinfo/edk2-devel _______________________________________________ edk2-devel mailing list [email protected] https://lists.01.org/mailman/listinfo/edk2-devel
