| Message ID | 1481053337-13319-1-git-send-email-ard.biesheuvel@linaro.org |
|---|---|
| State | Accepted |
| Commit | 30a70640879494d0b975d37c2ceaf10e17f2a32c |
| Headers | show |
On Tue, Dec 06, 2016 at 07:42:17PM +0000, Ard Biesheuvel wrote: > Map the DXE stack as non-executable, to prevent stack buffer overflows > from being exploitable. > > Contributed-under: TianoCore Contribution Agreement 1.0 > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> Any particular reason you're only doing this for the Styx platforms? Anyway: Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org> > --- > Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc | 3 +++ > Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc | 3 +++ > Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc | 3 +++ > 3 files changed, 9 insertions(+) > > diff --git a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc > index f833fe200422..0f299c388d00 100644 > --- a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc > +++ b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc > @@ -439,6 +439,9 @@ DEFINE DO_KCS = 0 > gAmdModulePkgTokenSpaceGuid.PcdSataSerdesBase|0xE1200000 > gAmdModulePkgTokenSpaceGuid.PcdSataSerdesOffset|0x00010000 > > + # map the stack as non-executable when entering the DXE phase > + gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE > + > [PcdsPatchableInModule] > # PCIe Configuration: x4x2x2 > gAmdModulePkgTokenSpaceGuid.PcdPcieCoreConfiguration|2 > diff --git a/Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc b/Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc > index 107205386c55..0d630fba1ca9 100644 > --- a/Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc > +++ b/Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc > @@ -461,6 +461,9 @@ DEFINE DO_KCS = 1 > gAmdModulePkgTokenSpaceGuid.PcdSataSerdesBase|0xE1200000 > gAmdModulePkgTokenSpaceGuid.PcdSataSerdesOffset|0x00010000 > > + # map the stack as non-executable when entering the DXE phase > + gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE > + > [PcdsPatchableInModule] > # PCIe Configuration: x4x2x2 (=2 See Include/FDKGionb.h) > gAmdModulePkgTokenSpaceGuid.PcdPcieCoreConfiguration|2 > diff --git a/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc b/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc > index 92721064a51f..944cee3d8536 100644 > --- a/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc > +++ b/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc > @@ -458,6 +458,9 @@ DEFINE DO_KCS = 1 > gAmdModulePkgTokenSpaceGuid.PcdSataSerdesBase|0xE1200000 > gAmdModulePkgTokenSpaceGuid.PcdSataSerdesOffset|0x00010000 > > + # map the stack as non-executable when entering the DXE phase > + gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE > + > !if $(DO_XGBE) > gAmdModulePkgTokenSpaceGuid.PcdXgbeEnable|TRUE > > -- > 2.7.4 >
On 7 December 2016 at 08:54, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Tue, Dec 06, 2016 at 07:42:17PM +0000, Ard Biesheuvel wrote: >> Map the DXE stack as non-executable, to prevent stack buffer overflows >> from being exploitable. >> >> Contributed-under: TianoCore Contribution Agreement 1.0 >> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > > Any particular reason you're only doing this for the Styx platforms? > Those are the only ones I can test > Anyway: > Signed-off-by: Leif Lindholm <leif.lindholm@linaro.org> > Thanks >> --- >> Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc | 3 +++ >> Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc | 3 +++ >> Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc | 3 +++ >> 3 files changed, 9 insertions(+) >> >> diff --git a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc >> index f833fe200422..0f299c388d00 100644 >> --- a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc >> +++ b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc >> @@ -439,6 +439,9 @@ DEFINE DO_KCS = 0 >> gAmdModulePkgTokenSpaceGuid.PcdSataSerdesBase|0xE1200000 >> gAmdModulePkgTokenSpaceGuid.PcdSataSerdesOffset|0x00010000 >> >> + # map the stack as non-executable when entering the DXE phase >> + gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE >> + >> [PcdsPatchableInModule] >> # PCIe Configuration: x4x2x2 >> gAmdModulePkgTokenSpaceGuid.PcdPcieCoreConfiguration|2 >> diff --git a/Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc b/Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc >> index 107205386c55..0d630fba1ca9 100644 >> --- a/Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc >> +++ b/Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc >> @@ -461,6 +461,9 @@ DEFINE DO_KCS = 1 >> gAmdModulePkgTokenSpaceGuid.PcdSataSerdesBase|0xE1200000 >> gAmdModulePkgTokenSpaceGuid.PcdSataSerdesOffset|0x00010000 >> >> + # map the stack as non-executable when entering the DXE phase >> + gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE >> + >> [PcdsPatchableInModule] >> # PCIe Configuration: x4x2x2 (=2 See Include/FDKGionb.h) >> gAmdModulePkgTokenSpaceGuid.PcdPcieCoreConfiguration|2 >> diff --git a/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc b/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc >> index 92721064a51f..944cee3d8536 100644 >> --- a/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc >> +++ b/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc >> @@ -458,6 +458,9 @@ DEFINE DO_KCS = 1 >> gAmdModulePkgTokenSpaceGuid.PcdSataSerdesBase|0xE1200000 >> gAmdModulePkgTokenSpaceGuid.PcdSataSerdesOffset|0x00010000 >> >> + # map the stack as non-executable when entering the DXE phase >> + gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE >> + >> !if $(DO_XGBE) >> gAmdModulePkgTokenSpaceGuid.PcdXgbeEnable|TRUE >> >> -- >> 2.7.4 >>
On 7 December 2016 at 09:00, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote: > On 7 December 2016 at 08:54, Leif Lindholm <leif.lindholm@linaro.org> wrote: >> On Tue, Dec 06, 2016 at 07:42:17PM +0000, Ard Biesheuvel wrote: >>> Map the DXE stack as non-executable, to prevent stack buffer overflows >>> from being exploitable. >>> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >> Any particular reason you're only doing this for the Styx platforms? >> > > Those are the only ones I can test > To elaborate: mapping the stack as executable involves the MMU page table splitting code, which could trigger subtle issues involving TLB conflicts. Of course, we'd like to know about those asap, but blindly enabling it for all platforms seems risky.
On Thu, Dec 08, 2016 at 09:30:26AM +0000, Ard Biesheuvel wrote: > >>> Map the DXE stack as non-executable, to prevent stack buffer overflows > >>> from being exploitable. > >>> > >>> Contributed-under: TianoCore Contribution Agreement 1.0 > >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> > >> > >> Any particular reason you're only doing this for the Styx platforms? > > > > Those are the only ones I can test > > To elaborate: mapping the stack as executable involves the MMU page > table splitting code, which could trigger subtle issues involving TLB > conflicts. > Of course, we'd like to know about those asap, but blindly enabling it > for all platforms seems risky. Sure - but I guess something we would like to progressively migrate towards? / Leif
On 8 December 2016 at 16:23, Leif Lindholm <leif.lindholm@linaro.org> wrote: > On Thu, Dec 08, 2016 at 09:30:26AM +0000, Ard Biesheuvel wrote: >> >>> Map the DXE stack as non-executable, to prevent stack buffer overflows >> >>> from being exploitable. >> >>> >> >>> Contributed-under: TianoCore Contribution Agreement 1.0 >> >>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> >> >> >> >> Any particular reason you're only doing this for the Styx platforms? >> > >> > Those are the only ones I can test >> >> To elaborate: mapping the stack as executable involves the MMU page >> table splitting code, which could trigger subtle issues involving TLB >> conflicts. >> Of course, we'd like to know about those asap, but blindly enabling it >> for all platforms seems risky. > > Sure - but I guess something we would like to progressively migrate > towards? > Yes, anything that improves robustness should be enabled where we can, especially given how pathetic EDK2/Tianocore/UEFI are in this respect.
diff --git a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc index f833fe200422..0f299c388d00 100644 --- a/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc +++ b/Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc @@ -439,6 +439,9 @@ DEFINE DO_KCS = 0 gAmdModulePkgTokenSpaceGuid.PcdSataSerdesBase|0xE1200000 gAmdModulePkgTokenSpaceGuid.PcdSataSerdesOffset|0x00010000 + # map the stack as non-executable when entering the DXE phase + gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE + [PcdsPatchableInModule] # PCIe Configuration: x4x2x2 gAmdModulePkgTokenSpaceGuid.PcdPcieCoreConfiguration|2 diff --git a/Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc b/Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc index 107205386c55..0d630fba1ca9 100644 --- a/Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc +++ b/Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc @@ -461,6 +461,9 @@ DEFINE DO_KCS = 1 gAmdModulePkgTokenSpaceGuid.PcdSataSerdesBase|0xE1200000 gAmdModulePkgTokenSpaceGuid.PcdSataSerdesOffset|0x00010000 + # map the stack as non-executable when entering the DXE phase + gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE + [PcdsPatchableInModule] # PCIe Configuration: x4x2x2 (=2 See Include/FDKGionb.h) gAmdModulePkgTokenSpaceGuid.PcdPcieCoreConfiguration|2 diff --git a/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc b/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc index 92721064a51f..944cee3d8536 100644 --- a/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc +++ b/Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc @@ -458,6 +458,9 @@ DEFINE DO_KCS = 1 gAmdModulePkgTokenSpaceGuid.PcdSataSerdesBase|0xE1200000 gAmdModulePkgTokenSpaceGuid.PcdSataSerdesOffset|0x00010000 + # map the stack as non-executable when entering the DXE phase + gEfiMdeModulePkgTokenSpaceGuid.PcdSetNxForStack|TRUE + !if $(DO_XGBE) gAmdModulePkgTokenSpaceGuid.PcdXgbeEnable|TRUE
Map the DXE stack as non-executable, to prevent stack buffer overflows from being exploitable. Contributed-under: TianoCore Contribution Agreement 1.0 Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org> --- Platforms/AMD/Styx/CelloBoard/CelloBoard.dsc | 3 +++ Platforms/AMD/Styx/Overdrive1000Board/Overdrive1000Board.dsc | 3 +++ Platforms/AMD/Styx/OverdriveBoard/OverdriveBoard.dsc | 3 +++ 3 files changed, 9 insertions(+)