* [PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain support
@ 2018-10-16 0:55 Jian J Wang
2018-10-16 0:59 ` Wang, Jian J
0 siblings, 1 reply; 11+ messages in thread
From: Jian J Wang @ 2018-10-16 0:55 UTC (permalink / raw)
To: edk2-devel; +Cc: Michael D Kinney, Liming Gao, Jiewen Yao, Andrew Fish
This patch adds stack check support for MSFT toolchain, with
compiler option /GS and /RTCs. This functionality is similar
to the original ones supported by GCC toolchain.
Usage example:
This is a NULL library instance. Add it under a [LibraryClasses]
section in dsc file to let it be built into all modules employed
in a platform.
[LibraryClasses]
NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
Please note all not modules can be built against this library. Most
of them are SEC type of modules, such as
OvmfPkg/ResetVector/ResetVector.inf
In this case, this library should not be added to a common
[LibraryClasses] section but to specific ones, like
[LibraryClasses.common.PEI_CORE/PEIM/...].
In addition, /GS and/or /RTCs should be added to compiler command line.
This can be done by adding something like below under [BuildOptions]
section in dsc file.
[BuildOptions]
MSFT:DEBUG_*_*_CC_FLAGS = /GS /GL-
MSFT:DEBUG_*_*_CC_FLAGS = /RTCs /Od
Note: /GL- is required for /GS, and /Od is required for /RTCs.
Note: The flash layout might be needed to update to accommodate larger
image size due to /Od is enforced.
Pass tests:
a. Overwrite a local buffer variable (in a 32-bit and 64-bit driver)and
check if it's caught by new code (on both real platform and virtual
platform)
b. Boot Windows 10 and Ubuntu 18.04 on real platform with this
lib built-in
Cc: Michael D Kinney <michael.d.kinney@intel.com>
Cc: Liming Gao <liming.gao@intel.com>
Cc: Jiewen Yao <jiewen.yao@intel.com>
Cc: Andrew Fish <afish@apple.com>
Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
---
.../BaseStackCheckLib/BaseStackCheckLib.inf | 11 +-
.../Library/BaseStackCheckLib/BaseStackCheckMsft.c | 221 +++++++++++++++++++++
.../Library/BaseStackCheckLib/BaseStackCheckNull.c | 15 --
.../BaseStackCheckLib/Ia32/StackCheckStubAsm.nasm | 76 +++++++
.../BaseStackCheckLib/X64/StackCheckStubAsm.nasm | 54 +++++
5 files changed, 360 insertions(+), 17 deletions(-)
create mode 100644 MdePkg/Library/BaseStackCheckLib/BaseStackCheckMsft.c
delete mode 100644 MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c
create mode 100644 MdePkg/Library/BaseStackCheckLib/Ia32/StackCheckStubAsm.nasm
create mode 100644 MdePkg/Library/BaseStackCheckLib/X64/StackCheckStubAsm.nasm
diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
index e280651b11..1c9e6710c6 100644
--- a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
+++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
@@ -4,6 +4,7 @@
# Stack Check Library
#
# Copyright (c) 2014, ARM Ltd. All rights reserved.<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/StackCheckStubAsm.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
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain support
2018-10-16 0:55 [PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain support Jian J Wang
@ 2018-10-16 0:59 ` Wang, Jian J
2018-10-18 1:18 ` Wang, Jian J
0 siblings, 1 reply; 11+ messages in thread
From: Wang, Jian J @ 2018-10-16 0:59 UTC (permalink / raw)
To: edk2-devel, edk2-devel@lists.01.org
Cc: Kinney, Michael D, Yao, Jiewen, Andrew Fish, Gao, Liming
REF: https://bugzilla.tianocore.org/show_bug.cgi?id=1239
Regards,
Jian
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org]
> Sent: Tuesday, October 16, 2018 8:55 AM
> To: edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Andrew Fish <afish@apple.com>; Gao, Liming
> <liming.gao@intel.com>
> 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.inf
>
> Please note all not modules can be built against this library. Most
> of them are SEC type of modules, such as
>
> OvmfPkg/ResetVector/ResetVector.inf
>
> In this case, this library should not be added to a common
> [LibraryClasses] section but to specific ones, like
> [LibraryClasses.common.PEI_CORE/PEIM/...].
>
> In addition, /GS and/or /RTCs should be added to compiler command line.
> This can be done by adding something like below under [BuildOptions]
> section in dsc file.
>
> [BuildOptions]
> MSFT:DEBUG_*_*_CC_FLAGS = /GS /GL-
> MSFT:DEBUG_*_*_CC_FLAGS = /RTCs /Od
>
> Note: /GL- is required for /GS, and /Od is required for /RTCs.
> Note: The flash layout might be needed to update to accommodate larger
> image size due to /Od is enforced.
>
> Pass tests:
> a. Overwrite a local buffer variable (in a 32-bit and 64-bit driver)and
> check if it's caught by new code (on both real platform and virtual
> platform)
> b. Boot Windows 10 and Ubuntu 18.04 on real platform with this
> lib built-in
>
> Cc: Michael D Kinney <michael.d.kinney@intel.com>
> Cc: Liming Gao <liming.gao@intel.com>
> Cc: Jiewen Yao <jiewen.yao@intel.com>
> Cc: Andrew Fish <afish@apple.com>
> Contributed-under: TianoCore Contribution Agreement 1.1
> Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> ---
> .../BaseStackCheckLib/BaseStackCheckLib.inf | 11 +-
> .../Library/BaseStackCheckLib/BaseStackCheckMsft.c | 221
> +++++++++++++++++++++
> .../Library/BaseStackCheckLib/BaseStackCheckNull.c | 15 --
> .../BaseStackCheckLib/Ia32/StackCheckStubAsm.nasm | 76 +++++++
> .../BaseStackCheckLib/X64/StackCheckStubAsm.nasm | 54 +++++
> 5 files changed, 360 insertions(+), 17 deletions(-)
> create mode 100644
> MdePkg/Library/BaseStackCheckLib/BaseStackCheckMsft.c
> delete mode 100644 MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c
> create mode 100644
> MdePkg/Library/BaseStackCheckLib/Ia32/StackCheckStubAsm.nasm
> create mode 100644
> MdePkg/Library/BaseStackCheckLib/X64/StackCheckStubAsm.nasm
>
> diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> index e280651b11..1c9e6710c6 100644
> --- a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> @@ -4,6 +4,7 @@
> # Stack Check Library
> #
> # Copyright (c) 2014, ARM Ltd. All rights reserved.<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/StackCheckStubAsm.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
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain support
2018-10-16 0:59 ` Wang, Jian J
@ 2018-10-18 1:18 ` Wang, Jian J
2018-10-18 1:22 ` Gao, Liming
0 siblings, 1 reply; 11+ messages in thread
From: Wang, Jian J @ 2018-10-18 1:18 UTC (permalink / raw)
To: edk2-devel, edk2-devel@lists.01.org
Cc: Kinney, Michael D, Yao, Jiewen, Andrew Fish, Gao, Liming
Ping ... :)
Regards,
Jian
> -----Original Message-----
> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org]
> Sent: Tuesday, October 16, 2018 8:59 AM
> To: edk2-devel <edk2-devel-bounces@lists.01.org>; edk2-devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Yao, Jiewen
> <jiewen.yao@intel.com>; Andrew Fish <afish@apple.com>; Gao, Liming
> <liming.gao@intel.com>
> 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-bounces@lists.01.org]
> > Sent: Tuesday, October 16, 2018 8:55 AM
> > To: edk2-devel@lists.01.org
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Yao, Jiewen
> > <jiewen.yao@intel.com>; Andrew Fish <afish@apple.com>; Gao, Liming
> > <liming.gao@intel.com>
> > 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.inf
> >
> > Please note all not modules can be built against this library. Most
> > of them are SEC type of modules, such as
> >
> > OvmfPkg/ResetVector/ResetVector.inf
> >
> > In this case, this library should not be added to a common
> > [LibraryClasses] section but to specific ones, like
> > [LibraryClasses.common.PEI_CORE/PEIM/...].
> >
> > In addition, /GS and/or /RTCs should be added to compiler command line.
> > This can be done by adding something like below under [BuildOptions]
> > section in dsc file.
> >
> > [BuildOptions]
> > MSFT:DEBUG_*_*_CC_FLAGS = /GS /GL-
> > MSFT:DEBUG_*_*_CC_FLAGS = /RTCs /Od
> >
> > Note: /GL- is required for /GS, and /Od is required for /RTCs.
> > Note: The flash layout might be needed to update to accommodate larger
> > image size due to /Od is enforced.
> >
> > Pass tests:
> > a. Overwrite a local buffer variable (in a 32-bit and 64-bit driver)and
> > check if it's caught by new code (on both real platform and virtual
> > platform)
> > b. Boot Windows 10 and Ubuntu 18.04 on real platform with this
> > lib built-in
> >
> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
> > Cc: Liming Gao <liming.gao@intel.com>
> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > Cc: Andrew Fish <afish@apple.com>
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> > ---
> > .../BaseStackCheckLib/BaseStackCheckLib.inf | 11 +-
> > .../Library/BaseStackCheckLib/BaseStackCheckMsft.c | 221
> > +++++++++++++++++++++
> > .../Library/BaseStackCheckLib/BaseStackCheckNull.c | 15 --
> > .../BaseStackCheckLib/Ia32/StackCheckStubAsm.nasm | 76 +++++++
> > .../BaseStackCheckLib/X64/StackCheckStubAsm.nasm | 54 +++++
> > 5 files changed, 360 insertions(+), 17 deletions(-)
> > create mode 100644
> > MdePkg/Library/BaseStackCheckLib/BaseStackCheckMsft.c
> > delete mode 100644
> MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c
> > create mode 100644
> > MdePkg/Library/BaseStackCheckLib/Ia32/StackCheckStubAsm.nasm
> > create mode 100644
> > MdePkg/Library/BaseStackCheckLib/X64/StackCheckStubAsm.nasm
> >
> > diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> > b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> > index e280651b11..1c9e6710c6 100644
> > --- a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
> > @@ -4,6 +4,7 @@
> > # Stack Check Library
> > #
> > # Copyright (c) 2014, ARM Ltd. All rights reserved.<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/StackCheckStubAsm.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
> > edk2-devel@lists.01.org
> > https://lists.01.org/mailman/listinfo/edk2-devel
> _______________________________________________
> edk2-devel mailing list
> edk2-devel@lists.01.org
> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain support
2018-10-18 1:18 ` Wang, Jian J
@ 2018-10-18 1:22 ` Gao, Liming
2018-10-18 1:36 ` Kinney, Michael D
0 siblings, 1 reply; 11+ messages in thread
From: Gao, Liming @ 2018-10-18 1:22 UTC (permalink / raw)
To: Wang, Jian J, edk2-devel, edk2-devel@lists.01.org
Cc: Kinney, Michael D, Yao, Jiewen, Andrew Fish
Jian:
The patch is good to me. Reviewed-by: Liming Gao <liming.gao@intel.com>
Thanks
Liming
>-----Original Message-----
>From: Wang, Jian J
>Sent: Thursday, October 18, 2018 9:19 AM
>To: edk2-devel <edk2-devel-bounces@lists.01.org>; edk2-devel@lists.01.org
>Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Yao, Jiewen
><jiewen.yao@intel.com>; Andrew Fish <afish@apple.com>; Gao, Liming
><liming.gao@intel.com>
>Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain
>support
>
>Ping ... :)
>
>Regards,
>Jian
>
>
>> -----Original Message-----
>> From: edk2-devel [mailto:edk2-devel-bounces@lists.01.org]
>> Sent: Tuesday, October 16, 2018 8:59 AM
>> To: edk2-devel <edk2-devel-bounces@lists.01.org>; edk2-
>devel@lists.01.org
>> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Yao, Jiewen
>> <jiewen.yao@intel.com>; Andrew Fish <afish@apple.com>; Gao, Liming
>> <liming.gao@intel.com>
>> 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-bounces@lists.01.org]
>> > Sent: Tuesday, October 16, 2018 8:55 AM
>> > To: edk2-devel@lists.01.org
>> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Yao, Jiewen
>> > <jiewen.yao@intel.com>; Andrew Fish <afish@apple.com>; Gao, Liming
>> > <liming.gao@intel.com>
>> > 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.inf
>> >
>> > Please note all not modules can be built against this library. Most
>> > of them are SEC type of modules, such as
>> >
>> > OvmfPkg/ResetVector/ResetVector.inf
>> >
>> > In this case, this library should not be added to a common
>> > [LibraryClasses] section but to specific ones, like
>> > [LibraryClasses.common.PEI_CORE/PEIM/...].
>> >
>> > In addition, /GS and/or /RTCs should be added to compiler command line.
>> > This can be done by adding something like below under [BuildOptions]
>> > section in dsc file.
>> >
>> > [BuildOptions]
>> > MSFT:DEBUG_*_*_CC_FLAGS = /GS /GL-
>> > MSFT:DEBUG_*_*_CC_FLAGS = /RTCs /Od
>> >
>> > Note: /GL- is required for /GS, and /Od is required for /RTCs.
>> > Note: The flash layout might be needed to update to accommodate larger
>> > image size due to /Od is enforced.
>> >
>> > Pass tests:
>> > a. Overwrite a local buffer variable (in a 32-bit and 64-bit driver)and
>> > check if it's caught by new code (on both real platform and virtual
>> > platform)
>> > b. Boot Windows 10 and Ubuntu 18.04 on real platform with this
>> > lib built-in
>> >
>> > Cc: Michael D Kinney <michael.d.kinney@intel.com>
>> > Cc: Liming Gao <liming.gao@intel.com>
>> > Cc: Jiewen Yao <jiewen.yao@intel.com>
>> > Cc: Andrew Fish <afish@apple.com>
>> > Contributed-under: TianoCore Contribution Agreement 1.1
>> > Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
>> > ---
>> > .../BaseStackCheckLib/BaseStackCheckLib.inf | 11 +-
>> > .../Library/BaseStackCheckLib/BaseStackCheckMsft.c | 221
>> > +++++++++++++++++++++
>> > .../Library/BaseStackCheckLib/BaseStackCheckNull.c | 15 --
>> > .../BaseStackCheckLib/Ia32/StackCheckStubAsm.nasm | 76 +++++++
>> > .../BaseStackCheckLib/X64/StackCheckStubAsm.nasm | 54 +++++
>> > 5 files changed, 360 insertions(+), 17 deletions(-)
>> > create mode 100644
>> > MdePkg/Library/BaseStackCheckLib/BaseStackCheckMsft.c
>> > delete mode 100644
>> MdePkg/Library/BaseStackCheckLib/BaseStackCheckNull.c
>> > create mode 100644
>> > MdePkg/Library/BaseStackCheckLib/Ia32/StackCheckStubAsm.nasm
>> > create mode 100644
>> > MdePkg/Library/BaseStackCheckLib/X64/StackCheckStubAsm.nasm
>> >
>> > diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
>> > b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
>> > index e280651b11..1c9e6710c6 100644
>> > --- a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
>> > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf
>> > @@ -4,6 +4,7 @@
>> > # Stack Check Library
>> > #
>> > # Copyright (c) 2014, ARM Ltd. All rights reserved.<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/StackCheckStubAsm.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
>> > edk2-devel@lists.01.org
>> > https://lists.01.org/mailman/listinfo/edk2-devel
>> _______________________________________________
>> edk2-devel mailing list
>> edk2-devel@lists.01.org
>> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain support
2018-10-18 1:22 ` Gao, Liming
@ 2018-10-18 1:36 ` Kinney, Michael D
2018-10-18 2:15 ` Wang, Jian J
2018-10-18 6:35 ` Wang, Jian J
0 siblings, 2 replies; 11+ messages in thread
From: Kinney, Michael D @ 2018-10-18 1:36 UTC (permalink / raw)
To: Gao, Liming, Wang, Jian J, edk2-devel, edk2-devel@lists.01.org,
Kinney, Michael D
Cc: Yao, Jiewen, Andrew Fish
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 <jian.j.wang@intel.com>; edk2-devel
> <edk2-devel-bounces@lists.01.org>; edk2-
> devel@lists.01.org
> Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Yao,
> Jiewen <jiewen.yao@intel.com>; Andrew Fish
> <afish@apple.com>
> Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib: add
> MSFT toolchain support
>
> Jian:
> The patch is good to me. Reviewed-by: Liming Gao
> <liming.gao@intel.com>
>
> Thanks
> Liming
> >-----Original Message-----
> >From: Wang, Jian J
> >Sent: Thursday, October 18, 2018 9:19 AM
> >To: edk2-devel <edk2-devel-bounces@lists.01.org>; edk2-
> devel@lists.01.org
> >Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Yao,
> Jiewen
> ><jiewen.yao@intel.com>; Andrew Fish <afish@apple.com>;
> Gao, Liming
> ><liming.gao@intel.com>
> >Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib:
> add MSFT toolchain
> >support
> >
> >Ping ... :)
> >
> >Regards,
> >Jian
> >
> >
> >> -----Original Message-----
> >> From: edk2-devel [mailto:edk2-devel-
> bounces@lists.01.org]
> >> Sent: Tuesday, October 16, 2018 8:59 AM
> >> To: edk2-devel <edk2-devel-bounces@lists.01.org>;
> edk2-
> >devel@lists.01.org
> >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> Yao, Jiewen
> >> <jiewen.yao@intel.com>; Andrew Fish <afish@apple.com>;
> Gao, Liming
> >> <liming.gao@intel.com>
> >> 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-
> bounces@lists.01.org]
> >> > Sent: Tuesday, October 16, 2018 8:55 AM
> >> > To: edk2-devel@lists.01.org
> >> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> Yao, Jiewen
> >> > <jiewen.yao@intel.com>; Andrew Fish
> <afish@apple.com>; Gao, Liming
> >> > <liming.gao@intel.com>
> >> > 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 <michael.d.kinney@intel.com>
> >> > Cc: Liming Gao <liming.gao@intel.com>
> >> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> >> > Cc: Andrew Fish <afish@apple.com>
> >> > Contributed-under: TianoCore Contribution Agreement
> 1.1
> >> > Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> >> > ---
> >> > .../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
> >> > edk2-devel@lists.01.org
> >> > https://lists.01.org/mailman/listinfo/edk2-devel
> >> _______________________________________________
> >> edk2-devel mailing list
> >> edk2-devel@lists.01.org
> >> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain support
2018-10-18 1:36 ` Kinney, Michael D
@ 2018-10-18 2:15 ` Wang, Jian J
2018-10-18 23:16 ` Kinney, Michael D
2018-10-18 6:35 ` Wang, Jian J
1 sibling, 1 reply; 11+ messages in thread
From: Wang, Jian J @ 2018-10-18 2:15 UTC (permalink / raw)
To: Kinney, Michael D, Gao, Liming, edk2-devel,
edk2-devel@lists.01.org
Cc: Yao, Jiewen, Andrew Fish
Mike,
All those functions are declared in source files in Visual Studio except to
__GSHandlerCheck(). But I can't find there's online link or document to those files
and APIs. Is it ok to just list the file names?
VC\INCLUDE\vcruntime.h:
__security_check_cookie
__report_gsfailure
VC\INCLUDE\rtcapi.h:
_RTC_CheckEsp
_RTC_CheckStackVars
_RTC_Shutdown
_RTC_InitBase
VC\crt\src\vcruntime\gs_report.c
__report_rangecheckfailure
I built the lib with all supported VS toolchains and all passed, but only tested
with VS2015. I think I need to do more tests. Thanks for pointing it out.
Regards,
Jian
> -----Original Message-----
> From: Kinney, Michael D
> Sent: Thursday, October 18, 2018 9:36 AM
> To: Gao, Liming <liming.gao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> edk2-devel <edk2-devel-bounces@lists.01.org>; edk2-devel@lists.01.org;
> Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Andrew Fish <afish@apple.com>
> Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain
> support
>
> 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 <jian.j.wang@intel.com>; edk2-devel
> > <edk2-devel-bounces@lists.01.org>; edk2-
> > devel@lists.01.org
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Yao,
> > Jiewen <jiewen.yao@intel.com>; Andrew Fish
> > <afish@apple.com>
> > Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib: add
> > MSFT toolchain support
> >
> > Jian:
> > The patch is good to me. Reviewed-by: Liming Gao
> > <liming.gao@intel.com>
> >
> > Thanks
> > Liming
> > >-----Original Message-----
> > >From: Wang, Jian J
> > >Sent: Thursday, October 18, 2018 9:19 AM
> > >To: edk2-devel <edk2-devel-bounces@lists.01.org>; edk2-
> > devel@lists.01.org
> > >Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Yao,
> > Jiewen
> > ><jiewen.yao@intel.com>; Andrew Fish <afish@apple.com>;
> > Gao, Liming
> > ><liming.gao@intel.com>
> > >Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib:
> > add MSFT toolchain
> > >support
> > >
> > >Ping ... :)
> > >
> > >Regards,
> > >Jian
> > >
> > >
> > >> -----Original Message-----
> > >> From: edk2-devel [mailto:edk2-devel-
> > bounces@lists.01.org]
> > >> Sent: Tuesday, October 16, 2018 8:59 AM
> > >> To: edk2-devel <edk2-devel-bounces@lists.01.org>;
> > edk2-
> > >devel@lists.01.org
> > >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> > Yao, Jiewen
> > >> <jiewen.yao@intel.com>; Andrew Fish <afish@apple.com>;
> > Gao, Liming
> > >> <liming.gao@intel.com>
> > >> 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-
> > bounces@lists.01.org]
> > >> > Sent: Tuesday, October 16, 2018 8:55 AM
> > >> > To: edk2-devel@lists.01.org
> > >> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> > Yao, Jiewen
> > >> > <jiewen.yao@intel.com>; Andrew Fish
> > <afish@apple.com>; Gao, Liming
> > >> > <liming.gao@intel.com>
> > >> > 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 <michael.d.kinney@intel.com>
> > >> > Cc: Liming Gao <liming.gao@intel.com>
> > >> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > >> > Cc: Andrew Fish <afish@apple.com>
> > >> > Contributed-under: TianoCore Contribution Agreement
> > 1.1
> > >> > Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> > >> > ---
> > >> > .../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
> > >> > edk2-devel@lists.01.org
> > >> > https://lists.01.org/mailman/listinfo/edk2-devel
> > >> _______________________________________________
> > >> edk2-devel mailing list
> > >> edk2-devel@lists.01.org
> > >> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain support
2018-10-18 1:36 ` Kinney, Michael D
2018-10-18 2:15 ` Wang, Jian J
@ 2018-10-18 6:35 ` Wang, Jian J
2018-10-19 0:05 ` Kinney, Michael D
1 sibling, 1 reply; 11+ messages in thread
From: Wang, Jian J @ 2018-10-18 6:35 UTC (permalink / raw)
To: Kinney, Michael D, Gao, Liming, edk2-devel,
edk2-devel@lists.01.org
Cc: Yao, Jiewen, Andrew Fish
Mike,
I tested the lib with all supported MSFT toolchains. No difference found so far.
Regards,
Jian
> -----Original Message-----
> From: Kinney, Michael D
> Sent: Thursday, October 18, 2018 9:36 AM
> To: Gao, Liming <liming.gao@intel.com>; Wang, Jian J <jian.j.wang@intel.com>;
> edk2-devel <edk2-devel-bounces@lists.01.org>; edk2-devel@lists.01.org;
> Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Andrew Fish <afish@apple.com>
> Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain
> support
>
> 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 <jian.j.wang@intel.com>; edk2-devel
> > <edk2-devel-bounces@lists.01.org>; edk2-
> > devel@lists.01.org
> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Yao,
> > Jiewen <jiewen.yao@intel.com>; Andrew Fish
> > <afish@apple.com>
> > Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib: add
> > MSFT toolchain support
> >
> > Jian:
> > The patch is good to me. Reviewed-by: Liming Gao
> > <liming.gao@intel.com>
> >
> > Thanks
> > Liming
> > >-----Original Message-----
> > >From: Wang, Jian J
> > >Sent: Thursday, October 18, 2018 9:19 AM
> > >To: edk2-devel <edk2-devel-bounces@lists.01.org>; edk2-
> > devel@lists.01.org
> > >Cc: Kinney, Michael D <michael.d.kinney@intel.com>; Yao,
> > Jiewen
> > ><jiewen.yao@intel.com>; Andrew Fish <afish@apple.com>;
> > Gao, Liming
> > ><liming.gao@intel.com>
> > >Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib:
> > add MSFT toolchain
> > >support
> > >
> > >Ping ... :)
> > >
> > >Regards,
> > >Jian
> > >
> > >
> > >> -----Original Message-----
> > >> From: edk2-devel [mailto:edk2-devel-
> > bounces@lists.01.org]
> > >> Sent: Tuesday, October 16, 2018 8:59 AM
> > >> To: edk2-devel <edk2-devel-bounces@lists.01.org>;
> > edk2-
> > >devel@lists.01.org
> > >> Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> > Yao, Jiewen
> > >> <jiewen.yao@intel.com>; Andrew Fish <afish@apple.com>;
> > Gao, Liming
> > >> <liming.gao@intel.com>
> > >> 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-
> > bounces@lists.01.org]
> > >> > Sent: Tuesday, October 16, 2018 8:55 AM
> > >> > To: edk2-devel@lists.01.org
> > >> > Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> > Yao, Jiewen
> > >> > <jiewen.yao@intel.com>; Andrew Fish
> > <afish@apple.com>; Gao, Liming
> > >> > <liming.gao@intel.com>
> > >> > 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 <michael.d.kinney@intel.com>
> > >> > Cc: Liming Gao <liming.gao@intel.com>
> > >> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > >> > Cc: Andrew Fish <afish@apple.com>
> > >> > Contributed-under: TianoCore Contribution Agreement
> > 1.1
> > >> > Signed-off-by: Jian J Wang <jian.j.wang@intel.com>
> > >> > ---
> > >> > .../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
> > >> > edk2-devel@lists.01.org
> > >> > https://lists.01.org/mailman/listinfo/edk2-devel
> > >> _______________________________________________
> > >> edk2-devel mailing list
> > >> edk2-devel@lists.01.org
> > >> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain support
2018-10-18 2:15 ` Wang, Jian J
@ 2018-10-18 23:16 ` Kinney, Michael D
2018-10-19 1:02 ` Wang, Jian J
2018-10-30 2:53 ` Wang, Jian J
0 siblings, 2 replies; 11+ messages in thread
From: Kinney, Michael D @ 2018-10-18 23:16 UTC (permalink / raw)
To: Wang, Jian J, Gao, Liming, edk2-devel@lists.01.org,
Kinney, Michael D
Cc: Yao, Jiewen, Andrew Fish
Jian,
I do not see a description of the parameters, return
values or required behavior for those APIs in the header
files. How do you know if they are implemented correctly?
How would we write tests for these APIs?
Mike
> -----Original Message-----
> From: Wang, Jian J
> Sent: Wednesday, October 17, 2018 7:16 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; Gao,
> Liming <liming.gao@intel.com>; edk2-devel <edk2-devel-
> bounces@lists.01.org>; edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Andrew Fish
> <afish@apple.com>
> Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib: add
> MSFT toolchain support
>
> Mike,
>
> All those functions are declared in source files in
> Visual Studio except to
> __GSHandlerCheck(). But I can't find there's online link
> or document to those files
> and APIs. Is it ok to just list the file names?
>
> VC\INCLUDE\vcruntime.h:
> __security_check_cookie
> __report_gsfailure
>
> VC\INCLUDE\rtcapi.h:
> _RTC_CheckEsp
> _RTC_CheckStackVars
> _RTC_Shutdown
> _RTC_InitBase
>
> VC\crt\src\vcruntime\gs_report.c
> __report_rangecheckfailure
>
> I built the lib with all supported VS toolchains and all
> passed, but only tested
> with VS2015. I think I need to do more tests. Thanks for
> pointing it out.
>
> Regards,
> Jian
>
>
> > -----Original Message-----
> > From: Kinney, Michael D
> > Sent: Thursday, October 18, 2018 9:36 AM
> > To: Gao, Liming <liming.gao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>;
> > edk2-devel <edk2-devel-bounces@lists.01.org>; edk2-
> devel@lists.01.org;
> > Kinney, Michael D <michael.d.kinney@intel.com>
> > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Andrew Fish
> <afish@apple.com>
> > Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib:
> add MSFT toolchain
> > support
> >
> > 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 <jian.j.wang@intel.com>; edk2-devel
> > > <edk2-devel-bounces@lists.01.org>; edk2-
> > > devel@lists.01.org
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> Yao,
> > > Jiewen <jiewen.yao@intel.com>; Andrew Fish
> > > <afish@apple.com>
> > > Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib:
> add
> > > MSFT toolchain support
> > >
> > > Jian:
> > > The patch is good to me. Reviewed-by: Liming Gao
> > > <liming.gao@intel.com>
> > >
> > > Thanks
> > > Liming
> > > >-----Original Message-----
> > > >From: Wang, Jian J
> > > >Sent: Thursday, October 18, 2018 9:19 AM
> > > >To: edk2-devel <edk2-devel-bounces@lists.01.org>;
> edk2-
> > > devel@lists.01.org
> > > >Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> Yao,
> > > Jiewen
> > > ><jiewen.yao@intel.com>; Andrew Fish
> <afish@apple.com>;
> > > Gao, Liming
> > > ><liming.gao@intel.com>
> > > >Subject: RE: [edk2] [PATCH]
> MdePkg/BaseStackCheckLib:
> > > add MSFT toolchain
> > > >support
> > > >
> > > >Ping ... :)
> > > >
> > > >Regards,
> > > >Jian
> > > >
> > > >
> > > >> -----Original Message-----
> > > >> From: edk2-devel [mailto:edk2-devel-
> > > bounces@lists.01.org]
> > > >> Sent: Tuesday, October 16, 2018 8:59 AM
> > > >> To: edk2-devel <edk2-devel-bounces@lists.01.org>;
> > > edk2-
> > > >devel@lists.01.org
> > > >> Cc: Kinney, Michael D
> <michael.d.kinney@intel.com>;
> > > Yao, Jiewen
> > > >> <jiewen.yao@intel.com>; Andrew Fish
> <afish@apple.com>;
> > > Gao, Liming
> > > >> <liming.gao@intel.com>
> > > >> 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-
> > > bounces@lists.01.org]
> > > >> > Sent: Tuesday, October 16, 2018 8:55 AM
> > > >> > To: edk2-devel@lists.01.org
> > > >> > Cc: Kinney, Michael D
> <michael.d.kinney@intel.com>;
> > > Yao, Jiewen
> > > >> > <jiewen.yao@intel.com>; Andrew Fish
> > > <afish@apple.com>; Gao, Liming
> > > >> > <liming.gao@intel.com>
> > > >> > 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
> <michael.d.kinney@intel.com>
> > > >> > Cc: Liming Gao <liming.gao@intel.com>
> > > >> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > >> > Cc: Andrew Fish <afish@apple.com>
> > > >> > Contributed-under: TianoCore Contribution
> Agreement
> > > 1.1
> > > >> > Signed-off-by: Jian J Wang
> <jian.j.wang@intel.com>
> > > >> > ---
> > > >> > .../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
> > > >> > edk2-devel@lists.01.org
> > > >> > https://lists.01.org/mailman/listinfo/edk2-devel
> > > >> _______________________________________________
> > > >> edk2-devel mailing list
> > > >> edk2-devel@lists.01.org
> > > >> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain support
2018-10-18 6:35 ` Wang, Jian J
@ 2018-10-19 0:05 ` Kinney, Michael D
0 siblings, 0 replies; 11+ messages in thread
From: Kinney, Michael D @ 2018-10-19 0:05 UTC (permalink / raw)
To: Wang, Jian J, Gao, Liming, edk2-devel@lists.01.org,
Kinney, Michael D
Cc: Yao, Jiewen, Andrew Fish
Jian,
Please add the list of VSxxx tool chain tags you
tested with in the commit message.
Mike
> -----Original Message-----
> From: Wang, Jian J
> Sent: Wednesday, October 17, 2018 11:36 PM
> To: Kinney, Michael D <michael.d.kinney@intel.com>; Gao,
> Liming <liming.gao@intel.com>; edk2-devel <edk2-devel-
> bounces@lists.01.org>; edk2-devel@lists.01.org
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Andrew Fish
> <afish@apple.com>
> Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib: add
> MSFT toolchain support
>
> Mike,
>
> I tested the lib with all supported MSFT toolchains. No
> difference found so far.
>
> Regards,
> Jian
>
>
> > -----Original Message-----
> > From: Kinney, Michael D
> > Sent: Thursday, October 18, 2018 9:36 AM
> > To: Gao, Liming <liming.gao@intel.com>; Wang, Jian J
> <jian.j.wang@intel.com>;
> > edk2-devel <edk2-devel-bounces@lists.01.org>; edk2-
> devel@lists.01.org;
> > Kinney, Michael D <michael.d.kinney@intel.com>
> > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Andrew Fish
> <afish@apple.com>
> > Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib:
> add MSFT toolchain
> > support
> >
> > 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 <jian.j.wang@intel.com>; edk2-devel
> > > <edk2-devel-bounces@lists.01.org>; edk2-
> > > devel@lists.01.org
> > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> Yao,
> > > Jiewen <jiewen.yao@intel.com>; Andrew Fish
> > > <afish@apple.com>
> > > Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib:
> add
> > > MSFT toolchain support
> > >
> > > Jian:
> > > The patch is good to me. Reviewed-by: Liming Gao
> > > <liming.gao@intel.com>
> > >
> > > Thanks
> > > Liming
> > > >-----Original Message-----
> > > >From: Wang, Jian J
> > > >Sent: Thursday, October 18, 2018 9:19 AM
> > > >To: edk2-devel <edk2-devel-bounces@lists.01.org>;
> edk2-
> > > devel@lists.01.org
> > > >Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> Yao,
> > > Jiewen
> > > ><jiewen.yao@intel.com>; Andrew Fish
> <afish@apple.com>;
> > > Gao, Liming
> > > ><liming.gao@intel.com>
> > > >Subject: RE: [edk2] [PATCH]
> MdePkg/BaseStackCheckLib:
> > > add MSFT toolchain
> > > >support
> > > >
> > > >Ping ... :)
> > > >
> > > >Regards,
> > > >Jian
> > > >
> > > >
> > > >> -----Original Message-----
> > > >> From: edk2-devel [mailto:edk2-devel-
> > > bounces@lists.01.org]
> > > >> Sent: Tuesday, October 16, 2018 8:59 AM
> > > >> To: edk2-devel <edk2-devel-bounces@lists.01.org>;
> > > edk2-
> > > >devel@lists.01.org
> > > >> Cc: Kinney, Michael D
> <michael.d.kinney@intel.com>;
> > > Yao, Jiewen
> > > >> <jiewen.yao@intel.com>; Andrew Fish
> <afish@apple.com>;
> > > Gao, Liming
> > > >> <liming.gao@intel.com>
> > > >> 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-
> > > bounces@lists.01.org]
> > > >> > Sent: Tuesday, October 16, 2018 8:55 AM
> > > >> > To: edk2-devel@lists.01.org
> > > >> > Cc: Kinney, Michael D
> <michael.d.kinney@intel.com>;
> > > Yao, Jiewen
> > > >> > <jiewen.yao@intel.com>; Andrew Fish
> > > <afish@apple.com>; Gao, Liming
> > > >> > <liming.gao@intel.com>
> > > >> > 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
> <michael.d.kinney@intel.com>
> > > >> > Cc: Liming Gao <liming.gao@intel.com>
> > > >> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > >> > Cc: Andrew Fish <afish@apple.com>
> > > >> > Contributed-under: TianoCore Contribution
> Agreement
> > > 1.1
> > > >> > Signed-off-by: Jian J Wang
> <jian.j.wang@intel.com>
> > > >> > ---
> > > >> > .../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
> > > >> > edk2-devel@lists.01.org
> > > >> > https://lists.01.org/mailman/listinfo/edk2-devel
> > > >> _______________________________________________
> > > >> edk2-devel mailing list
> > > >> edk2-devel@lists.01.org
> > > >> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain support
2018-10-18 23:16 ` Kinney, Michael D
@ 2018-10-19 1:02 ` Wang, Jian J
2018-10-30 2:53 ` Wang, Jian J
1 sibling, 0 replies; 11+ messages in thread
From: Wang, Jian J @ 2018-10-19 1:02 UTC (permalink / raw)
To: Kinney, Michael D, Gao, Liming, edk2-devel@lists.01.org
Cc: Yao, Jiewen, Andrew Fish
Mike,
The code originate from Jiewen's work done years before. I think he validated
related implementation. In addition, I also used following ways to verify those APIs:
a. Use debugger to verify the parameters passed in is as expected.
b. Disassemble the efi image to see if the calling convention is correct
c. Update existing drivers (one for PEI and one for DXE) to overwrite local variable
to see if it can catch the stack corruption.
d. Write a driver which has local buffer overflow issue (write array variable cross boundary)
and load it in shell
I validated above with /GS, /RTCs and /GS + /RTCs separately.
I think as long as we make sure the parameters are correct, we can decide how to
implement them. Those APIs in VC's lib are meant for OS. We can't follow them
with no changes for UEFI environment, right?
Regards,
Jian
> -----Original Message-----
> From: Kinney, Michael D
> Sent: Friday, October 19, 2018 7:17 AM
> To: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <liming.gao@intel.com>;
> edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Andrew Fish <afish@apple.com>
> Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain
> support
>
> Jian,
>
> I do not see a description of the parameters, return
> values or required behavior for those APIs in the header
> files. How do you know if they are implemented correctly?
>
> How would we write tests for these APIs?
>
> Mike
>
> > -----Original Message-----
> > From: Wang, Jian J
> > Sent: Wednesday, October 17, 2018 7:16 PM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>; Gao,
> > Liming <liming.gao@intel.com>; edk2-devel <edk2-devel-
> > bounces@lists.01.org>; edk2-devel@lists.01.org
> > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Andrew Fish
> > <afish@apple.com>
> > Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib: add
> > MSFT toolchain support
> >
> > Mike,
> >
> > All those functions are declared in source files in
> > Visual Studio except to
> > __GSHandlerCheck(). But I can't find there's online link
> > or document to those files
> > and APIs. Is it ok to just list the file names?
> >
> > VC\INCLUDE\vcruntime.h:
> > __security_check_cookie
> > __report_gsfailure
> >
> > VC\INCLUDE\rtcapi.h:
> > _RTC_CheckEsp
> > _RTC_CheckStackVars
> > _RTC_Shutdown
> > _RTC_InitBase
> >
> > VC\crt\src\vcruntime\gs_report.c
> > __report_rangecheckfailure
> >
> > I built the lib with all supported VS toolchains and all
> > passed, but only tested
> > with VS2015. I think I need to do more tests. Thanks for
> > pointing it out.
> >
> > Regards,
> > Jian
> >
> >
> > > -----Original Message-----
> > > From: Kinney, Michael D
> > > Sent: Thursday, October 18, 2018 9:36 AM
> > > To: Gao, Liming <liming.gao@intel.com>; Wang, Jian J
> > <jian.j.wang@intel.com>;
> > > edk2-devel <edk2-devel-bounces@lists.01.org>; edk2-
> > devel@lists.01.org;
> > > Kinney, Michael D <michael.d.kinney@intel.com>
> > > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Andrew Fish
> > <afish@apple.com>
> > > Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib:
> > add MSFT toolchain
> > > support
> > >
> > > 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 <jian.j.wang@intel.com>; edk2-devel
> > > > <edk2-devel-bounces@lists.01.org>; edk2-
> > > > devel@lists.01.org
> > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> > Yao,
> > > > Jiewen <jiewen.yao@intel.com>; Andrew Fish
> > > > <afish@apple.com>
> > > > Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib:
> > add
> > > > MSFT toolchain support
> > > >
> > > > Jian:
> > > > The patch is good to me. Reviewed-by: Liming Gao
> > > > <liming.gao@intel.com>
> > > >
> > > > Thanks
> > > > Liming
> > > > >-----Original Message-----
> > > > >From: Wang, Jian J
> > > > >Sent: Thursday, October 18, 2018 9:19 AM
> > > > >To: edk2-devel <edk2-devel-bounces@lists.01.org>;
> > edk2-
> > > > devel@lists.01.org
> > > > >Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> > Yao,
> > > > Jiewen
> > > > ><jiewen.yao@intel.com>; Andrew Fish
> > <afish@apple.com>;
> > > > Gao, Liming
> > > > ><liming.gao@intel.com>
> > > > >Subject: RE: [edk2] [PATCH]
> > MdePkg/BaseStackCheckLib:
> > > > add MSFT toolchain
> > > > >support
> > > > >
> > > > >Ping ... :)
> > > > >
> > > > >Regards,
> > > > >Jian
> > > > >
> > > > >
> > > > >> -----Original Message-----
> > > > >> From: edk2-devel [mailto:edk2-devel-
> > > > bounces@lists.01.org]
> > > > >> Sent: Tuesday, October 16, 2018 8:59 AM
> > > > >> To: edk2-devel <edk2-devel-bounces@lists.01.org>;
> > > > edk2-
> > > > >devel@lists.01.org
> > > > >> Cc: Kinney, Michael D
> > <michael.d.kinney@intel.com>;
> > > > Yao, Jiewen
> > > > >> <jiewen.yao@intel.com>; Andrew Fish
> > <afish@apple.com>;
> > > > Gao, Liming
> > > > >> <liming.gao@intel.com>
> > > > >> 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-
> > > > bounces@lists.01.org]
> > > > >> > Sent: Tuesday, October 16, 2018 8:55 AM
> > > > >> > To: edk2-devel@lists.01.org
> > > > >> > Cc: Kinney, Michael D
> > <michael.d.kinney@intel.com>;
> > > > Yao, Jiewen
> > > > >> > <jiewen.yao@intel.com>; Andrew Fish
> > > > <afish@apple.com>; Gao, Liming
> > > > >> > <liming.gao@intel.com>
> > > > >> > 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
> > <michael.d.kinney@intel.com>
> > > > >> > Cc: Liming Gao <liming.gao@intel.com>
> > > > >> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > > >> > Cc: Andrew Fish <afish@apple.com>
> > > > >> > Contributed-under: TianoCore Contribution
> > Agreement
> > > > 1.1
> > > > >> > Signed-off-by: Jian J Wang
> > <jian.j.wang@intel.com>
> > > > >> > ---
> > > > >> > .../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
> > > > >> > edk2-devel@lists.01.org
> > > > >> > https://lists.01.org/mailman/listinfo/edk2-devel
> > > > >> _______________________________________________
> > > > >> edk2-devel mailing list
> > > > >> edk2-devel@lists.01.org
> > > > >> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain support
2018-10-18 23:16 ` Kinney, Michael D
2018-10-19 1:02 ` Wang, Jian J
@ 2018-10-30 2:53 ` Wang, Jian J
1 sibling, 0 replies; 11+ messages in thread
From: Wang, Jian J @ 2018-10-30 2:53 UTC (permalink / raw)
To: Kinney, Michael D, Gao, Liming, edk2-devel@lists.01.org
Cc: Yao, Jiewen, Andrew Fish, Gao, Liming
Mike,
Since we're pending on the documentation of those APIs and it seems it won't
be solved soon, I'd suggest to drop this feature from the coming release event.
https://github.com/tianocore/tianocore.github.io/wiki/EDK-II-Release-Planning
Regards,
Jian
> -----Original Message-----
> From: Kinney, Michael D
> Sent: Friday, October 19, 2018 7:17 AM
> To: Wang, Jian J <jian.j.wang@intel.com>; Gao, Liming <liming.gao@intel.com>;
> edk2-devel@lists.01.org; Kinney, Michael D <michael.d.kinney@intel.com>
> Cc: Yao, Jiewen <jiewen.yao@intel.com>; Andrew Fish <afish@apple.com>
> Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain
> support
>
> Jian,
>
> I do not see a description of the parameters, return
> values or required behavior for those APIs in the header
> files. How do you know if they are implemented correctly?
>
> How would we write tests for these APIs?
>
> Mike
>
> > -----Original Message-----
> > From: Wang, Jian J
> > Sent: Wednesday, October 17, 2018 7:16 PM
> > To: Kinney, Michael D <michael.d.kinney@intel.com>; Gao,
> > Liming <liming.gao@intel.com>; edk2-devel <edk2-devel-
> > bounces@lists.01.org>; edk2-devel@lists.01.org
> > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Andrew Fish
> > <afish@apple.com>
> > Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib: add
> > MSFT toolchain support
> >
> > Mike,
> >
> > All those functions are declared in source files in
> > Visual Studio except to
> > __GSHandlerCheck(). But I can't find there's online link
> > or document to those files
> > and APIs. Is it ok to just list the file names?
> >
> > VC\INCLUDE\vcruntime.h:
> > __security_check_cookie
> > __report_gsfailure
> >
> > VC\INCLUDE\rtcapi.h:
> > _RTC_CheckEsp
> > _RTC_CheckStackVars
> > _RTC_Shutdown
> > _RTC_InitBase
> >
> > VC\crt\src\vcruntime\gs_report.c
> > __report_rangecheckfailure
> >
> > I built the lib with all supported VS toolchains and all
> > passed, but only tested
> > with VS2015. I think I need to do more tests. Thanks for
> > pointing it out.
> >
> > Regards,
> > Jian
> >
> >
> > > -----Original Message-----
> > > From: Kinney, Michael D
> > > Sent: Thursday, October 18, 2018 9:36 AM
> > > To: Gao, Liming <liming.gao@intel.com>; Wang, Jian J
> > <jian.j.wang@intel.com>;
> > > edk2-devel <edk2-devel-bounces@lists.01.org>; edk2-
> > devel@lists.01.org;
> > > Kinney, Michael D <michael.d.kinney@intel.com>
> > > Cc: Yao, Jiewen <jiewen.yao@intel.com>; Andrew Fish
> > <afish@apple.com>
> > > Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib:
> > add MSFT toolchain
> > > support
> > >
> > > 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 <jian.j.wang@intel.com>; edk2-devel
> > > > <edk2-devel-bounces@lists.01.org>; edk2-
> > > > devel@lists.01.org
> > > > Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> > Yao,
> > > > Jiewen <jiewen.yao@intel.com>; Andrew Fish
> > > > <afish@apple.com>
> > > > Subject: RE: [edk2] [PATCH] MdePkg/BaseStackCheckLib:
> > add
> > > > MSFT toolchain support
> > > >
> > > > Jian:
> > > > The patch is good to me. Reviewed-by: Liming Gao
> > > > <liming.gao@intel.com>
> > > >
> > > > Thanks
> > > > Liming
> > > > >-----Original Message-----
> > > > >From: Wang, Jian J
> > > > >Sent: Thursday, October 18, 2018 9:19 AM
> > > > >To: edk2-devel <edk2-devel-bounces@lists.01.org>;
> > edk2-
> > > > devel@lists.01.org
> > > > >Cc: Kinney, Michael D <michael.d.kinney@intel.com>;
> > Yao,
> > > > Jiewen
> > > > ><jiewen.yao@intel.com>; Andrew Fish
> > <afish@apple.com>;
> > > > Gao, Liming
> > > > ><liming.gao@intel.com>
> > > > >Subject: RE: [edk2] [PATCH]
> > MdePkg/BaseStackCheckLib:
> > > > add MSFT toolchain
> > > > >support
> > > > >
> > > > >Ping ... :)
> > > > >
> > > > >Regards,
> > > > >Jian
> > > > >
> > > > >
> > > > >> -----Original Message-----
> > > > >> From: edk2-devel [mailto:edk2-devel-
> > > > bounces@lists.01.org]
> > > > >> Sent: Tuesday, October 16, 2018 8:59 AM
> > > > >> To: edk2-devel <edk2-devel-bounces@lists.01.org>;
> > > > edk2-
> > > > >devel@lists.01.org
> > > > >> Cc: Kinney, Michael D
> > <michael.d.kinney@intel.com>;
> > > > Yao, Jiewen
> > > > >> <jiewen.yao@intel.com>; Andrew Fish
> > <afish@apple.com>;
> > > > Gao, Liming
> > > > >> <liming.gao@intel.com>
> > > > >> 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-
> > > > bounces@lists.01.org]
> > > > >> > Sent: Tuesday, October 16, 2018 8:55 AM
> > > > >> > To: edk2-devel@lists.01.org
> > > > >> > Cc: Kinney, Michael D
> > <michael.d.kinney@intel.com>;
> > > > Yao, Jiewen
> > > > >> > <jiewen.yao@intel.com>; Andrew Fish
> > > > <afish@apple.com>; Gao, Liming
> > > > >> > <liming.gao@intel.com>
> > > > >> > 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
> > <michael.d.kinney@intel.com>
> > > > >> > Cc: Liming Gao <liming.gao@intel.com>
> > > > >> > Cc: Jiewen Yao <jiewen.yao@intel.com>
> > > > >> > Cc: Andrew Fish <afish@apple.com>
> > > > >> > Contributed-under: TianoCore Contribution
> > Agreement
> > > > 1.1
> > > > >> > Signed-off-by: Jian J Wang
> > <jian.j.wang@intel.com>
> > > > >> > ---
> > > > >> > .../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
> > > > >> > edk2-devel@lists.01.org
> > > > >> > https://lists.01.org/mailman/listinfo/edk2-devel
> > > > >> _______________________________________________
> > > > >> edk2-devel mailing list
> > > > >> edk2-devel@lists.01.org
> > > > >> https://lists.01.org/mailman/listinfo/edk2-devel
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2018-10-30 2:53 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2018-10-16 0:55 [PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain support Jian J Wang
2018-10-16 0:59 ` Wang, Jian J
2018-10-18 1:18 ` Wang, Jian J
2018-10-18 1:22 ` Gao, Liming
2018-10-18 1:36 ` Kinney, Michael D
2018-10-18 2:15 ` Wang, Jian J
2018-10-18 23:16 ` Kinney, Michael D
2018-10-19 1:02 ` Wang, Jian J
2018-10-30 2:53 ` Wang, Jian J
2018-10-18 6:35 ` Wang, Jian J
2018-10-19 0:05 ` Kinney, Michael D
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox