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....@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.mar...@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.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/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....@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|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/ _______________________________________________ edk2-devel mailing list edk2-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/edk2-devel
