From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received-SPF: Pass (sender SPF authorized) identity=mailfrom; client-ip=192.55.52.115; helo=mga14.intel.com; envelope-from=jian.j.wang@intel.com; receiver=edk2-devel-bounces@lists.01.org Received: from mga14.intel.com (mga14.intel.com [192.55.52.115]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by ml01.01.org (Postfix) with ESMTPS id 028642116DFB4; Wed, 17 Oct 2018 18:18:40 -0700 (PDT) X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by fmsmga103.fm.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 17 Oct 2018 18:18:40 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.54,393,1534834800"; d="scan'208";a="273365576" Received: from fmsmsx105.amr.corp.intel.com ([10.18.124.203]) by fmsmga006.fm.intel.com with ESMTP; 17 Oct 2018 18:18:40 -0700 Received: from fmsmsx118.amr.corp.intel.com (10.18.116.18) by FMSMSX105.amr.corp.intel.com (10.18.124.203) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 17 Oct 2018 18:18:40 -0700 Received: from shsmsx104.ccr.corp.intel.com (10.239.4.70) by fmsmsx118.amr.corp.intel.com (10.18.116.18) with Microsoft SMTP Server (TLS) id 14.3.319.2; Wed, 17 Oct 2018 18:18:39 -0700 Received: from shsmsx103.ccr.corp.intel.com ([169.254.4.111]) by SHSMSX104.ccr.corp.intel.com ([169.254.5.183]) with mapi id 14.03.0319.002; Thu, 18 Oct 2018 09:18:37 +0800 From: "Wang, Jian J" To: edk2-devel , "edk2-devel@lists.01.org" CC: "Kinney, Michael D" , "Yao, Jiewen" , Andrew Fish , "Gao, Liming" Thread-Topic: [edk2] [PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain support Thread-Index: AQHUZOr5znIuht3+jEiFSa/u1klccKUhDQTggAMqDVA= Date: Thu, 18 Oct 2018 01:18:37 +0000 Message-ID: References: <20181016005518.5264-1-jian.j.wang@intel.com> In-Reply-To: Accept-Language: en-US X-MS-Has-Attach: X-MS-TNEF-Correlator: x-titus-metadata-40: eyJDYXRlZ29yeUxhYmVscyI6IiIsIk1ldGFkYXRhIjp7Im5zIjoiaHR0cDpcL1wvd3d3LnRpdHVzLmNvbVwvbnNcL0ludGVsMyIsImlkIjoiN2UyMzhiZTMtMzRlYi00MDVmLTg5OTgtYjJkZGRlNzA0NjdjIiwicHJvcHMiOlt7Im4iOiJDVFBDbGFzc2lmaWNhdGlvbiIsInZhbHMiOlt7InZhbHVlIjoiQ1RQX05UIn1dfV19LCJTdWJqZWN0TGFiZWxzIjpbXSwiVE1DVmVyc2lvbiI6IjE3LjEwLjE4MDQuNDkiLCJUcnVzdGVkTGFiZWxIYXNoIjoiT3R6WDVic0pQcXJBMWFuXC9xeG1XYzVzN3VLblFvKzFOaXBETWJKNEZQelRGXC9kXC90UDNqZHBQdmE4MkJRYkVmWiJ9 x-ctpclassification: CTP_NT dlp-product: dlpe-windows dlp-version: 11.0.400.15 dlp-reaction: no-action x-originating-ip: [10.239.127.40] MIME-Version: 1.0 Subject: Re: [PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain support X-BeenThere: edk2-devel@lists.01.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: EDK II Development List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 18 Oct 2018 01:18:41 -0000 Content-Language: en-US Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: quoted-printable 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@lists.01.org > Cc: Kinney, Michael D ; Yao, Jiewen > ; Andrew Fish ; Gao, Liming > > Subject: Re: [edk2] [PATCH] MdePkg/BaseStackCheckLib: add MSFT toolchain > support >=20 > REF: https://bugzilla.tianocore.org/show_bug.cgi?id=3D1239 >=20 > Regards, > Jian >=20 >=20 > > -----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 ; Yao, Jiewen > > ; Andrew Fish ; Gao, Liming > > > > 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 =3D /GS /GL- > > MSFT:DEBUG_*_*_CC_FLAGS =3D /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 > > Cc: Liming Gao > > Cc: Jiewen Yao > > Cc: Andrew Fish > > Contributed-under: TianoCore Contribution Agreement 1.1 > > Signed-off-by: Jian J Wang > > --- > > .../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.
> > +# Copyright (c) 2018, Intel Corporation. All rights reserved.
> > # > > # This program and the accompanying materials > > # are licensed and made available under the terms and conditions of t= he BSD > > License > > @@ -26,13 +27,19 @@ > > > > > > # > > -# VALID_ARCHITECTURES =3D ARM AARCH64 > > +# VALID_ARCHITECTURES =3D 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, R= TCs. > > + > > +Copyright (c) 2018, Intel Corporation. All rights reserved.
> > +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 > > + > > +#include > > +#include > > +#include > > + > > +// > > +// cookie value that is inserted by the MSFT compiler into the stack f= rame. > > +// > > +extern UINTN __security_cookie; > > + > > +// > > +// Data structure used by MSFT compiler to record local variable infor= mation. > > +// > > + > > +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 eve= n if > > + // BaseDebugLibNull is in use. > > + // > > + if ((PcdGet8 (PcdDebugPropertyMask) & > > DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) !=3D 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 th= e 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 eve= n if > > + // BaseDebugLibNull is in use. > > + // > > + if ((PcdGet8 (PcdDebugPropertyMask) & > > DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) !=3D 0) { > > + CpuBreakpoint (); > > + } else if ((PcdGet8 (PcdDebugPropertyMask) & > > DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) !=3D 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 =3D 0; Index < _Fd->VarCount; Index++) { > > + Addr =3D (UINT8 *)_Esp + _Fd->Variables[Index].Addr - sizeof(UINT3= 2); > > + if (*(UINT32 *)Addr !=3D RTC_STACK_CHECK_COOKIE) { > > + _RTC_StackFailure (_Fd->Variables[Index].Name, RETURN_ADDRESS(0)= ); > > + } > > + > > + Addr =3D (UINT8 *)_Esp + _Fd->Variables[Index].Addr + _Fd- > > >Variables[Index].Size; > > + if (*(UINT32 *)Addr !=3D RTC_STACK_CHECK_COOKIE) { > > + _RTC_StackFailure (_Fd->Variables[Index].Name, RETURN_ADDRESS(0)= ); > > + } > > + } > > +} > > + > > +/** > > + Function required by linker but not implemented by firmware image lo= ader. > > (/RTCs) > > + > > +**/ > > +VOID > > +__cdecl > > +_RTC_Shutdown ( > > + VOID > > + ) > > +{ > > + return; > > +} > > + > > +/** > > + Function required by linker but not implemented by firmware image lo= ader. > > (/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 eve= n if > > + // BaseDebugLibNull is in use. > > + // > > + if ((PcdGet8 (PcdDebugPropertyMask) & > > DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) !=3D 0) { > > + CpuBreakpoint (); > > + } else if ((PcdGet8 (PcdDebugPropertyMask) & > > DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) !=3D 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 eve= n if > > + // BaseDebugLibNull is in use. > > + // > > + if ((PcdGet8 (PcdDebugPropertyMask) & > > DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) !=3D 0) { > > + CpuBreakpoint (); > > + } else if ((PcdGet8 (PcdDebugPropertyMask) & > > DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) !=3D 0) { > > + CpuDeadLoop (); > > + } > > +} > > + > > +/** > > + Function required by linker but not implemented by firmware image lo= ader. > > (/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.
> > - 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 ma= y 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.
> > +; This program and the accompanying materials > > +; are licensed and made available under the terms and conditions of th= e BSD > > License > > +; which accompanies this distribution. The full text of the license m= ay 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, v= oid > *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 parame= ter > > + push ecx ; cookie value in stack is the first param= eter > > + 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.nas= m > > 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.
> > +; This program and the accompanying materials > > +; are licensed and made available under the terms and conditions of th= e BSD > > License > > +; which accompanies this distribution. The full text of the license m= ay 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, v= oid > *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 parame= ter > > + 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