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

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

From: "Olivier Martin" <olivier.martin@arm.com>

To: <edk2-devel@lists.sourceforge.net>, "Kinney, Michael D" <michael.d.kinney@intel.com>, "'Gao, Liming'" <liming.gao@intel.com>

Date: 2014-08-06 23:49:11

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