[Linaro-uefi] Platforms/ARM: enable memory protection features

Message ID 1488885917-10445-1-git-send-email-ard.biesheuvel@linaro.org
State Accepted
Commit 015ac218cfef6d399b05996e4f6072c7cd195861
Headers show

Commit Message

Ard Biesheuvel March 7, 2017, 11:25 a.m.
This enables the recently added and/or enhanced memory protection
features in upstream EDK2:
- strict code/data separation PE/COFF sections so that mappings can
  be made either read-only or non-executable
- remove exec permissions from all other (i.e., non-code) regions (as
  far as is feasible without breaking GRUB)
- remap the DXE stack as non-executable before entering DxeCore

Contributed-under: TianoCore Contribution Agreement 1.0
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
---
 Platforms/ARM/VExpress/ArmVExpress.dsc.inc | 21 ++++++++++++++++++++
 1 file changed, 21 insertions(+)

Comments

Leif Lindholm March 7, 2017, 1:47 p.m. | #1
On Tue, Mar 07, 2017 at 12:25:17PM +0100, Ard Biesheuvel wrote:
> This enables the recently added and/or enhanced memory protection
> features in upstream EDK2:
> - strict code/data separation PE/COFF sections so that mappings can
>   be made either read-only or non-executable
> - remove exec permissions from all other (i.e., non-code) regions (as
>   far as is feasible without breaking GRUB)
> - remap the DXE stack as non-executable before entering DxeCore
> 
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

I'm theoretically quite keen on getting this enabled, but would
definitely need a Tested-by from Ryan before I would want to see it merged.

/
    Leif

> ---
>  Platforms/ARM/VExpress/ArmVExpress.dsc.inc | 21 ++++++++++++++++++++
>  1 file changed, 21 insertions(+)
> 
> diff --git a/Platforms/ARM/VExpress/ArmVExpress.dsc.inc b/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
> index c94001b3bcdb..431d6d0f76ce 100644
> --- a/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
> +++ b/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
> @@ -14,6 +14,9 @@
>  [Defines]
>    SECURE_BOOT_ENABLE  = FALSE
>  
> +[BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
> +  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
> +
>  [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
>    GCC:*_*_ARM_DLINK_FLAGS = -z common-page-size=0x1000
>    GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x10000
> @@ -437,6 +440,24 @@
>    # GUID of the UI app
>    gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
>  
> +  #
> +  # Enable strict image permissions for all images. (This applies
> +  # only to images that were built with >= 4 KB section alignment.)
> +  #
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x3
> +
> +  #
> +  # Enable NX memory protection for all non-code regions, including OEM and OS
> +  # reserved ones, with the exception of LoaderData regions, of which OS loaders
> +  # (i.e., GRUB) may assume that its contents are executable.
> +  #
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
> +
> +  #
> +  # Enable the non-executable DXE stack. (This gets set up by DxeIpl)
> +  #
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
> +
>  [Components.common]
>    MdeModulePkg/Universal/PCD/Dxe/Pcd.inf {
>      <LibraryClasses>
> -- 
> 2.7.4
>
Ard Biesheuvel March 7, 2017, 2:03 p.m. | #2
On 7 March 2017 at 14:47, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Tue, Mar 07, 2017 at 12:25:17PM +0100, Ard Biesheuvel wrote:
>> This enables the recently added and/or enhanced memory protection
>> features in upstream EDK2:
>> - strict code/data separation PE/COFF sections so that mappings can
>>   be made either read-only or non-executable
>> - remove exec permissions from all other (i.e., non-code) regions (as
>>   far as is feasible without breaking GRUB)
>> - remap the DXE stack as non-executable before entering DxeCore
>>
>> Contributed-under: TianoCore Contribution Agreement 1.0
>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>
> I'm theoretically quite keen on getting this enabled, but would
> definitely need a Tested-by from Ryan before I would want to see it merged.
>

... hence the cc :-)
Ryan Harkin March 7, 2017, 3:56 p.m. | #3
On 7 Mar 2017 3:03 p.m., "Ard Biesheuvel" <ard.biesheuvel@linaro.org> wrote:
>

> On 7 March 2017 at 14:47, Leif Lindholm <leif.lindholm@linaro.org> wrote:

> > On Tue, Mar 07, 2017 at 12:25:17PM +0100, Ard Biesheuvel wrote:

> >> This enables the recently added and/or enhanced memory protection

> >> features in upstream EDK2:

> >> - strict code/data separation PE/COFF sections so that mappings can

> >>   be made either read-only or non-executable

> >> - remove exec permissions from all other (i.e., non-code) regions (as

> >>   far as is feasible without breaking GRUB)

> >> - remap the DXE stack as non-executable before entering DxeCore

> >>

> >> Contributed-under: TianoCore Contribution Agreement 1.0

> >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> >

> > I'm theoretically quite keen on getting this enabled, but would

> > definitely need a Tested-by from Ryan before I would want to see it

merged.
> >

>

> ... hence the cc :-)


It'll have to wait till next week as I'm currently on holiday :-)
Ryan Harkin March 16, 2017, 12:04 p.m. | #4
On 7 March 2017 at 11:25, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> This enables the recently added and/or enhanced memory protection
> features in upstream EDK2:
> - strict code/data separation PE/COFF sections so that mappings can
>   be made either read-only or non-executable
> - remove exec permissions from all other (i.e., non-code) regions (as
>   far as is feasible without breaking GRUB)
> - remap the DXE stack as non-executable before entering DxeCore
>
> Contributed-under: TianoCore Contribution Agreement 1.0
> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Tested-by: Ryan Harkin <ryan.harkin@linaro.org>

Tested on FVP Foundation & AEMv8, TC2 and Juno R0/1/2 when combined
with the HEAD of EDK2 (currently 056563f) and OpenPlatformPkg at
c6cdf9e, but with my hack to get TC2 booting [1].


> ---
>  Platforms/ARM/VExpress/ArmVExpress.dsc.inc | 21 ++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>
> diff --git a/Platforms/ARM/VExpress/ArmVExpress.dsc.inc b/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
> index c94001b3bcdb..431d6d0f76ce 100644
> --- a/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
> +++ b/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
> @@ -14,6 +14,9 @@
>  [Defines]
>    SECURE_BOOT_ENABLE  = FALSE
>
> +[BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
> +  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
> +
>  [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
>    GCC:*_*_ARM_DLINK_FLAGS = -z common-page-size=0x1000
>    GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x10000
> @@ -437,6 +440,24 @@
>    # GUID of the UI app
>    gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
>
> +  #
> +  # Enable strict image permissions for all images. (This applies
> +  # only to images that were built with >= 4 KB section alignment.)
> +  #
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x3
> +
> +  #
> +  # Enable NX memory protection for all non-code regions, including OEM and OS
> +  # reserved ones, with the exception of LoaderData regions, of which OS loaders
> +  # (i.e., GRUB) may assume that its contents are executable.
> +  #
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
> +
> +  #
> +  # Enable the non-executable DXE stack. (This gets set up by DxeIpl)
> +  #
> +  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
> +
>  [Components.common]
>    MdeModulePkg/Universal/PCD/Dxe/Pcd.inf {
>      <LibraryClasses>
> --
> 2.7.4
>


[1] https://git.linaro.org/landing-teams/working/arm/OpenPlatformPkg.git/commit/?h=17.03&id=408d6004ea673c04eff6c19b12363fade04330f1
Leif Lindholm March 16, 2017, 1:03 p.m. | #5
On Thu, Mar 16, 2017 at 12:04:50PM +0000, Ryan Harkin wrote:
> On 7 March 2017 at 11:25, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> > This enables the recently added and/or enhanced memory protection
> > features in upstream EDK2:
> > - strict code/data separation PE/COFF sections so that mappings can
> >   be made either read-only or non-executable
> > - remove exec permissions from all other (i.e., non-code) regions (as
> >   far as is feasible without breaking GRUB)
> > - remap the DXE stack as non-executable before entering DxeCore
> >
> > Contributed-under: TianoCore Contribution Agreement 1.0
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> 
> Tested-by: Ryan Harkin <ryan.harkin@linaro.org>
> 
> Tested on FVP Foundation & AEMv8, TC2 and Juno R0/1/2 when combined
> with the HEAD of EDK2 (currently 056563f) and OpenPlatformPkg at
> c6cdf9e, but with my hack to get TC2 booting [1].

I can't really defend to myself keeping that patch out of the public
tree any more. We don't care enough to try to figure the actual issue
out and it's needed for the platform to be of any use.

Want me to just push it?

/
    Leif

> > ---
> >  Platforms/ARM/VExpress/ArmVExpress.dsc.inc | 21 ++++++++++++++++++++
> >  1 file changed, 21 insertions(+)
> >
> > diff --git a/Platforms/ARM/VExpress/ArmVExpress.dsc.inc b/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
> > index c94001b3bcdb..431d6d0f76ce 100644
> > --- a/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
> > +++ b/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
> > @@ -14,6 +14,9 @@
> >  [Defines]
> >    SECURE_BOOT_ENABLE  = FALSE
> >
> > +[BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
> > +  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
> > +
> >  [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
> >    GCC:*_*_ARM_DLINK_FLAGS = -z common-page-size=0x1000
> >    GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x10000
> > @@ -437,6 +440,24 @@
> >    # GUID of the UI app
> >    gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
> >
> > +  #
> > +  # Enable strict image permissions for all images. (This applies
> > +  # only to images that were built with >= 4 KB section alignment.)
> > +  #
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x3
> > +
> > +  #
> > +  # Enable NX memory protection for all non-code regions, including OEM and OS
> > +  # reserved ones, with the exception of LoaderData regions, of which OS loaders
> > +  # (i.e., GRUB) may assume that its contents are executable.
> > +  #
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
> > +
> > +  #
> > +  # Enable the non-executable DXE stack. (This gets set up by DxeIpl)
> > +  #
> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
> > +
> >  [Components.common]
> >    MdeModulePkg/Universal/PCD/Dxe/Pcd.inf {
> >      <LibraryClasses>
> > --
> > 2.7.4
> >
> 
> 
> [1] https://git.linaro.org/landing-teams/working/arm/OpenPlatformPkg.git/commit/?h=17.03&id=408d6004ea673c04eff6c19b12363fade04330f1
Ryan Harkin March 16, 2017, 1:06 p.m. | #6
On 16 March 2017 at 13:03, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Thu, Mar 16, 2017 at 12:04:50PM +0000, Ryan Harkin wrote:
>> On 7 March 2017 at 11:25, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> > This enables the recently added and/or enhanced memory protection
>> > features in upstream EDK2:
>> > - strict code/data separation PE/COFF sections so that mappings can
>> >   be made either read-only or non-executable
>> > - remove exec permissions from all other (i.e., non-code) regions (as
>> >   far as is feasible without breaking GRUB)
>> > - remap the DXE stack as non-executable before entering DxeCore
>> >
>> > Contributed-under: TianoCore Contribution Agreement 1.0
>> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>>
>> Tested-by: Ryan Harkin <ryan.harkin@linaro.org>
>>
>> Tested on FVP Foundation & AEMv8, TC2 and Juno R0/1/2 when combined
>> with the HEAD of EDK2 (currently 056563f) and OpenPlatformPkg at
>> c6cdf9e, but with my hack to get TC2 booting [1].
>
> I can't really defend to myself keeping that patch out of the public
> tree any more. We don't care enough to try to figure the actual issue
> out and it's needed for the platform to be of any use.
>
> Want me to just push it?
>

Yes please :-)

If you could be so kind as to remove the "HACK" prefix from the title,
that would be much appreciated. Although, it is still a hack...


> /
>     Leif
>
>> > ---
>> >  Platforms/ARM/VExpress/ArmVExpress.dsc.inc | 21 ++++++++++++++++++++
>> >  1 file changed, 21 insertions(+)
>> >
>> > diff --git a/Platforms/ARM/VExpress/ArmVExpress.dsc.inc b/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
>> > index c94001b3bcdb..431d6d0f76ce 100644
>> > --- a/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
>> > +++ b/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
>> > @@ -14,6 +14,9 @@
>> >  [Defines]
>> >    SECURE_BOOT_ENABLE  = FALSE
>> >
>> > +[BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
>> > +  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
>> > +
>> >  [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
>> >    GCC:*_*_ARM_DLINK_FLAGS = -z common-page-size=0x1000
>> >    GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x10000
>> > @@ -437,6 +440,24 @@
>> >    # GUID of the UI app
>> >    gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
>> >
>> > +  #
>> > +  # Enable strict image permissions for all images. (This applies
>> > +  # only to images that were built with >= 4 KB section alignment.)
>> > +  #
>> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x3
>> > +
>> > +  #
>> > +  # Enable NX memory protection for all non-code regions, including OEM and OS
>> > +  # reserved ones, with the exception of LoaderData regions, of which OS loaders
>> > +  # (i.e., GRUB) may assume that its contents are executable.
>> > +  #
>> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
>> > +
>> > +  #
>> > +  # Enable the non-executable DXE stack. (This gets set up by DxeIpl)
>> > +  #
>> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
>> > +
>> >  [Components.common]
>> >    MdeModulePkg/Universal/PCD/Dxe/Pcd.inf {
>> >      <LibraryClasses>
>> > --
>> > 2.7.4
>> >
>>
>>
>> [1] https://git.linaro.org/landing-teams/working/arm/OpenPlatformPkg.git/commit/?h=17.03&id=408d6004ea673c04eff6c19b12363fade04330f1
Leif Lindholm March 16, 2017, 1:12 p.m. | #7
On Thu, Mar 16, 2017 at 01:06:14PM +0000, Ryan Harkin wrote:
> On 16 March 2017 at 13:03, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> > On Thu, Mar 16, 2017 at 12:04:50PM +0000, Ryan Harkin wrote:
> >> On 7 March 2017 at 11:25, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> >> > This enables the recently added and/or enhanced memory protection
> >> > features in upstream EDK2:
> >> > - strict code/data separation PE/COFF sections so that mappings can
> >> >   be made either read-only or non-executable
> >> > - remove exec permissions from all other (i.e., non-code) regions (as
> >> >   far as is feasible without breaking GRUB)
> >> > - remap the DXE stack as non-executable before entering DxeCore
> >> >
> >> > Contributed-under: TianoCore Contribution Agreement 1.0
> >> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
> >>
> >> Tested-by: Ryan Harkin <ryan.harkin@linaro.org>
> >>
> >> Tested on FVP Foundation & AEMv8, TC2 and Juno R0/1/2 when combined
> >> with the HEAD of EDK2 (currently 056563f) and OpenPlatformPkg at
> >> c6cdf9e, but with my hack to get TC2 booting [1].
> >
> > I can't really defend to myself keeping that patch out of the public
> > tree any more. We don't care enough to try to figure the actual issue
> > out and it's needed for the platform to be of any use.
> >
> > Want me to just push it?
> >
> 
> Yes please :-)
> 
> If you could be so kind as to remove the "HACK" prefix from the title,
> that would be much appreciated. Although, it is still a hack...

Done. (And shortened the remainder of the subject slightly.)

I kept the rest of the commit message as-is, in violation of
guidelines, because it's archeologically useful (and includes indented
copies of other commit messages, which I wouldn't want to reflow).

/
    Leif

> 
> > /
> >     Leif
> >
> >> > ---
> >> >  Platforms/ARM/VExpress/ArmVExpress.dsc.inc | 21 ++++++++++++++++++++
> >> >  1 file changed, 21 insertions(+)
> >> >
> >> > diff --git a/Platforms/ARM/VExpress/ArmVExpress.dsc.inc b/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
> >> > index c94001b3bcdb..431d6d0f76ce 100644
> >> > --- a/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
> >> > +++ b/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
> >> > @@ -14,6 +14,9 @@
> >> >  [Defines]
> >> >    SECURE_BOOT_ENABLE  = FALSE
> >> >
> >> > +[BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
> >> > +  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
> >> > +
> >> >  [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
> >> >    GCC:*_*_ARM_DLINK_FLAGS = -z common-page-size=0x1000
> >> >    GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x10000
> >> > @@ -437,6 +440,24 @@
> >> >    # GUID of the UI app
> >> >    gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
> >> >
> >> > +  #
> >> > +  # Enable strict image permissions for all images. (This applies
> >> > +  # only to images that were built with >= 4 KB section alignment.)
> >> > +  #
> >> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x3
> >> > +
> >> > +  #
> >> > +  # Enable NX memory protection for all non-code regions, including OEM and OS
> >> > +  # reserved ones, with the exception of LoaderData regions, of which OS loaders
> >> > +  # (i.e., GRUB) may assume that its contents are executable.
> >> > +  #
> >> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
> >> > +
> >> > +  #
> >> > +  # Enable the non-executable DXE stack. (This gets set up by DxeIpl)
> >> > +  #
> >> > +  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
> >> > +
> >> >  [Components.common]
> >> >    MdeModulePkg/Universal/PCD/Dxe/Pcd.inf {
> >> >      <LibraryClasses>
> >> > --
> >> > 2.7.4
> >> >
> >>
> >>
> >> [1] https://git.linaro.org/landing-teams/working/arm/OpenPlatformPkg.git/commit/?h=17.03&id=408d6004ea673c04eff6c19b12363fade04330f1
Ard Biesheuvel March 16, 2017, 1:43 p.m. | #8
On 16 March 2017 at 13:12, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Thu, Mar 16, 2017 at 01:06:14PM +0000, Ryan Harkin wrote:
>> On 16 March 2017 at 13:03, Leif Lindholm <leif.lindholm@linaro.org> wrote:
>> > On Thu, Mar 16, 2017 at 12:04:50PM +0000, Ryan Harkin wrote:
>> >> On 7 March 2017 at 11:25, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
>> >> > This enables the recently added and/or enhanced memory protection
>> >> > features in upstream EDK2:
>> >> > - strict code/data separation PE/COFF sections so that mappings can
>> >> >   be made either read-only or non-executable
>> >> > - remove exec permissions from all other (i.e., non-code) regions (as
>> >> >   far as is feasible without breaking GRUB)
>> >> > - remap the DXE stack as non-executable before entering DxeCore
>> >> >
>> >> > Contributed-under: TianoCore Contribution Agreement 1.0
>> >> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>
>> >>
>> >> Tested-by: Ryan Harkin <ryan.harkin@linaro.org>
>> >>
>> >> Tested on FVP Foundation & AEMv8, TC2 and Juno R0/1/2 when combined
>> >> with the HEAD of EDK2 (currently 056563f) and OpenPlatformPkg at
>> >> c6cdf9e, but with my hack to get TC2 booting [1].
>> >
>> > I can't really defend to myself keeping that patch out of the public
>> > tree any more. We don't care enough to try to figure the actual issue
>> > out and it's needed for the platform to be of any use.
>> >
>> > Want me to just push it?
>> >
>>
>> Yes please :-)
>>
>> If you could be so kind as to remove the "HACK" prefix from the title,
>> that would be much appreciated. Although, it is still a hack...
>
> Done. (And shortened the remainder of the subject slightly.)
>
> I kept the rest of the commit message as-is, in violation of
> guidelines, because it's archeologically useful (and includes indented
> copies of other commit messages, which I wouldn't want to reflow).
>

Back on $subject: are you ok with this patch now that Ryan has
confirmed it does not break anything?
Leif Lindholm March 16, 2017, 1:47 p.m. | #9
On 16 March 2017 at 13:43, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> Back on $subject: are you ok with this patch now that Ryan has
> confirmed it does not break anything?

Yes, sorry. My initial comment was meant as an implicit "Reviewed-by:
with a Tested-by: from Ryan".

Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>

/
    Leif

Patch

diff --git a/Platforms/ARM/VExpress/ArmVExpress.dsc.inc b/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
index c94001b3bcdb..431d6d0f76ce 100644
--- a/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
+++ b/Platforms/ARM/VExpress/ArmVExpress.dsc.inc
@@ -14,6 +14,9 @@ 
 [Defines]
   SECURE_BOOT_ENABLE  = FALSE
 
+[BuildOptions.common.EDKII.DXE_CORE,BuildOptions.common.EDKII.DXE_DRIVER,BuildOptions.common.EDKII.UEFI_DRIVER,BuildOptions.common.EDKII.UEFI_APPLICATION]
+  GCC:*_*_*_DLINK_FLAGS = -z common-page-size=0x1000
+
 [BuildOptions.common.EDKII.DXE_RUNTIME_DRIVER]
   GCC:*_*_ARM_DLINK_FLAGS = -z common-page-size=0x1000
   GCC:*_*_AARCH64_DLINK_FLAGS = -z common-page-size=0x10000
@@ -437,6 +440,24 @@ 
   # GUID of the UI app
   gEfiMdeModulePkgTokenSpaceGuid.PcdBootManagerMenuFile|{ 0x21, 0xaa, 0x2c, 0x46, 0x14, 0x76, 0x03, 0x45, 0x83, 0x6e, 0x8a, 0xb6, 0xf4, 0x66, 0x23, 0x31 }
 
+  #
+  # Enable strict image permissions for all images. (This applies
+  # only to images that were built with >= 4 KB section alignment.)
+  #
+  gEfiMdeModulePkgTokenSpaceGuid.PcdImageProtectionPolicy|0x3
+
+  #
+  # Enable NX memory protection for all non-code regions, including OEM and OS
+  # reserved ones, with the exception of LoaderData regions, of which OS loaders
+  # (i.e., GRUB) may assume that its contents are executable.
+  #
+  gEfiMdeModulePkgTokenSpaceGuid.PcdDxeNxMemoryProtectionPolicy|0xC000000000007FD1
+
+  #
+  # Enable the non-executable DXE stack. (This gets set up by DxeIpl)
+  #
+  gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE
+
 [Components.common]
   MdeModulePkg/Universal/PCD/Dxe/Pcd.inf {
     <LibraryClasses>