diff mbox series

[edk2,platforms:,05/10] Marvell/Library: MppLib: Disable the stack protector

Message ID 1508980777-29006-6-git-send-email-mw@semihalf.com
State Superseded
Headers show
Series None | expand

Commit Message

Marcin Wojtas Oct. 26, 2017, 1:19 a.m. UTC
From: Ard Biesheuvel <ard.biesheuvel@linaro.org>


MppLib may be used very early (in SEC), at which point stack protection
measures are more likely to cause harm than help, given that not even
the UART has been configured to the point where we can complain usefully.
So just disable it.

Contributed-under: TianoCore Contribution Agreement 1.1
Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

Signed-off-by: Marcin Wojtas <mw@semihalf.com>

---
 Platform/Marvell/Library/MppLib/MppLib.inf | 3 +++
 1 file changed, 3 insertions(+)

-- 
2.7.4

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel

Comments

Leif Lindholm Oct. 26, 2017, 1:26 p.m. UTC | #1
On Thu, Oct 26, 2017 at 03:19:32AM +0200, Marcin Wojtas wrote:
> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> 

> MppLib may be used very early (in SEC), at which point stack protection

> measures are more likely to cause harm than help, given that not even

> the UART has been configured to the point where we can complain usefully.

> So just disable it.


It may. But it is also used by PlatInitDxe.
Can we use different build options for SEC and later phases?

/
    Leif

> Contributed-under: TianoCore Contribution Agreement 1.1

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

> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

> ---

>  Platform/Marvell/Library/MppLib/MppLib.inf | 3 +++

>  1 file changed, 3 insertions(+)

> 

> diff --git a/Platform/Marvell/Library/MppLib/MppLib.inf b/Platform/Marvell/Library/MppLib/MppLib.inf

> index 2de9cd0..1268542 100644

> --- a/Platform/Marvell/Library/MppLib/MppLib.inf

> +++ b/Platform/Marvell/Library/MppLib/MppLib.inf

> @@ -106,3 +106,6 @@

>    gMarvellTokenSpaceGuid.PcdChip3MppSel7

>  

>    gMarvellTokenSpaceGuid.PcdPciESdhci

> +

> +[BuildOptions]

> +  *_*_*_CC_FLAGS = -fno-stack-protector

> -- 

> 2.7.4

> 

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Ard Biesheuvel Oct. 26, 2017, 1:29 p.m. UTC | #2
On 26 October 2017 at 14:26, Leif Lindholm <leif.lindholm@linaro.org> wrote:
> On Thu, Oct 26, 2017 at 03:19:32AM +0200, Marcin Wojtas wrote:

>> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

>>

>> MppLib may be used very early (in SEC), at which point stack protection

>> measures are more likely to cause harm than help, given that not even

>> the UART has been configured to the point where we can complain usefully.

>> So just disable it.

>

> It may. But it is also used by PlatInitDxe.

> Can we use different build options for SEC and later phases?

>


No, libraries are only built a single time during the build, and
linked into every module that depends on them. This is the same issue
we had with -mstrict-align.


>

>> Contributed-under: TianoCore Contribution Agreement 1.1

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

>> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

>> ---

>>  Platform/Marvell/Library/MppLib/MppLib.inf | 3 +++

>>  1 file changed, 3 insertions(+)

>>

>> diff --git a/Platform/Marvell/Library/MppLib/MppLib.inf b/Platform/Marvell/Library/MppLib/MppLib.inf

>> index 2de9cd0..1268542 100644

>> --- a/Platform/Marvell/Library/MppLib/MppLib.inf

>> +++ b/Platform/Marvell/Library/MppLib/MppLib.inf

>> @@ -106,3 +106,6 @@

>>    gMarvellTokenSpaceGuid.PcdChip3MppSel7

>>

>>    gMarvellTokenSpaceGuid.PcdPciESdhci

>> +

>> +[BuildOptions]

>> +  *_*_*_CC_FLAGS = -fno-stack-protector

>> --

>> 2.7.4

>>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
Leif Lindholm Oct. 26, 2017, 1:57 p.m. UTC | #3
On Thu, Oct 26, 2017 at 02:29:02PM +0100, Ard Biesheuvel wrote:
> On 26 October 2017 at 14:26, Leif Lindholm <leif.lindholm@linaro.org> wrote:

> > On Thu, Oct 26, 2017 at 03:19:32AM +0200, Marcin Wojtas wrote:

> >> From: Ard Biesheuvel <ard.biesheuvel@linaro.org>

> >>

> >> MppLib may be used very early (in SEC), at which point stack protection

> >> measures are more likely to cause harm than help, given that not even

> >> the UART has been configured to the point where we can complain usefully.

> >> So just disable it.

> >

> > It may. But it is also used by PlatInitDxe.

> > Can we use different build options for SEC and later phases?

> 

> No, libraries are only built a single time during the build, and

> linked into every module that depends on them. This is the same issue

> we had with -mstrict-align.


Sure, but we could have duplicated the .inf and have a SEC version and
use that mapping for SEC...

Clearly that's tedious. I guess it comes down to how useful we think
the stack checking is in general. If the answer is "not very, to be
honest":
Reviewed-by: Leif Lindholm <leif.lindholm@linaro.org>


/
    Leif

> >> Contributed-under: TianoCore Contribution Agreement 1.1

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

> >> Signed-off-by: Marcin Wojtas <mw@semihalf.com>

> >> ---

> >>  Platform/Marvell/Library/MppLib/MppLib.inf | 3 +++

> >>  1 file changed, 3 insertions(+)

> >>

> >> diff --git a/Platform/Marvell/Library/MppLib/MppLib.inf b/Platform/Marvell/Library/MppLib/MppLib.inf

> >> index 2de9cd0..1268542 100644

> >> --- a/Platform/Marvell/Library/MppLib/MppLib.inf

> >> +++ b/Platform/Marvell/Library/MppLib/MppLib.inf

> >> @@ -106,3 +106,6 @@

> >>    gMarvellTokenSpaceGuid.PcdChip3MppSel7

> >>

> >>    gMarvellTokenSpaceGuid.PcdPciESdhci

> >> +

> >> +[BuildOptions]

> >> +  *_*_*_CC_FLAGS = -fno-stack-protector

> >> --

> >> 2.7.4

> >>

_______________________________________________
edk2-devel mailing list
edk2-devel@lists.01.org
https://lists.01.org/mailman/listinfo/edk2-devel
diff mbox series

Patch

diff --git a/Platform/Marvell/Library/MppLib/MppLib.inf b/Platform/Marvell/Library/MppLib/MppLib.inf
index 2de9cd0..1268542 100644
--- a/Platform/Marvell/Library/MppLib/MppLib.inf
+++ b/Platform/Marvell/Library/MppLib/MppLib.inf
@@ -106,3 +106,6 @@ 
   gMarvellTokenSpaceGuid.PcdChip3MppSel7
 
   gMarvellTokenSpaceGuid.PcdPciESdhci
+
+[BuildOptions]
+  *_*_*_CC_FLAGS = -fno-stack-protector