| Message ID | 1404747833-653-3-git-send-email-olivier.martin@arm.com |
|---|---|
| State | New |
| Headers | show |
On Jul 7, 2014, at 5:40 PM, Gao, Liming <liming.gao@intel.com> wrote: > Martin: > What is 0x0AFF for? Is it an address or a value? This value is the recommended constant if you can not generate a real random number for the “canary” value. It has NULL (will terminate strings), LF, and -1. http://wiki.osdev.org/GCC_Stack_Smashing_Protector. So it helps contains read based overruns. __stack_chk_guard is the “canary” placed on the stack by the compiler. __stack_check_fail() is called if the “canary” has been over written. These are both compiler intrinsics. Thanks, Andrew Fish ~/work/Compiler>cat stack.c void test (int i, char v) { char test[0x100]; test[i] = v; return; } ~/work/Compiler>clang -S stack.c ~/work/Compiler>cat stack.S .section __TEXT,__text,regular,pure_instructions .globl _test .align 4, 0x90 _test: ## @test .cfi_startproc ## BB#0: pushq %rbp Ltmp2: .cfi_def_cfa_offset 16 Ltmp3: .cfi_offset %rbp, -16 movq %rsp, %rbp Ltmp4: .cfi_def_cfa_register %rbp subq $272, %rsp ## imm = 0x110 movb %sil, %al movq ___stack_chk_guard@GOTPCREL(%rip), %rcx movq (%rcx), %rcx movq %rcx, -8(%rbp) movq ___stack_chk_guard@GOTPCREL(%rip), %rcx movl %edi, -12(%rbp) movb %al, -13(%rbp) movb -13(%rbp), %al movslq -12(%rbp), %rdx movb %al, -272(%rbp,%rdx) movq (%rcx), %rcx movq -8(%rbp), %rdx cmpq %rdx, %rcx jne LBB0_2 ## BB#1: ## %SP_return addq $272, %rsp ## imm = 0x110 popq %rbp ret LBB0_2: ## %CallStackCheckFailBlk callq ___stack_chk_fail .cfi_endproc .subsections_via_symbols > +/// "canary" value that is inserted by the compiler into the stack frame. > +VOID *__stack_chk_guard = (VOID*)0x0AFF; > > And, this library instance is used as NULL class instance. Its library class should be NULL. > > Thanks > Liming > -----Original Message----- > From: Olivier Martin [mailto:olivier.martin@arm.com] > Sent: Monday, July 07, 2014 11:44 PM > To: Kinney, Michael D; edk2-buildtools-devel@lists.sourceforge.net > Cc: andrew.fish@apple.com; edk2-devel@lists.sourceforge.net > Subject: [edk2-buildtools] [PATCH 2/3] MdePkg: Introduced BaseStackCheckLib > > This library only support GCC and XCode for now. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Andrew Fish <andrew.fish@apple.com> > Signed-off-by: Olivier Martin <olivier.martin@arm.com > > Change-Id: Ib51239b7d315637fd22c34a9a78a7a830f2ffdc7 > --- > .../Library/BaseStackCheckLib/BaseStackCheckGcc.c | 62 ++++++++++++++++++++++ > .../BaseStackCheckLib/BaseStackCheckLib.inf | 41 ++++++++++++++ > 2 files changed, 103 insertions(+) > create mode 100644 MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c > create mode 100644 MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > > diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c > new file mode 100644 > index 0000000..9ec76d8 > --- /dev/null > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c > @@ -0,0 +1,62 @@ > +/** @file > + Base Stack Check library for GCC/clang. > + > + Use -fstack-protector-all compiler flag to make the compiler insert > + the __stack_chk_guard "canary" value into the stack and check the > + value prior to exiting the function. If the "canary" is overwritten > + __stack_chk_fail() is called. This is GCC specific code. > + > + Copyright (c) 2012, Apple Inc. 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. > + > +**/ > + > +#include <Base.h> > +#include <Library/BaseLib.h> > +#include <Library/DebugLib.h> > +#include <Library/PcdLib.h> > + > +VOID > +__stack_chk_fail ( > + VOID > + ); > + > + > +/// "canary" value that is inserted by the compiler into the stack frame. > +VOID *__stack_chk_guard = (VOID*)0x0AFF; > + > +// If ASLR was enabled we could use > +//void (*__stack_chk_guard)(void) = __stack_chk_fail; > + > +/** > + Error path for compiler generated stack "canary" value check code. If > +the stack canary has been overwritten this function gets called on > +exit of the function. > +**/ > +VOID > +__stack_chk_fail ( > + VOID > + ) > +{ > + UINT8 DebugPropertyMask; > + > + DEBUG ((DEBUG_ERROR, "STACK FAULT: Buffer Overflow in function > + %a.\n", __builtin_return_address(0))); > + > + // > + // Generate a Breakpoint, DeadLoop, or NOP based on PCD settings even > +if > + // BaseDebugLibNull is in use. > + // > + DebugPropertyMask = PcdGet8 (PcdDebugPropertyMask); > + if ((DebugPropertyMask & DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) { > + CpuBreakpoint (); > + } else if ((DebugPropertyMask & DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) != 0) { > + CpuDeadLoop (); > + } > +} > + > diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > new file mode 100644 > index 0000000..4e2285d > --- /dev/null > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > @@ -0,0 +1,41 @@ > +## @file > +# Stack Check Library > +# > +# Copyright (c) 2014, ARM Ltd. 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. > +# > +# > +## > + > +[Defines] > + INF_VERSION = 0x00010005 > + BASE_NAME = BaseStackCheckLib > + FILE_GUID = 5f6579f7-b648-4fdb-9f19-4c17e27e8eff > + MODULE_TYPE = BASE > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = BaseStackCheckLib > + > + > +# > +# VALID_ARCHITECTURES = ARM AARCH64 > +# > + > +[Sources] > + BaseStackCheckGcc.c | GCC > + > +[Packages] > + MdePkg/MdePkg.dec > + > +[LibraryClasses] > + BaseLib > + DebugLib > + > +[Pcd] > + gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask > + > -- > 1.8.5 > > > ------------------------------------------------------------------------------ > Open source business process management suite built on Java and Eclipse > Turn processes into business applications with Bonita BPM Community Edition > Quickly connect people, data, and systems into organized workflows > Winner of BOSSIE, CODIE, OW2 and Gartner awards > http://p.sf.net/sfu/Bonitasoft > _______________________________________________ > edk2-buildtools-devel mailing list > edk2-buildtools-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-buildtools-devel > > ------------------------------------------------------------------------------ > Open source business process management suite built on Java and Eclipse > Turn processes into business applications with Bonita BPM Community Edition > Quickly connect people, data, and systems into organized workflows > Winner of BOSSIE, CODIE, OW2 and Gartner awards > http://p.sf.net/sfu/Bonitasoft > _______________________________________________ > edk2-buildtools-devel mailing list > edk2-buildtools-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-buildtools-devel ------------------------------------------------------------------------------ Open source business process management suite built on Java and Eclipse Turn processes into business applications with Bonita BPM Community Edition Quickly connect people, data, and systems into organized workflows Winner of BOSSIE, CODIE, OW2 and Gartner awards http://p.sf.net/sfu/Bonitasoft
Only one nibble is NULL. Is that sufficient to terminate a string? Cameron Esfahani dirty@apple.com > On Jul 7, 2014, at 18:14, Andrew Fish <afish@apple.com> wrote: > > It has NULL (will terminate strings), LF, and -1. http://wiki.osdev.org/GCC_Stack_Smashing_Protector. So it helps contains read based overruns. ------------------------------------------------------------------------------ Open source business process management suite built on Java and Eclipse Turn processes into business applications with Bonita BPM Community Edition Quickly connect people, data, and systems into organized workflows Winner of BOSSIE, CODIE, OW2 and Gartner awards http://p.sf.net/sfu/Bonitasoft
On Jul 7, 2014, at 6:39 PM, Cameron Esfahani <dirty@apple.com> wrote: > Only one nibble is NULL. Is that sufficient to terminate a string? > It is a void * so: 0x00000AFF or 0x0000000000000AFF It is about stopping prior going past the “canary”, not the beginning of the “canary”. Thanks, Andrew Fish > Cameron Esfahani > dirty@apple.com > > > On Jul 7, 2014, at 18:14, Andrew Fish <afish@apple.com> wrote: > >> It has NULL (will terminate strings), LF, and -1. http://wiki.osdev.org/GCC_Stack_Smashing_Protector. So it helps contains read based overruns. > ------------------------------------------------------------------------------ > Open source business process management suite built on Java and Eclipse > Turn processes into business applications with Bonita BPM Community Edition > Quickly connect people, data, and systems into organized workflows > Winner of BOSSIE, CODIE, OW2 and Gartner awards > http://p.sf.net/sfu/Bonitasoft_______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel ------------------------------------------------------------------------------ Open source business process management suite built on Java and Eclipse Turn processes into business applications with Bonita BPM Community Edition Quickly connect people, data, and systems into organized workflows Winner of BOSSIE, CODIE, OW2 and Gartner awards http://p.sf.net/sfu/Bonitasoft
On Aug 4, 2014, at 8:30 AM, Olivier Martin <olivier.martin@arm.com> wrote: > Recommended value does not mean unique value. Anyway, no one has commented whether this value should be kept in the code or be replaced by a PCD. So, I will keep the original code. > I will send a new patchset in the next couple of minutes to define this class instance as NULL instead of BASE. > The recommendation is to use a constant like the one in the code, or a random number per boot. So if there was a PCD it would be to use the constant vs. a random number per boot. I don’t think we have a BaseLib compatible way to get a random number? The per boot random number makes it harder for attacking code to figure out where the “canary” is so it can be defeated. It is like ASLR, and a constant PCD value is not going to help with that. Thanks, Andrew Fish > Olivier > > From: Gao, Liming [mailto:liming.gao@intel.com] > Sent: 09 July 2014 03:42 > To: Olivier Martin; 'Andrew Fish' > Cc: Kinney, Michael D; edk2-buildtools-devel@lists.sourceforge.net; edk2-devel@lists.sourceforge.net > Subject: RE: [edk2-buildtools] [PATCH 2/3] MdePkg: Introduced BaseStackCheckLib > > Martin: > Per Andrew comment, this value is the recommended constant. If so, we don’t need to add one PCD for it unless someone has the strong request to configure it. > > Thanks > Liming > From: Olivier Martin [mailto:olivier.martin@arm.com] > Sent: Tuesday, July 08, 2014 9:26 PM > To: 'Andrew Fish'; Gao, Liming > Cc: Kinney, Michael D; edk2-buildtools-devel@lists.sourceforge.net; edk2-devel@lists.sourceforge.net > Subject: RE: [edk2-buildtools] [PATCH 2/3] MdePkg: Introduced BaseStackCheckLib > > Actually, I was thinking to replace this canary value by a FixedPcd but I do not remember why I have not done it. I might have just forgot it. > Olivier > > From: Andrew Fish [mailto:afish@apple.com] > Sent: 08 July 2014 02:14 > To: Gao, Liming > Cc: Olivier Martin; Mike Kinney; edk2-buildtools-devel@lists.sourceforge.net; edk2-devel@lists.sourceforge.net > Subject: Re: [edk2-buildtools] [PATCH 2/3] MdePkg: Introduced BaseStackCheckLib > > > On Jul 7, 2014, at 5:40 PM, Gao, Liming <liming.gao@intel.com> wrote: > > > Martin: > What is 0x0AFF for? Is it an address or a value? > > This value is the recommended constant if you can not generate a real random number for the “canary” value. > > It has NULL (will terminate strings), LF, and -1. http://wiki.osdev.org/GCC_Stack_Smashing_Protector. So it helps contains read based overruns. > > __stack_chk_guard is the “canary” placed on the stack by the compiler. __stack_check_fail() is called if the “canary” has been over written. These are both compiler intrinsics. > > Thanks, > > Andrew Fish > > ~/work/Compiler>cat stack.c > > void > test (int i, char v) > { > char test[0x100]; > > test[i] = v; > return; > } > > ~/work/Compiler>clang -S stack.c > ~/work/Compiler>cat stack.S > .section __TEXT,__text,regular,pure_instructions > .globl _test > .align 4, 0x90 > _test: ## @test > .cfi_startproc > ## BB#0: > pushq %rbp > Ltmp2: > .cfi_def_cfa_offset 16 > Ltmp3: > .cfi_offset %rbp, -16 > movq %rsp, %rbp > Ltmp4: > .cfi_def_cfa_register %rbp > subq $272, %rsp ## imm = 0x110 > movb %sil, %al > movq ___stack_chk_guard@GOTPCREL(%rip), %rcx > movq (%rcx), %rcx > movq %rcx, -8(%rbp) > movq ___stack_chk_guard@GOTPCREL(%rip), %rcx > movl %edi, -12(%rbp) > movb %al, -13(%rbp) > movb -13(%rbp), %al > movslq -12(%rbp), %rdx > movb %al, -272(%rbp,%rdx) > movq (%rcx), %rcx > movq -8(%rbp), %rdx > cmpq %rdx, %rcx > jne LBB0_2 > ## BB#1: ## %SP_return > addq $272, %rsp ## imm = 0x110 > popq %rbp > ret > LBB0_2: ## %CallStackCheckFailBlk > callq ___stack_chk_fail > .cfi_endproc > > > .subsections_via_symbols > > > > +/// "canary" value that is inserted by the compiler into the stack frame. > +VOID *__stack_chk_guard = (VOID*)0x0AFF; > > And, this library instance is used as NULL class instance. Its library class should be NULL. > > Thanks > Liming > -----Original Message----- > From: Olivier Martin [mailto:olivier.martin@arm.com] > Sent: Monday, July 07, 2014 11:44 PM > To: Kinney, Michael D; edk2-buildtools-devel@lists.sourceforge.net > Cc: andrew.fish@apple.com; edk2-devel@lists.sourceforge.net > Subject: [edk2-buildtools] [PATCH 2/3] MdePkg: Introduced BaseStackCheckLib > > This library only support GCC and XCode for now. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Andrew Fish <andrew.fish@apple.com> > Signed-off-by: Olivier Martin <olivier.martin@arm.com > > Change-Id: Ib51239b7d315637fd22c34a9a78a7a830f2ffdc7 > --- > .../Library/BaseStackCheckLib/BaseStackCheckGcc.c | 62 ++++++++++++++++++++++ > .../BaseStackCheckLib/BaseStackCheckLib.inf | 41 ++++++++++++++ > 2 files changed, 103 insertions(+) > create mode 100644 MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c > create mode 100644 MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > > diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c > new file mode 100644 > index 0000000..9ec76d8 > --- /dev/null > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c > @@ -0,0 +1,62 @@ > +/** @file > + Base Stack Check library for GCC/clang. > + > + Use -fstack-protector-all compiler flag to make the compiler insert > + the __stack_chk_guard "canary" value into the stack and check the > + value prior to exiting the function. If the "canary" is overwritten > + __stack_chk_fail() is called. This is GCC specific code. > + > + Copyright (c) 2012, Apple Inc. 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. > + > +**/ > + > +#include <Base.h> > +#include <Library/BaseLib.h> > +#include <Library/DebugLib.h> > +#include <Library/PcdLib.h> > + > +VOID > +__stack_chk_fail ( > + VOID > + ); > + > + > +/// "canary" value that is inserted by the compiler into the stack frame. > +VOID *__stack_chk_guard = (VOID*)0x0AFF; > + > +// If ASLR was enabled we could use > +//void (*__stack_chk_guard)(void) = __stack_chk_fail; > + > +/** > + Error path for compiler generated stack "canary" value check code. If > +the stack canary has been overwritten this function gets called on > +exit of the function. > +**/ > +VOID > +__stack_chk_fail ( > + VOID > + ) > +{ > + UINT8 DebugPropertyMask; > + > + DEBUG ((DEBUG_ERROR, "STACK FAULT: Buffer Overflow in function > + %a.\n", __builtin_return_address(0))); > + > + // > + // Generate a Breakpoint, DeadLoop, or NOP based on PCD settings even > +if > + // BaseDebugLibNull is in use. > + // > + DebugPropertyMask = PcdGet8 (PcdDebugPropertyMask); > + if ((DebugPropertyMask & DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) { > + CpuBreakpoint (); > + } else if ((DebugPropertyMask & DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) != 0) { > + CpuDeadLoop (); > + } > +} > + > diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > new file mode 100644 > index 0000000..4e2285d > --- /dev/null > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > @@ -0,0 +1,41 @@ > +## @file > +# Stack Check Library > +# > +# Copyright (c) 2014, ARM Ltd. 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. > +# > +# > +## > + > +[Defines] > + INF_VERSION = 0x00010005 > + BASE_NAME = BaseStackCheckLib > + FILE_GUID = 5f6579f7-b648-4fdb-9f19-4c17e27e8eff > + MODULE_TYPE = BASE > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = BaseStackCheckLib > + > + > +# > +# VALID_ARCHITECTURES = ARM AARCH64 > +# > + > +[Sources] > + BaseStackCheckGcc.c | GCC > + > +[Packages] > + MdePkg/MdePkg.dec > + > +[LibraryClasses] > + BaseLib > + DebugLib > + > +[Pcd] > + gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask > + > -- > 1.8.5 > > > ------------------------------------------------------------------------------ > Open source business process management suite built on Java and Eclipse > Turn processes into business applications with Bonita BPM Community Edition > Quickly connect people, data, and systems into organized workflows > Winner of BOSSIE, CODIE, OW2 and Gartner awards > http://p.sf.net/sfu/Bonitasoft > _______________________________________________ > edk2-buildtools-devel mailing list > edk2-buildtools-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-buildtools-devel > > ------------------------------------------------------------------------------ > Open source business process management suite built on Java and Eclipse > Turn processes into business applications with Bonita BPM Community Edition > Quickly connect people, data, and systems into organized workflows > Winner of BOSSIE, CODIE, OW2 and Gartner awards > http://p.sf.net/sfu/Bonitasoft > _______________________________________________ > edk2-buildtools-devel mailing list > edk2-buildtools-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-buildtools-devel ------------------------------------------------------------------------------ Infragistics Professional Build stunning WinForms apps today! Reboot your WinForms applications with our WinForms controls. Build a bridge from your legacy apps to the future. http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk
On Aug 4, 2014, at 8:50 AM, Olivier Martin <olivier.martin@arm.com> wrote: > The benefit of the FixedPcd was to be an easy alternative between the recommended constant value and the random number. > A Firmware vendor might want to use a different canary value as the open source project. > I guess the PCD could be useful for debugging a specific issue as well. As long as the reasoning behind the default value for the “canary” is documented in the PCD default I don’t see an issue using a FixedAtBuildPcd. Thanks, Andrew Fish > Olivier > > From: Andrew Fish [mailto:afish@apple.com] > Sent: 04 August 2014 16:44 > To: Olivier Martin > Cc: Gao, Liming; Mike Kinney; edk2-buildtools-devel@lists.sourceforge.net; edk2-devel@lists.sourceforge.net > Subject: Re: [edk2-buildtools] [PATCH 2/3] MdePkg: Introduced BaseStackCheckLib > > > On Aug 4, 2014, at 8:30 AM, Olivier Martin <olivier.martin@arm.com> wrote: > > > Recommended value does not mean unique value. Anyway, no one has commented whether this value should be kept in the code or be replaced by a PCD. So, I will keep the original code. > I will send a new patchset in the next couple of minutes to define this class instance as NULL instead of BASE. > > > The recommendation is to use a constant like the one in the code, or a random number per boot. So if there was a PCD it would be to use the constant vs. a random number per boot. I don’t think we have a BaseLib compatible way to get a random number? > > The per boot random number makes it harder for attacking code to figure out where the “canary” is so it can be defeated. It is like ASLR, and a constant PCD value is not going to help with that. > > Thanks, > > Andrew Fish > > > Olivier > > From: Gao, Liming [mailto:liming.gao@intel.com] > Sent: 09 July 2014 03:42 > To: Olivier Martin; 'Andrew Fish' > Cc: Kinney, Michael D; edk2-buildtools-devel@lists.sourceforge.net; edk2-devel@lists.sourceforge.net > Subject: RE: [edk2-buildtools] [PATCH 2/3] MdePkg: Introduced BaseStackCheckLib > > Martin: > Per Andrew comment, this value is the recommended constant. If so, we don’t need to add one PCD for it unless someone has the strong request to configure it. > > Thanks > Liming > From: Olivier Martin [mailto:olivier.martin@arm.com] > Sent: Tuesday, July 08, 2014 9:26 PM > To: 'Andrew Fish'; Gao, Liming > Cc: Kinney, Michael D; edk2-buildtools-devel@lists.sourceforge.net; edk2-devel@lists.sourceforge.net > Subject: RE: [edk2-buildtools] [PATCH 2/3] MdePkg: Introduced BaseStackCheckLib > > Actually, I was thinking to replace this canary value by a FixedPcd but I do not remember why I have not done it. I might have just forgot it. > Olivier > > From: Andrew Fish [mailto:afish@apple.com] > Sent: 08 July 2014 02:14 > To: Gao, Liming > Cc: Olivier Martin; Mike Kinney; edk2-buildtools-devel@lists.sourceforge.net; edk2-devel@lists.sourceforge.net > Subject: Re: [edk2-buildtools] [PATCH 2/3] MdePkg: Introduced BaseStackCheckLib > > > On Jul 7, 2014, at 5:40 PM, Gao, Liming <liming.gao@intel.com> wrote: > > > Martin: > What is 0x0AFF for? Is it an address or a value? > > This value is the recommended constant if you can not generate a real random number for the “canary” value. > > It has NULL (will terminate strings), LF, and -1. http://wiki.osdev.org/GCC_Stack_Smashing_Protector. So it helps contains read based overruns. > > __stack_chk_guard is the “canary” placed on the stack by the compiler. __stack_check_fail() is called if the “canary” has been over written. These are both compiler intrinsics. > > Thanks, > > Andrew Fish > > ~/work/Compiler>cat stack.c > > void > test (int i, char v) > { > char test[0x100]; > > test[i] = v; > return; > } > > ~/work/Compiler>clang -S stack.c > ~/work/Compiler>cat stack.S > .section __TEXT,__text,regular,pure_instructions > .globl _test > .align 4, 0x90 > _test: ## @test > .cfi_startproc > ## BB#0: > pushq %rbp > Ltmp2: > .cfi_def_cfa_offset 16 > Ltmp3: > .cfi_offset %rbp, -16 > movq %rsp, %rbp > Ltmp4: > .cfi_def_cfa_register %rbp > subq $272, %rsp ## imm = 0x110 > movb %sil, %al > movq ___stack_chk_guard@GOTPCREL(%rip), %rcx > movq (%rcx), %rcx > movq %rcx, -8(%rbp) > movq ___stack_chk_guard@GOTPCREL(%rip), %rcx > movl %edi, -12(%rbp) > movb %al, -13(%rbp) > movb -13(%rbp), %al > movslq -12(%rbp), %rdx > movb %al, -272(%rbp,%rdx) > movq (%rcx), %rcx > movq -8(%rbp), %rdx > cmpq %rdx, %rcx > jne LBB0_2 > ## BB#1: ## %SP_return > addq $272, %rsp ## imm = 0x110 > popq %rbp > ret > LBB0_2: ## %CallStackCheckFailBlk > callq ___stack_chk_fail > .cfi_endproc > > > .subsections_via_symbols > > > > +/// "canary" value that is inserted by the compiler into the stack frame. > +VOID *__stack_chk_guard = (VOID*)0x0AFF; > > And, this library instance is used as NULL class instance. Its library class should be NULL. > > Thanks > Liming > -----Original Message----- > From: Olivier Martin [mailto:olivier.martin@arm.com] > Sent: Monday, July 07, 2014 11:44 PM > To: Kinney, Michael D; edk2-buildtools-devel@lists.sourceforge.net > Cc: andrew.fish@apple.com; edk2-devel@lists.sourceforge.net > Subject: [edk2-buildtools] [PATCH 2/3] MdePkg: Introduced BaseStackCheckLib > > This library only support GCC and XCode for now. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Andrew Fish <andrew.fish@apple.com> > Signed-off-by: Olivier Martin <olivier.martin@arm.com > > Change-Id: Ib51239b7d315637fd22c34a9a78a7a830f2ffdc7 > --- > .../Library/BaseStackCheckLib/BaseStackCheckGcc.c | 62 ++++++++++++++++++++++ > .../BaseStackCheckLib/BaseStackCheckLib.inf | 41 ++++++++++++++ > 2 files changed, 103 insertions(+) > create mode 100644 MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c > create mode 100644 MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > > diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c > new file mode 100644 > index 0000000..9ec76d8 > --- /dev/null > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c > @@ -0,0 +1,62 @@ > +/** @file > + Base Stack Check library for GCC/clang. > + > + Use -fstack-protector-all compiler flag to make the compiler insert > + the __stack_chk_guard "canary" value into the stack and check the > + value prior to exiting the function. If the "canary" is overwritten > + __stack_chk_fail() is called. This is GCC specific code. > + > + Copyright (c) 2012, Apple Inc. 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. > + > +**/ > + > +#include <Base.h> > +#include <Library/BaseLib.h> > +#include <Library/DebugLib.h> > +#include <Library/PcdLib.h> > + > +VOID > +__stack_chk_fail ( > + VOID > + ); > + > + > +/// "canary" value that is inserted by the compiler into the stack frame. > +VOID *__stack_chk_guard = (VOID*)0x0AFF; > + > +// If ASLR was enabled we could use > +//void (*__stack_chk_guard)(void) = __stack_chk_fail; > + > +/** > + Error path for compiler generated stack "canary" value check code. If > +the stack canary has been overwritten this function gets called on > +exit of the function. > +**/ > +VOID > +__stack_chk_fail ( > + VOID > + ) > +{ > + UINT8 DebugPropertyMask; > + > + DEBUG ((DEBUG_ERROR, "STACK FAULT: Buffer Overflow in function > + %a.\n", __builtin_return_address(0))); > + > + // > + // Generate a Breakpoint, DeadLoop, or NOP based on PCD settings even > +if > + // BaseDebugLibNull is in use. > + // > + DebugPropertyMask = PcdGet8 (PcdDebugPropertyMask); > + if ((DebugPropertyMask & DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) { > + CpuBreakpoint (); > + } else if ((DebugPropertyMask & DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) != 0) { > + CpuDeadLoop (); > + } > +} > + > diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > new file mode 100644 > index 0000000..4e2285d > --- /dev/null > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > @@ -0,0 +1,41 @@ > +## @file > +# Stack Check Library > +# > +# Copyright (c) 2014, ARM Ltd. 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. > +# > +# > +## > + > +[Defines] > + INF_VERSION = 0x00010005 > + BASE_NAME = BaseStackCheckLib > + FILE_GUID = 5f6579f7-b648-4fdb-9f19-4c17e27e8eff > + MODULE_TYPE = BASE > + VERSION_STRING = 1.0 > + LIBRARY_CLASS = BaseStackCheckLib > + > + > +# > +# VALID_ARCHITECTURES = ARM AARCH64 > +# > + > +[Sources] > + BaseStackCheckGcc.c | GCC > + > +[Packages] > + MdePkg/MdePkg.dec > + > +[LibraryClasses] > + BaseLib > + DebugLib > + > +[Pcd] > + gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask > + > -- > 1.8.5 > > > ------------------------------------------------------------------------------ > Open source business process management suite built on Java and Eclipse > Turn processes into business applications with Bonita BPM Community Edition > Quickly connect people, data, and systems into organized workflows > Winner of BOSSIE, CODIE, OW2 and Gartner awards > http://p.sf.net/sfu/Bonitasoft > _______________________________________________ > edk2-buildtools-devel mailing list > edk2-buildtools-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-buildtools-devel > > ------------------------------------------------------------------------------ > Open source business process management suite built on Java and Eclipse > Turn processes into business applications with Bonita BPM Community Edition > Quickly connect people, data, and systems into organized workflows > Winner of BOSSIE, CODIE, OW2 and Gartner awards > http://p.sf.net/sfu/Bonitasoft > _______________________________________________ > edk2-buildtools-devel mailing list > edk2-buildtools-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-buildtools-devel ------------------------------------------------------------------------------ Infragistics Professional Build stunning WinForms apps today! Reboot your WinForms applications with our WinForms controls. Build a bridge from your legacy apps to the future. http://pubads.g.doubleclick.net/gampad/clk?id=153845071&iu=/4140/ostg.clktrk
diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c new file mode 100644 index 0000000..9ec76d8 --- /dev/null +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c @@ -0,0 +1,62 @@ +/** @file + Base Stack Check library for GCC/clang. + + Use -fstack-protector-all compiler flag to make the compiler insert the + __stack_chk_guard "canary" value into the stack and check the value prior + to exiting the function. If the "canary" is overwritten __stack_chk_fail() + is called. This is GCC specific code. + + Copyright (c) 2012, Apple Inc. 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. + +**/ + +#include <Base.h> +#include <Library/BaseLib.h> +#include <Library/DebugLib.h> +#include <Library/PcdLib.h> + +VOID +__stack_chk_fail ( + VOID + ); + + +/// "canary" value that is inserted by the compiler into the stack frame. +VOID *__stack_chk_guard = (VOID*)0x0AFF; + +// If ASLR was enabled we could use +//void (*__stack_chk_guard)(void) = __stack_chk_fail; + +/** + Error path for compiler generated stack "canary" value check code. If the + stack canary has been overwritten this function gets called on exit of the + function. +**/ +VOID +__stack_chk_fail ( + VOID + ) +{ + UINT8 DebugPropertyMask; + + DEBUG ((DEBUG_ERROR, "STACK FAULT: Buffer Overflow in function %a.\n", __builtin_return_address(0))); + + // + // Generate a Breakpoint, DeadLoop, or NOP based on PCD settings even if + // BaseDebugLibNull is in use. + // + DebugPropertyMask = PcdGet8 (PcdDebugPropertyMask); + if ((DebugPropertyMask & DEBUG_PROPERTY_ASSERT_BREAKPOINT_ENABLED) != 0) { + CpuBreakpoint (); + } else if ((DebugPropertyMask & DEBUG_PROPERTY_ASSERT_DEADLOOP_ENABLED) != 0) { + CpuDeadLoop (); + } +} + diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf new file mode 100644 index 0000000..4e2285d --- /dev/null +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf @@ -0,0 +1,41 @@ +## @file +# Stack Check Library +# +# Copyright (c) 2014, ARM Ltd. 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. +# +# +## + +[Defines] + INF_VERSION = 0x00010005 + BASE_NAME = BaseStackCheckLib + FILE_GUID = 5f6579f7-b648-4fdb-9f19-4c17e27e8eff + MODULE_TYPE = BASE + VERSION_STRING = 1.0 + LIBRARY_CLASS = BaseStackCheckLib + + +# +# VALID_ARCHITECTURES = ARM AARCH64 +# + +[Sources] + BaseStackCheckGcc.c | GCC + +[Packages] + MdePkg/MdePkg.dec + +[LibraryClasses] + BaseLib + DebugLib + +[Pcd] + gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask +