[PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain support

* [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