All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/1] boot/arm-trusted-firmware: Forward stack protection configuration
@ 2020-11-20 21:29 Christoph Müllner
  2020-11-21 16:59 ` Baruch Siach
  0 siblings, 1 reply; 6+ messages in thread
From: Christoph Müllner @ 2020-11-20 21:29 UTC (permalink / raw)
  To: buildroot

TF-A supports stack smashing protection (-fstack-protector-*).
Since we already forward the required compiler flag, let's
also tell TF-A that we actually want the required symbols
(e.g. __stack_chk_guard) to be available.

Signed-off-by: Christoph M?llner <christoph.muellner@theobroma-systems.com>
---
 boot/arm-trusted-firmware/arm-trusted-firmware.mk | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/boot/arm-trusted-firmware/arm-trusted-firmware.mk b/boot/arm-trusted-firmware/arm-trusted-firmware.mk
index a3553e36cf..f5be39100e 100644
--- a/boot/arm-trusted-firmware/arm-trusted-firmware.mk
+++ b/boot/arm-trusted-firmware/arm-trusted-firmware.mk
@@ -100,6 +100,18 @@ ARM_TRUSTED_FIRMWARE_MAKE_OPTS += MV_DDR_PATH=$(MV_DDR_MARVELL_DIR)
 ARM_TRUSTED_FIRMWARE_DEPENDENCIES += mv-ddr-marvell
 endif
 
+ifeq ($(BR2_SSP_REGULAR),y)
+ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ENABLE_STACK_PROTECTOR=default
+endif
+
+ifeq ($(BR2_SSP_STRONG),y)
+ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ENABLE_STACK_PROTECTOR=strong
+endif
+
+ifeq ($(BR2_SSP_ALL),y)
+ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ENABLE_STACK_PROTECTOR=all
+endif
+
 ARM_TRUSTED_FIRMWARE_MAKE_TARGETS = all
 
 ifeq ($(BR2_TARGET_ARM_TRUSTED_FIRMWARE_FIP),y)
-- 
2.28.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH 1/1] boot/arm-trusted-firmware: Forward stack protection configuration
  2020-11-20 21:29 [Buildroot] [PATCH 1/1] boot/arm-trusted-firmware: Forward stack protection configuration Christoph Müllner
@ 2020-11-21 16:59 ` Baruch Siach
  2020-11-21 17:28   ` Christoph Müllner
  0 siblings, 1 reply; 6+ messages in thread
From: Baruch Siach @ 2020-11-21 16:59 UTC (permalink / raw)
  To: buildroot

Hi Christoph,

On Fri, Nov 20 2020, Christoph M?llner wrote:
> TF-A supports stack smashing protection (-fstack-protector-*).
> Since we already forward the required compiler flag, let's
> also tell TF-A that we actually want the required symbols
> (e.g. __stack_chk_guard) to be available.
>
> Signed-off-by: Christoph M?llner <christoph.muellner@theobroma-systems.com>
> ---
>  boot/arm-trusted-firmware/arm-trusted-firmware.mk | 12 ++++++++++++
>  1 file changed, 12 insertions(+)
>
> diff --git a/boot/arm-trusted-firmware/arm-trusted-firmware.mk b/boot/arm-trusted-firmware/arm-trusted-firmware.mk
> index a3553e36cf..f5be39100e 100644
> --- a/boot/arm-trusted-firmware/arm-trusted-firmware.mk
> +++ b/boot/arm-trusted-firmware/arm-trusted-firmware.mk
> @@ -100,6 +100,18 @@ ARM_TRUSTED_FIRMWARE_MAKE_OPTS += MV_DDR_PATH=$(MV_DDR_MARVELL_DIR)
>  ARM_TRUSTED_FIRMWARE_DEPENDENCIES += mv-ddr-marvell
>  endif
>  
> +ifeq ($(BR2_SSP_REGULAR),y)
> +ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ENABLE_STACK_PROTECTOR=default
> +endif
> +
> +ifeq ($(BR2_SSP_STRONG),y)
> +ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ENABLE_STACK_PROTECTOR=strong
> +endif
> +
> +ifeq ($(BR2_SSP_ALL),y)
> +ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ENABLE_STACK_PROTECTOR=all
> +endif

It looks like the toolchain wrapper does that already. See
toolchain/toolchain-wrapper.mk. Are you sure this is needed?

baruch

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH 1/1] boot/arm-trusted-firmware: Forward stack protection configuration
  2020-11-21 16:59 ` Baruch Siach
@ 2020-11-21 17:28   ` Christoph Müllner
  2020-11-21 17:49     ` Baruch Siach
  0 siblings, 1 reply; 6+ messages in thread
From: Christoph Müllner @ 2020-11-21 17:28 UTC (permalink / raw)
  To: buildroot

On 11/21/20 5:59 PM, Baruch Siach wrote:
> Hi Christoph,
> 
> On Fri, Nov 20 2020, Christoph M?llner wrote:
>> TF-A supports stack smashing protection (-fstack-protector-*).
>> Since we already forward the required compiler flag, let's
>> also tell TF-A that we actually want the required symbols
>> (e.g. __stack_chk_guard) to be available.
>>
>> Signed-off-by: Christoph M?llner <christoph.muellner@theobroma-systems.com>
>> ---
>>  boot/arm-trusted-firmware/arm-trusted-firmware.mk | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/boot/arm-trusted-firmware/arm-trusted-firmware.mk b/boot/arm-trusted-firmware/arm-trusted-firmware.mk
>> index a3553e36cf..f5be39100e 100644
>> --- a/boot/arm-trusted-firmware/arm-trusted-firmware.mk
>> +++ b/boot/arm-trusted-firmware/arm-trusted-firmware.mk
>> @@ -100,6 +100,18 @@ ARM_TRUSTED_FIRMWARE_MAKE_OPTS += MV_DDR_PATH=$(MV_DDR_MARVELL_DIR)
>>  ARM_TRUSTED_FIRMWARE_DEPENDENCIES += mv-ddr-marvell
>>  endif
>>  
>> +ifeq ($(BR2_SSP_REGULAR),y)
>> +ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ENABLE_STACK_PROTECTOR=default
>> +endif
>> +
>> +ifeq ($(BR2_SSP_STRONG),y)
>> +ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ENABLE_STACK_PROTECTOR=strong
>> +endif
>> +
>> +ifeq ($(BR2_SSP_ALL),y)
>> +ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ENABLE_STACK_PROTECTOR=all
>> +endif
> 
> It looks like the toolchain wrapper does that already. See
> toolchain/toolchain-wrapper.mk. Are you sure this is needed?

Hi Baruch,

I don't see this functionality there.

Note, that you need two things for getting the stack protector working:

1) Toolchain to emit additional code requiring external symbols
2) Runtime that provides the required external symbols

The toolchain wrapper correctly addresses 1) by providing "-fstack-protector*" flags
and your libc will then provide the required symbols for 2).
However, TF-A does not link against libc and requires these build flags
(ENABLE_STACK_PROTECTOR=x) in order to emit the required symbols.

BR
Christoph

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH 1/1] boot/arm-trusted-firmware: Forward stack protection configuration
  2020-11-21 17:28   ` Christoph Müllner
@ 2020-11-21 17:49     ` Baruch Siach
  0 siblings, 0 replies; 6+ messages in thread
From: Baruch Siach @ 2020-11-21 17:49 UTC (permalink / raw)
  To: buildroot

Hi Christoph,

On Sat, Nov 21 2020, Christoph M?llner wrote:
> On 11/21/20 5:59 PM, Baruch Siach wrote:
>> On Fri, Nov 20 2020, Christoph M?llner wrote:
>>> TF-A supports stack smashing protection (-fstack-protector-*).
>>> Since we already forward the required compiler flag, let's
>>> also tell TF-A that we actually want the required symbols
>>> (e.g. __stack_chk_guard) to be available.
>>>
>>> Signed-off-by: Christoph M?llner <christoph.muellner@theobroma-systems.com>
>>> ---
>>>  boot/arm-trusted-firmware/arm-trusted-firmware.mk | 12 ++++++++++++
>>>  1 file changed, 12 insertions(+)
>>>
>>> diff --git a/boot/arm-trusted-firmware/arm-trusted-firmware.mk b/boot/arm-trusted-firmware/arm-trusted-firmware.mk
>>> index a3553e36cf..f5be39100e 100644
>>> --- a/boot/arm-trusted-firmware/arm-trusted-firmware.mk
>>> +++ b/boot/arm-trusted-firmware/arm-trusted-firmware.mk
>>> @@ -100,6 +100,18 @@ ARM_TRUSTED_FIRMWARE_MAKE_OPTS += MV_DDR_PATH=$(MV_DDR_MARVELL_DIR)
>>>  ARM_TRUSTED_FIRMWARE_DEPENDENCIES += mv-ddr-marvell
>>>  endif
>>>  
>>> +ifeq ($(BR2_SSP_REGULAR),y)
>>> +ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ENABLE_STACK_PROTECTOR=default
>>> +endif
>>> +
>>> +ifeq ($(BR2_SSP_STRONG),y)
>>> +ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ENABLE_STACK_PROTECTOR=strong
>>> +endif
>>> +
>>> +ifeq ($(BR2_SSP_ALL),y)
>>> +ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ENABLE_STACK_PROTECTOR=all
>>> +endif
>> 
>> It looks like the toolchain wrapper does that already. See
>> toolchain/toolchain-wrapper.mk. Are you sure this is needed?
>
> Hi Baruch,
>
> I don't see this functionality there.
>
> Note, that you need two things for getting the stack protector working:
>
> 1) Toolchain to emit additional code requiring external symbols
> 2) Runtime that provides the required external symbols
>
> The toolchain wrapper correctly addresses 1) by providing "-fstack-protector*" flags
> and your libc will then provide the required symbols for 2).
> However, TF-A does not link against libc and requires these build flags
> (ENABLE_STACK_PROTECTOR=x) in order to emit the required symbols.

Thanks for the additional explanation. Your commit log mentions that
already, but maybe it needs to be more verbose. Especially the fact that
ATF needs to provide the symbols itself because it is self contained
would be nice to mention in the commit log.

One more thing. Since all these BR2_SSP_ options are mutually
exclusive, I think you should use 'else ifeq' instead of separate
conditionals.

Thanks,
baruch

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch at tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH 1/1] boot/arm-trusted-firmware: Forward stack protection configuration
@ 2020-11-22 14:37 Christoph Müllner
  0 siblings, 0 replies; 6+ messages in thread
From: Christoph Müllner @ 2020-11-22 14:37 UTC (permalink / raw)
  To: buildroot

TF-A supports stack smashing protection (-fstack-protector-*).
However it currenlty fails to build when built with BR2_SSP_*
enabled, because stack protection needs to be enabled for the
TF-A build process itself as well to generate the required
symbols (e.g. __stack_chk_guard).

So in case we see that BR2_SSP_* is enabled, let's enable
the corresponding build flag for TF-A as documented in
the TF-A user guide.

Tested on a Rockchip PX30 based system.

Signed-off-by: Christoph M?llner <christoph.muellner@theobroma-systems.com>
---
 boot/arm-trusted-firmware/arm-trusted-firmware.mk | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/boot/arm-trusted-firmware/arm-trusted-firmware.mk b/boot/arm-trusted-firmware/arm-trusted-firmware.mk
index a3553e36cf..0597cecf71 100644
--- a/boot/arm-trusted-firmware/arm-trusted-firmware.mk
+++ b/boot/arm-trusted-firmware/arm-trusted-firmware.mk
@@ -100,6 +100,14 @@ ARM_TRUSTED_FIRMWARE_MAKE_OPTS += MV_DDR_PATH=$(MV_DDR_MARVELL_DIR)
 ARM_TRUSTED_FIRMWARE_DEPENDENCIES += mv-ddr-marvell
 endif
 
+ifeq ($(BR2_SSP_REGULAR),y)
+ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ENABLE_STACK_PROTECTOR=default
+else ifeq ($(BR2_SSP_STRONG),y)
+ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ENABLE_STACK_PROTECTOR=strong
+else ifeq ($(BR2_SSP_ALL),y)
+ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ENABLE_STACK_PROTECTOR=all
+endif
+
 ARM_TRUSTED_FIRMWARE_MAKE_TARGETS = all
 
 ifeq ($(BR2_TARGET_ARM_TRUSTED_FIRMWARE_FIP),y)
-- 
2.28.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [Buildroot] [PATCH 1/1] boot/arm-trusted-firmware: Forward stack protection configuration
@ 2020-11-20 21:35 Christoph Müllner
  0 siblings, 0 replies; 6+ messages in thread
From: Christoph Müllner @ 2020-11-20 21:35 UTC (permalink / raw)
  To: buildroot

TF-A supports stack smashing protection (-fstack-protector-*).
Since we already forward the required compiler flag, let's
also tell TF-A that we actually want the required symbols
(e.g. __stack_chk_guard) to be available.

Signed-off-by: Christoph M?llner <christoph.muellner@theobroma-systems.com>
---
 boot/arm-trusted-firmware/arm-trusted-firmware.mk | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/boot/arm-trusted-firmware/arm-trusted-firmware.mk b/boot/arm-trusted-firmware/arm-trusted-firmware.mk
index a3553e36cf..f5be39100e 100644
--- a/boot/arm-trusted-firmware/arm-trusted-firmware.mk
+++ b/boot/arm-trusted-firmware/arm-trusted-firmware.mk
@@ -100,6 +100,18 @@ ARM_TRUSTED_FIRMWARE_MAKE_OPTS += MV_DDR_PATH=$(MV_DDR_MARVELL_DIR)
 ARM_TRUSTED_FIRMWARE_DEPENDENCIES += mv-ddr-marvell
 endif
 
+ifeq ($(BR2_SSP_REGULAR),y)
+ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ENABLE_STACK_PROTECTOR=default
+endif
+
+ifeq ($(BR2_SSP_STRONG),y)
+ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ENABLE_STACK_PROTECTOR=strong
+endif
+
+ifeq ($(BR2_SSP_ALL),y)
+ARM_TRUSTED_FIRMWARE_MAKE_OPTS += ENABLE_STACK_PROTECTOR=all
+endif
+
 ARM_TRUSTED_FIRMWARE_MAKE_TARGETS = all
 
 ifeq ($(BR2_TARGET_ARM_TRUSTED_FIRMWARE_FIP),y)
-- 
2.28.0

^ permalink raw reply related	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-11-22 14:37 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-11-20 21:29 [Buildroot] [PATCH 1/1] boot/arm-trusted-firmware: Forward stack protection configuration Christoph Müllner
2020-11-21 16:59 ` Baruch Siach
2020-11-21 17:28   ` Christoph Müllner
2020-11-21 17:49     ` Baruch Siach
2020-11-20 21:35 Christoph Müllner
2020-11-22 14:37 Christoph Müllner

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.