Re: [edk2] [PATCH v3 1/3] MdePkg: Introduced BaseStackCheckLib

Subject: Re: [edk2] [PATCH v3 1/3] MdePkg: Introduced BaseStackCheckLib

From: "Gao, Liming" <liming.gao@intel.com>

To: Olivier Martin <olivier.martin@arm.com>, "edk2-devel@lists.sourceforge.net" <edk2-devel@lists.sourceforge.net>, "Kinney, Michael D" <michael.d.kinney@intel.com>

Date: 2014-08-20 23:23:10

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 
> > 
> >
> > 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 
> > Signed-off-by: Olivier Martin 
> > ---
> >  .../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.
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 > > +#include > > +#include > > +#include > > + > > +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.
# # 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