Any feedback? Thanks, Olivier > -----Original Message----- > From: Olivier Martin [mailto:olivier.mar...@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....@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....@intel.com> > > > > Thanks > > Liming > > -----Original Message----- > > From: Olivier Martin [mailto:olivier.mar...@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 <af...@apple.com> > > Signed-off-by: Olivier Martin <olivier.mar...@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/ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
