| Message ID | 1407232103-27603-1-git-send-email-olivier.martin@arm.com |
|---|---|
| State | New |
| Headers | show |
I addressed your comment 1. into my local patch. But for comment 2., I had initially added BaseStackLib to MdePkg.dsc. But it was not building as the FixedPcd was not defined: ------- /home/olivier/tianocore/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c :26:34: error: "_PCD_VALUE_PcdBaseStackCanary" undeclared here (not in a function) VOID *__stack_chk_guard = (VOID*)FixedPcdGet64 (PcdBaseStackCanary); ------- _PCD_VALUE_* are only generated when a binary module is built. And there is no Module in MdePkg so adding 'NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf' to [LibraryClasses] would not help neither. One solution would be to add 'NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf' to MdeModulePkg.dsc Which solution would you suggested for your comment 2.? Thanks, Olivier > -----Original Message----- > From: Gao, Liming [mailto:liming.gao@intel.com] > Sent: 06 August 2014 04:00 > To: edk2-devel@lists.sourceforge.net; Kinney, Michael D > Subject: Re: [edk2] [PATCH v3 1/3] MdePkg: Introduced BaseStackCheckLib > > Olivier: > I have two minor comment. > 1. The declaration of __stack_chk_fail() misses the function comments. > 2. Add it into Mdepkg.dsc [Components.ARM, Components.AARCH64] section > to verify its build. > > Other part is good to me. Reviewed-by: Gao, Liming > <liming.gao@intel.com> > > Thanks > Liming > -----Original Message----- > From: Olivier Martin [mailto:olivier.martin@arm.com] > Sent: Tuesday, August 05, 2014 5:48 PM > To: Kinney, Michael D > Cc: edk2-devel@lists.sourceforge.net > Subject: [edk2] [PATCH v3 1/3] MdePkg: Introduced BaseStackCheckLib > > This library only support GCC, RVCT and XCode for now. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Andrew Fish <afish@apple.com> > Signed-off-by: Olivier Martin <olivier.martin@arm.com> > --- > .../Library/BaseStackCheckLib/BaseStackCheckGcc.c | 61 > ++++++++++++++++++++++ > .../BaseStackCheckLib/BaseStackCheckLib.inf | 42 > +++++++++++++++ > MdePkg/MdePkg.dec | 4 ++ > 3 files changed, 107 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..4160aff > --- /dev/null > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c > @@ -0,0 +1,61 @@ > +/** @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*)FixedPcdGet64 (PcdBaseStackCanary); > + > +// 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..3304284 > --- /dev/null > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > @@ -0,0 +1,42 @@ > +## @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 = NULL > + > + > +# > +# VALID_ARCHITECTURES = ARM AARCH64 > +# > + > +[Sources] > + BaseStackCheckGcc.c | GCC > + BaseStackCheckGcc.c | RVCT > + > +[Packages] > + MdePkg/MdePkg.dec > + > +[LibraryClasses] > + BaseLib > + DebugLib > + > +[FixedPcd] > + gEfiMdePkgTokenSpaceGuid.PcdBaseStackCanary > + gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index > 4daf3e6..fbb7d2b 100644 > --- a/MdePkg/MdePkg.dec > +++ b/MdePkg/MdePkg.dec > @@ -1544,6 +1544,10 @@ > # The required memory space is decided by the value of > PcdMaximumGuidedExtractHandler. > > gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|0x1000000| > UINT64|0x30001015 > > + ## Canary value for the stack overflow protection. This PCD can be > + used by a firmware vendor # or for debugging purposes to change the > recommended value. > + gEfiMdePkgTokenSpaceGuid.PcdBaseStackCanary|0x0AFF|UINT64|0x0000002A > + > [PcdsFixedAtBuild.IPF] > ## The base address of IO port space for IA64 arch > > gEfiMdePkgTokenSpaceGuid.PcdIoBlockBaseAddressForIpf|0x0ffffc000000|UIN > T64|0x0000000f > -- > 1.8.5 > > > ----------------------------------------------------------------------- > ------- > 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.c > lktrk > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-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.c > lktrk > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-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
Any feedback? Thanks, Olivier > -----Original Message----- > From: Olivier Martin [mailto:olivier.martin@arm.com] > Sent: 06 August 2014 15:49 > To: edk2-devel@lists.sourceforge.net; Kinney, Michael D; 'Gao, Liming' > Subject: Re: [edk2] [PATCH v3 1/3] MdePkg: Introduced BaseStackCheckLib > > I addressed your comment 1. into my local patch. > > But for comment 2., I had initially added BaseStackLib to MdePkg.dsc. > But it was not building as the FixedPcd was not defined: > ------- > /home/olivier/tianocore/MdePkg/Library/BaseStackCheckLib/BaseStackCheck > Gcc.c > :26:34: error: "_PCD_VALUE_PcdBaseStackCanary" undeclared here (not in > a > function) > VOID *__stack_chk_guard = (VOID*)FixedPcdGet64 (PcdBaseStackCanary); > ------- > > _PCD_VALUE_* are only generated when a binary module is built. And > there is > no Module in MdePkg so adding > 'NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf' to > [LibraryClasses] would not help neither. > One solution would be to add > 'NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf' to > MdeModulePkg.dsc > > Which solution would you suggested for your comment 2.? > > Thanks, > Olivier > > > -----Original Message----- > > From: Gao, Liming [mailto:liming.gao@intel.com] > > Sent: 06 August 2014 04:00 > > To: edk2-devel@lists.sourceforge.net; Kinney, Michael D > > Subject: Re: [edk2] [PATCH v3 1/3] MdePkg: Introduced > BaseStackCheckLib > > > > Olivier: > > I have two minor comment. > > 1. The declaration of __stack_chk_fail() misses the function > comments. > > 2. Add it into Mdepkg.dsc [Components.ARM, Components.AARCH64] > section > > to verify its build. > > > > Other part is good to me. Reviewed-by: Gao, Liming > > <liming.gao@intel.com> > > > > Thanks > > Liming > > -----Original Message----- > > From: Olivier Martin [mailto:olivier.martin@arm.com] > > Sent: Tuesday, August 05, 2014 5:48 PM > > To: Kinney, Michael D > > Cc: edk2-devel@lists.sourceforge.net > > Subject: [edk2] [PATCH v3 1/3] MdePkg: Introduced BaseStackCheckLib > > > > This library only support GCC, RVCT and XCode for now. > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Andrew Fish <afish@apple.com> > > Signed-off-by: Olivier Martin <olivier.martin@arm.com> > > --- > > .../Library/BaseStackCheckLib/BaseStackCheckGcc.c | 61 > > ++++++++++++++++++++++ > > .../BaseStackCheckLib/BaseStackCheckLib.inf | 42 > > +++++++++++++++ > > MdePkg/MdePkg.dec | 4 ++ > > 3 files changed, 107 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..4160aff > > --- /dev/null > > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c > > @@ -0,0 +1,61 @@ > > +/** @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*)FixedPcdGet64 (PcdBaseStackCanary); > > + > > +// 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..3304284 > > --- /dev/null > > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > > @@ -0,0 +1,42 @@ > > +## @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 = NULL > > + > > + > > +# > > +# VALID_ARCHITECTURES = ARM AARCH64 > > +# > > + > > +[Sources] > > + BaseStackCheckGcc.c | GCC > > + BaseStackCheckGcc.c | RVCT > > + > > +[Packages] > > + MdePkg/MdePkg.dec > > + > > +[LibraryClasses] > > + BaseLib > > + DebugLib > > + > > +[FixedPcd] > > + gEfiMdePkgTokenSpaceGuid.PcdBaseStackCanary > > + gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask > > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index > > 4daf3e6..fbb7d2b 100644 > > --- a/MdePkg/MdePkg.dec > > +++ b/MdePkg/MdePkg.dec > > @@ -1544,6 +1544,10 @@ > > # The required memory space is decided by the value of > > PcdMaximumGuidedExtractHandler. > > > > > gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|0x1000000| > > UINT64|0x30001015 > > > > + ## Canary value for the stack overflow protection. This PCD can be > > + used by a firmware vendor # or for debugging purposes to change > the > > recommended value. > > + > gEfiMdePkgTokenSpaceGuid.PcdBaseStackCanary|0x0AFF|UINT64|0x0000002A > > + > > [PcdsFixedAtBuild.IPF] > > ## The base address of IO port space for IA64 arch > > > > > gEfiMdePkgTokenSpaceGuid.PcdIoBlockBaseAddressForIpf|0x0ffffc000000|UIN > > T64|0x0000000f > > -- > > 1.8.5 > > > > > > --------------------------------------------------------------------- > -- > > ------- > > 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.c > > lktrk > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/edk2-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.c > > lktrk > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/edk2-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.c > lktrk > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that matters. http://tv.slashdot.org/
Oliver: Sorry for late response. Library can't use FixedPcdGet(), because it may be linked to the different drivers those may configure this PCD with the different value. That may be the reason you don't do it before. Because this value is the recommended constant, it should be fine to hard code it first without PCD. Thanks Liming -----Original Message----- From: Olivier Martin [mailto:olivier.martin@arm.com] Sent: Wednesday, August 20, 2014 5:47 PM To: edk2-devel@lists.sourceforge.net; Kinney, Michael D; Gao, Liming Subject: RE: [edk2] [PATCH v3 1/3] MdePkg: Introduced BaseStackCheckLib Any feedback? Thanks, Olivier > -----Original Message----- > From: Olivier Martin [mailto:olivier.martin@arm.com] > Sent: 06 August 2014 15:49 > To: edk2-devel@lists.sourceforge.net; Kinney, Michael D; 'Gao, Liming' > Subject: Re: [edk2] [PATCH v3 1/3] MdePkg: Introduced > BaseStackCheckLib > > I addressed your comment 1. into my local patch. > > But for comment 2., I had initially added BaseStackLib to MdePkg.dsc. > But it was not building as the FixedPcd was not defined: > ------- > /home/olivier/tianocore/MdePkg/Library/BaseStackCheckLib/BaseStackChec > k > Gcc.c > :26:34: error: "_PCD_VALUE_PcdBaseStackCanary" undeclared here (not in > a > function) > VOID *__stack_chk_guard = (VOID*)FixedPcdGet64 (PcdBaseStackCanary); > ------- > > _PCD_VALUE_* are only generated when a binary module is built. And > there is no Module in MdePkg so adding > 'NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf' to > [LibraryClasses] would not help neither. > One solution would be to add > 'NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf' to > MdeModulePkg.dsc > > Which solution would you suggested for your comment 2.? > > Thanks, > Olivier > > > -----Original Message----- > > From: Gao, Liming [mailto:liming.gao@intel.com] > > Sent: 06 August 2014 04:00 > > To: edk2-devel@lists.sourceforge.net; Kinney, Michael D > > Subject: Re: [edk2] [PATCH v3 1/3] MdePkg: Introduced > BaseStackCheckLib > > > > Olivier: > > I have two minor comment. > > 1. The declaration of __stack_chk_fail() misses the function > comments. > > 2. Add it into Mdepkg.dsc [Components.ARM, Components.AARCH64] > section > > to verify its build. > > > > Other part is good to me. Reviewed-by: Gao, Liming > > <liming.gao@intel.com> > > > > Thanks > > Liming > > -----Original Message----- > > From: Olivier Martin [mailto:olivier.martin@arm.com] > > Sent: Tuesday, August 05, 2014 5:48 PM > > To: Kinney, Michael D > > Cc: edk2-devel@lists.sourceforge.net > > Subject: [edk2] [PATCH v3 1/3] MdePkg: Introduced BaseStackCheckLib > > > > This library only support GCC, RVCT and XCode for now. > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > Signed-off-by: Andrew Fish <afish@apple.com> > > Signed-off-by: Olivier Martin <olivier.martin@arm.com> > > --- > > .../Library/BaseStackCheckLib/BaseStackCheckGcc.c | 61 > > ++++++++++++++++++++++ > > .../BaseStackCheckLib/BaseStackCheckLib.inf | 42 > > +++++++++++++++ > > MdePkg/MdePkg.dec | 4 ++ > > 3 files changed, 107 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..4160aff > > --- /dev/null > > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c > > @@ -0,0 +1,61 @@ > > +/** @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*)FixedPcdGet64 > > +(PcdBaseStackCanary); > > + > > +// 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..3304284 > > --- /dev/null > > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > > @@ -0,0 +1,42 @@ > > +## @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 = NULL > > + > > + > > +# > > +# VALID_ARCHITECTURES = ARM AARCH64 > > +# > > + > > +[Sources] > > + BaseStackCheckGcc.c | GCC > > + BaseStackCheckGcc.c | RVCT > > + > > +[Packages] > > + MdePkg/MdePkg.dec > > + > > +[LibraryClasses] > > + BaseLib > > + DebugLib > > + > > +[FixedPcd] > > + gEfiMdePkgTokenSpaceGuid.PcdBaseStackCanary > > + gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask > > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index > > 4daf3e6..fbb7d2b 100644 > > --- a/MdePkg/MdePkg.dec > > +++ b/MdePkg/MdePkg.dec > > @@ -1544,6 +1544,10 @@ > > # The required memory space is decided by the value of > > PcdMaximumGuidedExtractHandler. > > > > > gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|0x1000000 > | > > UINT64|0x30001015 > > > > + ## Canary value for the stack overflow protection. This PCD can > > + be used by a firmware vendor # or for debugging purposes to > > + change > the > > recommended value. > > + > gEfiMdePkgTokenSpaceGuid.PcdBaseStackCanary|0x0AFF|UINT64|0x0000002A > > + > > [PcdsFixedAtBuild.IPF] > > ## The base address of IO port space for IA64 arch > > > > > gEfiMdePkgTokenSpaceGuid.PcdIoBlockBaseAddressForIpf|0x0ffffc000000|UI > N > > T64|0x0000000f > > -- > > 1.8.5 > > > > > > -------------------------------------------------------------------- > > - > -- > > ------- > > 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. > c > > lktrk > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/edk2-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. > c > > lktrk > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/edk2-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. > c > lktrk > _______________________________________________ > edk2-devel mailing list > edk2-devel@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/edk2-devel ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that matters. http://tv.slashdot.org/
Are you happy to add your "Reviewed-By" for this patch? If yes do you want me to commit it? Or you are happy to do it? Thanks, Olivier > -----Original Message----- > From: Gao, Liming [mailto:liming.gao@intel.com] > Sent: 20 August 2014 16:23 > To: Olivier Martin; edk2-devel@lists.sourceforge.net; Kinney, Michael D > Subject: RE: [edk2] [PATCH v3 1/3] MdePkg: Introduced BaseStackCheckLib > > Oliver: > Sorry for late response. Library can't use FixedPcdGet(), because it > may be linked to the different drivers those may configure this PCD > with the different value. That may be the reason you don't do it > before. > Because this value is the recommended constant, it should be fine to > hard code it first without PCD. > > Thanks > Liming > -----Original Message----- > From: Olivier Martin [mailto:olivier.martin@arm.com] > Sent: Wednesday, August 20, 2014 5:47 PM > To: edk2-devel@lists.sourceforge.net; Kinney, Michael D; Gao, Liming > Subject: RE: [edk2] [PATCH v3 1/3] MdePkg: Introduced BaseStackCheckLib > > Any feedback? > Thanks, > Olivier > > > -----Original Message----- > > From: Olivier Martin [mailto:olivier.martin@arm.com] > > Sent: 06 August 2014 15:49 > > To: edk2-devel@lists.sourceforge.net; Kinney, Michael D; 'Gao, > Liming' > > Subject: Re: [edk2] [PATCH v3 1/3] MdePkg: Introduced > > BaseStackCheckLib > > > > I addressed your comment 1. into my local patch. > > > > But for comment 2., I had initially added BaseStackLib to MdePkg.dsc. > > But it was not building as the FixedPcd was not defined: > > ------- > > > /home/olivier/tianocore/MdePkg/Library/BaseStackCheckLib/BaseStackChec > > k > > Gcc.c > > :26:34: error: "_PCD_VALUE_PcdBaseStackCanary" undeclared here (not > in > > a > > function) > > VOID *__stack_chk_guard = (VOID*)FixedPcdGet64 (PcdBaseStackCanary); > > ------- > > > > _PCD_VALUE_* are only generated when a binary module is built. And > > there is no Module in MdePkg so adding > > 'NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf' to > > [LibraryClasses] would not help neither. > > One solution would be to add > > 'NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf' to > > MdeModulePkg.dsc > > > > Which solution would you suggested for your comment 2.? > > > > Thanks, > > Olivier > > > > > -----Original Message----- > > > From: Gao, Liming [mailto:liming.gao@intel.com] > > > Sent: 06 August 2014 04:00 > > > To: edk2-devel@lists.sourceforge.net; Kinney, Michael D > > > Subject: Re: [edk2] [PATCH v3 1/3] MdePkg: Introduced > > BaseStackCheckLib > > > > > > Olivier: > > > I have two minor comment. > > > 1. The declaration of __stack_chk_fail() misses the function > > comments. > > > 2. Add it into Mdepkg.dsc [Components.ARM, Components.AARCH64] > > section > > > to verify its build. > > > > > > Other part is good to me. Reviewed-by: Gao, Liming > > > <liming.gao@intel.com> > > > > > > Thanks > > > Liming > > > -----Original Message----- > > > From: Olivier Martin [mailto:olivier.martin@arm.com] > > > Sent: Tuesday, August 05, 2014 5:48 PM > > > To: Kinney, Michael D > > > Cc: edk2-devel@lists.sourceforge.net > > > Subject: [edk2] [PATCH v3 1/3] MdePkg: Introduced BaseStackCheckLib > > > > > > This library only support GCC, RVCT and XCode for now. > > > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > > Signed-off-by: Andrew Fish <afish@apple.com> > > > Signed-off-by: Olivier Martin <olivier.martin@arm.com> > > > --- > > > .../Library/BaseStackCheckLib/BaseStackCheckGcc.c | 61 > > > ++++++++++++++++++++++ > > > .../BaseStackCheckLib/BaseStackCheckLib.inf | 42 > > > +++++++++++++++ > > > MdePkg/MdePkg.dec | 4 ++ > > > 3 files changed, 107 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..4160aff > > > --- /dev/null > > > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c > > > @@ -0,0 +1,61 @@ > > > +/** @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*)FixedPcdGet64 > > > +(PcdBaseStackCanary); > > > + > > > +// 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..3304284 > > > --- /dev/null > > > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > > > @@ -0,0 +1,42 @@ > > > +## @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 = NULL > > > + > > > + > > > +# > > > +# VALID_ARCHITECTURES = ARM AARCH64 > > > +# > > > + > > > +[Sources] > > > + BaseStackCheckGcc.c | GCC > > > + BaseStackCheckGcc.c | RVCT > > > + > > > +[Packages] > > > + MdePkg/MdePkg.dec > > > + > > > +[LibraryClasses] > > > + BaseLib > > > + DebugLib > > > + > > > +[FixedPcd] > > > + gEfiMdePkgTokenSpaceGuid.PcdBaseStackCanary > > > + gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask > > > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index > > > 4daf3e6..fbb7d2b 100644 > > > --- a/MdePkg/MdePkg.dec > > > +++ b/MdePkg/MdePkg.dec > > > @@ -1544,6 +1544,10 @@ > > > # The required memory space is decided by the value of > > > PcdMaximumGuidedExtractHandler. > > > > > > > > > gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|0x1000000 > > | > > > UINT64|0x30001015 > > > > > > + ## Canary value for the stack overflow protection. This PCD can > > > + be used by a firmware vendor # or for debugging purposes to > > > + change > > the > > > recommended value. > > > + > > gEfiMdePkgTokenSpaceGuid.PcdBaseStackCanary|0x0AFF|UINT64|0x0000002A > > > + > > > [PcdsFixedAtBuild.IPF] > > > ## The base address of IO port space for IA64 arch > > > > > > > > > gEfiMdePkgTokenSpaceGuid.PcdIoBlockBaseAddressForIpf|0x0ffffc000000|UI > > N > > > T64|0x0000000f > > > -- > > > 1.8.5 > > > > > > > > > ------------------------------------------------------------------- > - > > > - > > -- > > > ------- > > > 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. > > c > > > lktrk > > > _______________________________________________ > > > edk2-devel mailing list > > > edk2-devel@lists.sourceforge.net > > > https://lists.sourceforge.net/lists/listinfo/edk2-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. > > c > > > lktrk > > > _______________________________________________ > > > edk2-devel mailing list > > > edk2-devel@lists.sourceforge.net > > > https://lists.sourceforge.net/lists/listinfo/edk2-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. > > c > > lktrk > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/edk2-devel > > > > ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that matters. http://tv.slashdot.org/
Yes. Please go ahead. Reviewed-by: Gao, Liming <liming.gao@intel.com> -----Original Message----- From: Olivier Martin [mailto:olivier.martin@arm.com] Sent: Wednesday, August 20, 2014 11:29 PM To: Gao, Liming; edk2-devel@lists.sourceforge.net; Kinney, Michael D Subject: RE: [edk2] [PATCH v3 1/3] MdePkg: Introduced BaseStackCheckLib Are you happy to add your "Reviewed-By" for this patch? If yes do you want me to commit it? Or you are happy to do it? Thanks, Olivier > -----Original Message----- > From: Gao, Liming [mailto:liming.gao@intel.com] > Sent: 20 August 2014 16:23 > To: Olivier Martin; edk2-devel@lists.sourceforge.net; Kinney, Michael > D > Subject: RE: [edk2] [PATCH v3 1/3] MdePkg: Introduced > BaseStackCheckLib > > Oliver: > Sorry for late response. Library can't use FixedPcdGet(), because it > may be linked to the different drivers those may configure this PCD > with the different value. That may be the reason you don't do it > before. > Because this value is the recommended constant, it should be fine to > hard code it first without PCD. > > Thanks > Liming > -----Original Message----- > From: Olivier Martin [mailto:olivier.martin@arm.com] > Sent: Wednesday, August 20, 2014 5:47 PM > To: edk2-devel@lists.sourceforge.net; Kinney, Michael D; Gao, Liming > Subject: RE: [edk2] [PATCH v3 1/3] MdePkg: Introduced > BaseStackCheckLib > > Any feedback? > Thanks, > Olivier > > > -----Original Message----- > > From: Olivier Martin [mailto:olivier.martin@arm.com] > > Sent: 06 August 2014 15:49 > > To: edk2-devel@lists.sourceforge.net; Kinney, Michael D; 'Gao, > Liming' > > Subject: Re: [edk2] [PATCH v3 1/3] MdePkg: Introduced > > BaseStackCheckLib > > > > I addressed your comment 1. into my local patch. > > > > But for comment 2., I had initially added BaseStackLib to MdePkg.dsc. > > But it was not building as the FixedPcd was not defined: > > ------- > > > /home/olivier/tianocore/MdePkg/Library/BaseStackCheckLib/BaseStackChec > > k > > Gcc.c > > :26:34: error: "_PCD_VALUE_PcdBaseStackCanary" undeclared here (not > in > > a > > function) > > VOID *__stack_chk_guard = (VOID*)FixedPcdGet64 > > (PcdBaseStackCanary); > > ------- > > > > _PCD_VALUE_* are only generated when a binary module is built. And > > there is no Module in MdePkg so adding > > 'NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf' to > > [LibraryClasses] would not help neither. > > One solution would be to add > > 'NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf' to > > MdeModulePkg.dsc > > > > Which solution would you suggested for your comment 2.? > > > > Thanks, > > Olivier > > > > > -----Original Message----- > > > From: Gao, Liming [mailto:liming.gao@intel.com] > > > Sent: 06 August 2014 04:00 > > > To: edk2-devel@lists.sourceforge.net; Kinney, Michael D > > > Subject: Re: [edk2] [PATCH v3 1/3] MdePkg: Introduced > > BaseStackCheckLib > > > > > > Olivier: > > > I have two minor comment. > > > 1. The declaration of __stack_chk_fail() misses the function > > comments. > > > 2. Add it into Mdepkg.dsc [Components.ARM, Components.AARCH64] > > section > > > to verify its build. > > > > > > Other part is good to me. Reviewed-by: Gao, Liming > > > <liming.gao@intel.com> > > > > > > Thanks > > > Liming > > > -----Original Message----- > > > From: Olivier Martin [mailto:olivier.martin@arm.com] > > > Sent: Tuesday, August 05, 2014 5:48 PM > > > To: Kinney, Michael D > > > Cc: edk2-devel@lists.sourceforge.net > > > Subject: [edk2] [PATCH v3 1/3] MdePkg: Introduced > > > BaseStackCheckLib > > > > > > This library only support GCC, RVCT and XCode for now. > > > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > > Signed-off-by: Andrew Fish <afish@apple.com> > > > Signed-off-by: Olivier Martin <olivier.martin@arm.com> > > > --- > > > .../Library/BaseStackCheckLib/BaseStackCheckGcc.c | 61 > > > ++++++++++++++++++++++ > > > .../BaseStackCheckLib/BaseStackCheckLib.inf | 42 > > > +++++++++++++++ > > > MdePkg/MdePkg.dec | 4 ++ > > > 3 files changed, 107 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..4160aff > > > --- /dev/null > > > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c > > > @@ -0,0 +1,61 @@ > > > +/** @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*)FixedPcdGet64 > > > +(PcdBaseStackCanary); > > > + > > > +// 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..3304284 > > > --- /dev/null > > > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > > > @@ -0,0 +1,42 @@ > > > +## @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 = NULL > > > + > > > + > > > +# > > > +# VALID_ARCHITECTURES = ARM AARCH64 > > > +# > > > + > > > +[Sources] > > > + BaseStackCheckGcc.c | GCC > > > + BaseStackCheckGcc.c | RVCT > > > + > > > +[Packages] > > > + MdePkg/MdePkg.dec > > > + > > > +[LibraryClasses] > > > + BaseLib > > > + DebugLib > > > + > > > +[FixedPcd] > > > + gEfiMdePkgTokenSpaceGuid.PcdBaseStackCanary > > > + gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask > > > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index > > > 4daf3e6..fbb7d2b 100644 > > > --- a/MdePkg/MdePkg.dec > > > +++ b/MdePkg/MdePkg.dec > > > @@ -1544,6 +1544,10 @@ > > > # The required memory space is decided by the value of > > > PcdMaximumGuidedExtractHandler. > > > > > > > > > gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|0x1000000 > > | > > > UINT64|0x30001015 > > > > > > + ## Canary value for the stack overflow protection. This PCD can > > > + be used by a firmware vendor # or for debugging purposes to > > > + change > > the > > > recommended value. > > > + > > gEfiMdePkgTokenSpaceGuid.PcdBaseStackCanary|0x0AFF|UINT64|0x0000002A > > > + > > > [PcdsFixedAtBuild.IPF] > > > ## The base address of IO port space for IA64 arch > > > > > > > > > gEfiMdePkgTokenSpaceGuid.PcdIoBlockBaseAddressForIpf|0x0ffffc000000|UI > > N > > > T64|0x0000000f > > > -- > > > 1.8.5 > > > > > > > > > ------------------------------------------------------------------ > > > - > - > > > - > > -- > > > ------- > > > 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. > > c > > > lktrk > > > _______________________________________________ > > > edk2-devel mailing list > > > edk2-devel@lists.sourceforge.net > > > https://lists.sourceforge.net/lists/listinfo/edk2-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. > > c > > > lktrk > > > _______________________________________________ > > > edk2-devel mailing list > > > edk2-devel@lists.sourceforge.net > > > https://lists.sourceforge.net/lists/listinfo/edk2-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. > > c > > lktrk > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/edk2-devel > > > > ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that matters. http://tv.slashdot.org/
Oliver: I see your commit patch still includes FixedPcdGet() usage in library. Does it pass build? If not, how about remove PCD first. And, another issue that new introduced PCD gEfiMdePkgTokenSpaceGuid.PcdBaseStackCanary token number is same to gEfiMdePkgTokenSpaceGuid.PcdValidateOrderedCollection. But, PCD token number should be unique. So, its token number should be changed to another value, such as 0x0000002b. Thanks Liming -----Original Message----- From: Olivier Martin [mailto:olivier.martin@arm.com] Sent: Wednesday, August 20, 2014 11:29 PM To: Gao, Liming; edk2-devel@lists.sourceforge.net; Kinney, Michael D Subject: RE: [edk2] [PATCH v3 1/3] MdePkg: Introduced BaseStackCheckLib Are you happy to add your "Reviewed-By" for this patch? If yes do you want me to commit it? Or you are happy to do it? Thanks, Olivier > -----Original Message----- > From: Gao, Liming [mailto:liming.gao@intel.com] > Sent: 20 August 2014 16:23 > To: Olivier Martin; edk2-devel@lists.sourceforge.net; Kinney, Michael > D > Subject: RE: [edk2] [PATCH v3 1/3] MdePkg: Introduced > BaseStackCheckLib > > Oliver: > Sorry for late response. Library can't use FixedPcdGet(), because it > may be linked to the different drivers those may configure this PCD > with the different value. That may be the reason you don't do it > before. > Because this value is the recommended constant, it should be fine to > hard code it first without PCD. > > Thanks > Liming > -----Original Message----- > From: Olivier Martin [mailto:olivier.martin@arm.com] > Sent: Wednesday, August 20, 2014 5:47 PM > To: edk2-devel@lists.sourceforge.net; Kinney, Michael D; Gao, Liming > Subject: RE: [edk2] [PATCH v3 1/3] MdePkg: Introduced > BaseStackCheckLib > > Any feedback? > Thanks, > Olivier > > > -----Original Message----- > > From: Olivier Martin [mailto:olivier.martin@arm.com] > > Sent: 06 August 2014 15:49 > > To: edk2-devel@lists.sourceforge.net; Kinney, Michael D; 'Gao, > Liming' > > Subject: Re: [edk2] [PATCH v3 1/3] MdePkg: Introduced > > BaseStackCheckLib > > > > I addressed your comment 1. into my local patch. > > > > But for comment 2., I had initially added BaseStackLib to MdePkg.dsc. > > But it was not building as the FixedPcd was not defined: > > ------- > > > /home/olivier/tianocore/MdePkg/Library/BaseStackCheckLib/BaseStackChec > > k > > Gcc.c > > :26:34: error: "_PCD_VALUE_PcdBaseStackCanary" undeclared here (not > in > > a > > function) > > VOID *__stack_chk_guard = (VOID*)FixedPcdGet64 > > (PcdBaseStackCanary); > > ------- > > > > _PCD_VALUE_* are only generated when a binary module is built. And > > there is no Module in MdePkg so adding > > 'NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf' to > > [LibraryClasses] would not help neither. > > One solution would be to add > > 'NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf' to > > MdeModulePkg.dsc > > > > Which solution would you suggested for your comment 2.? > > > > Thanks, > > Olivier > > > > > -----Original Message----- > > > From: Gao, Liming [mailto:liming.gao@intel.com] > > > Sent: 06 August 2014 04:00 > > > To: edk2-devel@lists.sourceforge.net; Kinney, Michael D > > > Subject: Re: [edk2] [PATCH v3 1/3] MdePkg: Introduced > > BaseStackCheckLib > > > > > > Olivier: > > > I have two minor comment. > > > 1. The declaration of __stack_chk_fail() misses the function > > comments. > > > 2. Add it into Mdepkg.dsc [Components.ARM, Components.AARCH64] > > section > > > to verify its build. > > > > > > Other part is good to me. Reviewed-by: Gao, Liming > > > <liming.gao@intel.com> > > > > > > Thanks > > > Liming > > > -----Original Message----- > > > From: Olivier Martin [mailto:olivier.martin@arm.com] > > > Sent: Tuesday, August 05, 2014 5:48 PM > > > To: Kinney, Michael D > > > Cc: edk2-devel@lists.sourceforge.net > > > Subject: [edk2] [PATCH v3 1/3] MdePkg: Introduced > > > BaseStackCheckLib > > > > > > This library only support GCC, RVCT and XCode for now. > > > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > > Signed-off-by: Andrew Fish <afish@apple.com> > > > Signed-off-by: Olivier Martin <olivier.martin@arm.com> > > > --- > > > .../Library/BaseStackCheckLib/BaseStackCheckGcc.c | 61 > > > ++++++++++++++++++++++ > > > .../BaseStackCheckLib/BaseStackCheckLib.inf | 42 > > > +++++++++++++++ > > > MdePkg/MdePkg.dec | 4 ++ > > > 3 files changed, 107 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..4160aff > > > --- /dev/null > > > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c > > > @@ -0,0 +1,61 @@ > > > +/** @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*)FixedPcdGet64 > > > +(PcdBaseStackCanary); > > > + > > > +// 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..3304284 > > > --- /dev/null > > > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > > > @@ -0,0 +1,42 @@ > > > +## @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 = NULL > > > + > > > + > > > +# > > > +# VALID_ARCHITECTURES = ARM AARCH64 > > > +# > > > + > > > +[Sources] > > > + BaseStackCheckGcc.c | GCC > > > + BaseStackCheckGcc.c | RVCT > > > + > > > +[Packages] > > > + MdePkg/MdePkg.dec > > > + > > > +[LibraryClasses] > > > + BaseLib > > > + DebugLib > > > + > > > +[FixedPcd] > > > + gEfiMdePkgTokenSpaceGuid.PcdBaseStackCanary > > > + gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask > > > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index > > > 4daf3e6..fbb7d2b 100644 > > > --- a/MdePkg/MdePkg.dec > > > +++ b/MdePkg/MdePkg.dec > > > @@ -1544,6 +1544,10 @@ > > > # The required memory space is decided by the value of > > > PcdMaximumGuidedExtractHandler. > > > > > > > > > gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|0x1000000 > > | > > > UINT64|0x30001015 > > > > > > + ## Canary value for the stack overflow protection. This PCD can > > > + be used by a firmware vendor # or for debugging purposes to > > > + change > > the > > > recommended value. > > > + > > gEfiMdePkgTokenSpaceGuid.PcdBaseStackCanary|0x0AFF|UINT64|0x0000002A > > > + > > > [PcdsFixedAtBuild.IPF] > > > ## The base address of IO port space for IA64 arch > > > > > > > > > gEfiMdePkgTokenSpaceGuid.PcdIoBlockBaseAddressForIpf|0x0ffffc000000|UI > > N > > > T64|0x0000000f > > > -- > > > 1.8.5 > > > > > > > > > ------------------------------------------------------------------ > > > - > - > > > - > > -- > > > ------- > > > 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. > > c > > > lktrk > > > _______________________________________________ > > > edk2-devel mailing list > > > edk2-devel@lists.sourceforge.net > > > https://lists.sourceforge.net/lists/listinfo/edk2-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. > > c > > > lktrk > > > _______________________________________________ > > > edk2-devel mailing list > > > edk2-devel@lists.sourceforge.net > > > https://lists.sourceforge.net/lists/listinfo/edk2-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. > > c > > lktrk > > _______________________________________________ > > edk2-devel mailing list > > edk2-devel@lists.sourceforge.net > > https://lists.sourceforge.net/lists/listinfo/edk2-devel > > > > ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that matters. http://tv.slashdot.org/
Sorry, I committed the v3 patch without taking attention of its content. The FixedPcdGet() works well in the NULL library because the library is linked to a module. So the FixedPcdGet behaves correctly in this case. But the library cannot be built by itself with FixedPcdGet(). I think it was better to keep the PCD. But you are the maintainer. So, I have just pushed SVN rev15866 that makes the changes you suggested (that removes the PCD). Thanks, Olivier > -----Original Message----- > From: Gao, Liming [mailto:liming.gao@intel.com] > Sent: 21 August 2014 09:59 > To: Olivier Martin; edk2-devel@lists.sourceforge.net; Kinney, Michael D > Subject: RE: [edk2] [PATCH v3 1/3] MdePkg: Introduced BaseStackCheckLib > > Oliver: > I see your commit patch still includes FixedPcdGet() usage in > library. Does it pass build? If not, how about remove PCD first. > > And, another issue that new introduced PCD > gEfiMdePkgTokenSpaceGuid.PcdBaseStackCanary token number is same to > gEfiMdePkgTokenSpaceGuid.PcdValidateOrderedCollection. But, PCD token > number should be unique. So, its token number should be changed to > another value, such as 0x0000002b. > > Thanks > Liming > -----Original Message----- > From: Olivier Martin [mailto:olivier.martin@arm.com] > Sent: Wednesday, August 20, 2014 11:29 PM > To: Gao, Liming; edk2-devel@lists.sourceforge.net; Kinney, Michael D > Subject: RE: [edk2] [PATCH v3 1/3] MdePkg: Introduced BaseStackCheckLib > > Are you happy to add your "Reviewed-By" for this patch? > If yes do you want me to commit it? Or you are happy to do it? > > Thanks, > Olivier > > > -----Original Message----- > > From: Gao, Liming [mailto:liming.gao@intel.com] > > Sent: 20 August 2014 16:23 > > To: Olivier Martin; edk2-devel@lists.sourceforge.net; Kinney, Michael > > D > > Subject: RE: [edk2] [PATCH v3 1/3] MdePkg: Introduced > > BaseStackCheckLib > > > > Oliver: > > Sorry for late response. Library can't use FixedPcdGet(), because > it > > may be linked to the different drivers those may configure this PCD > > with the different value. That may be the reason you don't do it > > before. > > Because this value is the recommended constant, it should be fine > to > > hard code it first without PCD. > > > > Thanks > > Liming > > -----Original Message----- > > From: Olivier Martin [mailto:olivier.martin@arm.com] > > Sent: Wednesday, August 20, 2014 5:47 PM > > To: edk2-devel@lists.sourceforge.net; Kinney, Michael D; Gao, Liming > > Subject: RE: [edk2] [PATCH v3 1/3] MdePkg: Introduced > > BaseStackCheckLib > > > > Any feedback? > > Thanks, > > Olivier > > > > > -----Original Message----- > > > From: Olivier Martin [mailto:olivier.martin@arm.com] > > > Sent: 06 August 2014 15:49 > > > To: edk2-devel@lists.sourceforge.net; Kinney, Michael D; 'Gao, > > Liming' > > > Subject: Re: [edk2] [PATCH v3 1/3] MdePkg: Introduced > > > BaseStackCheckLib > > > > > > I addressed your comment 1. into my local patch. > > > > > > But for comment 2., I had initially added BaseStackLib to > MdePkg.dsc. > > > But it was not building as the FixedPcd was not defined: > > > ------- > > > > > > /home/olivier/tianocore/MdePkg/Library/BaseStackCheckLib/BaseStackChec > > > k > > > Gcc.c > > > :26:34: error: "_PCD_VALUE_PcdBaseStackCanary" undeclared here (not > > in > > > a > > > function) > > > VOID *__stack_chk_guard = (VOID*)FixedPcdGet64 > > > (PcdBaseStackCanary); > > > ------- > > > > > > _PCD_VALUE_* are only generated when a binary module is built. And > > > there is no Module in MdePkg so adding > > > 'NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf' to > > > [LibraryClasses] would not help neither. > > > One solution would be to add > > > 'NULL|MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf' to > > > MdeModulePkg.dsc > > > > > > Which solution would you suggested for your comment 2.? > > > > > > Thanks, > > > Olivier > > > > > > > -----Original Message----- > > > > From: Gao, Liming [mailto:liming.gao@intel.com] > > > > Sent: 06 August 2014 04:00 > > > > To: edk2-devel@lists.sourceforge.net; Kinney, Michael D > > > > Subject: Re: [edk2] [PATCH v3 1/3] MdePkg: Introduced > > > BaseStackCheckLib > > > > > > > > Olivier: > > > > I have two minor comment. > > > > 1. The declaration of __stack_chk_fail() misses the function > > > comments. > > > > 2. Add it into Mdepkg.dsc [Components.ARM, Components.AARCH64] > > > section > > > > to verify its build. > > > > > > > > Other part is good to me. Reviewed-by: Gao, Liming > > > > <liming.gao@intel.com> > > > > > > > > Thanks > > > > Liming > > > > -----Original Message----- > > > > From: Olivier Martin [mailto:olivier.martin@arm.com] > > > > Sent: Tuesday, August 05, 2014 5:48 PM > > > > To: Kinney, Michael D > > > > Cc: edk2-devel@lists.sourceforge.net > > > > Subject: [edk2] [PATCH v3 1/3] MdePkg: Introduced > > > > BaseStackCheckLib > > > > > > > > This library only support GCC, RVCT and XCode for now. > > > > > > > > Contributed-under: TianoCore Contribution Agreement 1.0 > > > > Signed-off-by: Andrew Fish <afish@apple.com> > > > > Signed-off-by: Olivier Martin <olivier.martin@arm.com> > > > > --- > > > > .../Library/BaseStackCheckLib/BaseStackCheckGcc.c | 61 > > > > ++++++++++++++++++++++ > > > > .../BaseStackCheckLib/BaseStackCheckLib.inf | 42 > > > > +++++++++++++++ > > > > MdePkg/MdePkg.dec | 4 ++ > > > > 3 files changed, 107 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..4160aff > > > > --- /dev/null > > > > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c > > > > @@ -0,0 +1,61 @@ > > > > +/** @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*)FixedPcdGet64 > > > > +(PcdBaseStackCanary); > > > > + > > > > +// 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..3304284 > > > > --- /dev/null > > > > +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf > > > > @@ -0,0 +1,42 @@ > > > > +## @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 = NULL > > > > + > > > > + > > > > +# > > > > +# VALID_ARCHITECTURES = ARM AARCH64 > > > > +# > > > > + > > > > +[Sources] > > > > + BaseStackCheckGcc.c | GCC > > > > + BaseStackCheckGcc.c | RVCT > > > > + > > > > +[Packages] > > > > + MdePkg/MdePkg.dec > > > > + > > > > +[LibraryClasses] > > > > + BaseLib > > > > + DebugLib > > > > + > > > > +[FixedPcd] > > > > + gEfiMdePkgTokenSpaceGuid.PcdBaseStackCanary > > > > + gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask > > > > diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index > > > > 4daf3e6..fbb7d2b 100644 > > > > --- a/MdePkg/MdePkg.dec > > > > +++ b/MdePkg/MdePkg.dec > > > > @@ -1544,6 +1544,10 @@ > > > > # The required memory space is decided by the value of > > > > PcdMaximumGuidedExtractHandler. > > > > > > > > > > > > > > gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|0x1000000 > > > | > > > > UINT64|0x30001015 > > > > > > > > + ## Canary value for the stack overflow protection. This PCD > can > > > > + be used by a firmware vendor # or for debugging purposes to > > > > + change > > > the > > > > recommended value. > > > > + > > > > gEfiMdePkgTokenSpaceGuid.PcdBaseStackCanary|0x0AFF|UINT64|0x0000002A > > > > + > > > > [PcdsFixedAtBuild.IPF] > > > > ## The base address of IO port space for IA64 arch > > > > > > > > > > > > > > gEfiMdePkgTokenSpaceGuid.PcdIoBlockBaseAddressForIpf|0x0ffffc000000|UI > > > N > > > > T64|0x0000000f > > > > -- > > > > 1.8.5 > > > > > > > > > > > > ----------------------------------------------------------------- > - > > > > - > > - > > > > - > > > -- > > > > ------- > > > > 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. > > > c > > > > lktrk > > > > _______________________________________________ > > > > edk2-devel mailing list > > > > edk2-devel@lists.sourceforge.net > > > > https://lists.sourceforge.net/lists/listinfo/edk2-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. > > > c > > > > lktrk > > > > _______________________________________________ > > > > edk2-devel mailing list > > > > edk2-devel@lists.sourceforge.net > > > > https://lists.sourceforge.net/lists/listinfo/edk2-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. > > > c > > > lktrk > > > _______________________________________________ > > > edk2-devel mailing list > > > edk2-devel@lists.sourceforge.net > > > https://lists.sourceforge.net/lists/listinfo/edk2-devel > > > > > > > > > > > > ------------------------------------------------------------------------------ Slashdot TV. Video for Nerds. Stuff that matters. http://tv.slashdot.org/
diff --git a/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c new file mode 100644 index 0000000..4160aff --- /dev/null +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckGcc.c @@ -0,0 +1,61 @@ +/** @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*)FixedPcdGet64 (PcdBaseStackCanary); + +// 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..3304284 --- /dev/null +++ b/MdePkg/Library/BaseStackCheckLib/BaseStackCheckLib.inf @@ -0,0 +1,42 @@ +## @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 = NULL + + +# +# VALID_ARCHITECTURES = ARM AARCH64 +# + +[Sources] + BaseStackCheckGcc.c | GCC + BaseStackCheckGcc.c | RVCT + +[Packages] + MdePkg/MdePkg.dec + +[LibraryClasses] + BaseLib + DebugLib + +[FixedPcd] + gEfiMdePkgTokenSpaceGuid.PcdBaseStackCanary + gEfiMdePkgTokenSpaceGuid.PcdDebugPropertyMask diff --git a/MdePkg/MdePkg.dec b/MdePkg/MdePkg.dec index 4daf3e6..fbb7d2b 100644 --- a/MdePkg/MdePkg.dec +++ b/MdePkg/MdePkg.dec @@ -1544,6 +1544,10 @@ # The required memory space is decided by the value of PcdMaximumGuidedExtractHandler. gEfiMdePkgTokenSpaceGuid.PcdGuidedExtractHandlerTableAddress|0x1000000|UINT64|0x30001015 + ## Canary value for the stack overflow protection. This PCD can be used by a firmware vendor + # or for debugging purposes to change the recommended value. + gEfiMdePkgTokenSpaceGuid.PcdBaseStackCanary|0x0AFF|UINT64|0x0000002A + [PcdsFixedAtBuild.IPF] ## The base address of IO port space for IA64 arch gEfiMdePkgTokenSpaceGuid.PcdIoBlockBaseAddressForIpf|0x0ffffc000000|UINT64|0x0000000f