diff mbox series

[v4,1/1] boot/arm-trusted-firmware: Forward stack protection configuration

Message ID 20201123131506.834183-1-christoph.muellner@theobroma-systems.com
State Accepted
Headers show
Series [v4,1/1] boot/arm-trusted-firmware: Forward stack protection configuration | expand

Commit Message

Christoph Müllner Nov. 23, 2020, 1:15 p.m. UTC
TF-A supports stack smashing protection (-fstack-protector-*).
However, that feature is currenlty silently disabled because
ENABLE_STACK_PROTECTOR is not set during build time.

As documented in the TF-A user guide, the flag ENABLE_STACK_PROTECTOR
is required to enable stack protection support. When enabled the symbols
for the stack protector (e.g. __stack_chk_guard) are built.
This needs to be done because TF-A does not link against an external
library that provides that symbols (e.g. libc).

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

This patch also fixes a the following linker errors with older TF-A versions
if BR2_SSP_* is enabled (i.e. -fstack-protector-* is used as compiler flag)
and ENABLE_STACK_PROTECTOR is not set, which are caused by the missing
stack protector symbols:

  [...]
  params_setup.c:(.text.params_early_setup+0xc): undefined reference to `__stack_chk_guard'
  aarch64-none-linux-gnu-ld: params_setup.c:(.text.params_early_setup+0x14): undefined reference to `__stack_chk_guard'
  aarch64-none-linux-gnu-ld: params_setup.c:(.text.params_early_setup+0x104): undefined reference to `__stack_chk_guard'
  aarch64-none-linux-gnu-ld: params_setup.c:(.text.params_early_setup+0x118): undefined reference to `__stack_chk_fail'
  aarch64-none-linux-gnu-ld: ./build/px30/release/bl31/pmu.o: in function `rockchip_soc_sys_pwr_dm_suspend':
  pmu.c:(.text.rockchip_soc_sys_pwr_dm_suspend+0xc): undefined reference to `__stack_chk_guard'
  [...]

TF-A releases after Nov 2019, that include 7af195e29a4, will circumvent
these issue by explicitliy and silently disabling the stack protector
by appending '-fno-stack-protector' to the compiler flags in case
ENABLE_STACK_PROTECTOR is not set.

Tested on a Rockchip PX30 based system (TF-A v2.2 and upstream/master).

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(+)

Comments

Baruch Siach Nov. 23, 2020, 6:23 p.m. UTC | #1
Hi Christoph,

On Mon, Nov 23 2020, Christoph Müllner wrote:
> TF-A supports stack smashing protection (-fstack-protector-*).
> However, that feature is currenlty silently disabled because
> ENABLE_STACK_PROTECTOR is not set during build time.
>
> As documented in the TF-A user guide, the flag ENABLE_STACK_PROTECTOR
> is required to enable stack protection support. When enabled the symbols
> for the stack protector (e.g. __stack_chk_guard) are built.
> This needs to be done because TF-A does not link against an external
> library that provides that symbols (e.g. libc).
>
> So in case we see that BR2_SSP_* is enabled, let's enable the corresponding
> ENABLE_STACK_PROTECTOR build flag for TF-A as documented in the TF-A user guide.
>
> This patch also fixes a the following linker errors with older TF-A versions
> if BR2_SSP_* is enabled (i.e. -fstack-protector-* is used as compiler flag)
> and ENABLE_STACK_PROTECTOR is not set, which are caused by the missing
> stack protector symbols:
>
>   [...]
>   params_setup.c:(.text.params_early_setup+0xc): undefined reference to `__stack_chk_guard'
>   aarch64-none-linux-gnu-ld: params_setup.c:(.text.params_early_setup+0x14): undefined reference to `__stack_chk_guard'
>   aarch64-none-linux-gnu-ld: params_setup.c:(.text.params_early_setup+0x104): undefined reference to `__stack_chk_guard'
>   aarch64-none-linux-gnu-ld: params_setup.c:(.text.params_early_setup+0x118): undefined reference to `__stack_chk_fail'
>   aarch64-none-linux-gnu-ld: ./build/px30/release/bl31/pmu.o: in function `rockchip_soc_sys_pwr_dm_suspend':
>   pmu.c:(.text.rockchip_soc_sys_pwr_dm_suspend+0xc): undefined reference to `__stack_chk_guard'
>   [...]
>
> TF-A releases after Nov 2019, that include 7af195e29a4, will circumvent
> these issue by explicitliy and silently disabling the stack protector
> by appending '-fno-stack-protector' to the compiler flags in case
> ENABLE_STACK_PROTECTOR is not set.
>
> Tested on a Rockchip PX30 based system (TF-A v2.2 and upstream/master).
>
> Signed-off-by: Christoph Müllner <christoph.muellner@theobroma-systems.com>

Thanks. FWIW,

Reviewed-by: Baruch Siach <baruch@tkos.co.il>

baruch

> ---
>  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)
Arnout Vandecappelle Dec. 15, 2020, 8:26 p.m. UTC | #2
On 23/11/2020 14:15, Christoph Müllner wrote:
> TF-A supports stack smashing protection (-fstack-protector-*).
> However, that feature is currenlty silently disabled because
> ENABLE_STACK_PROTECTOR is not set during build time.
> 
> As documented in the TF-A user guide, the flag ENABLE_STACK_PROTECTOR
> is required to enable stack protection support. When enabled the symbols
> for the stack protector (e.g. __stack_chk_guard) are built.
> This needs to be done because TF-A does not link against an external
> library that provides that symbols (e.g. libc).
> 
> So in case we see that BR2_SSP_* is enabled, let's enable the corresponding
> ENABLE_STACK_PROTECTOR build flag for TF-A as documented in the TF-A user guide.
> 
> This patch also fixes a the following linker errors with older TF-A versions
> if BR2_SSP_* is enabled (i.e. -fstack-protector-* is used as compiler flag)
> and ENABLE_STACK_PROTECTOR is not set, which are caused by the missing
> stack protector symbols:
> 
>   [...]
>   params_setup.c:(.text.params_early_setup+0xc): undefined reference to `__stack_chk_guard'
>   aarch64-none-linux-gnu-ld: params_setup.c:(.text.params_early_setup+0x14): undefined reference to `__stack_chk_guard'
>   aarch64-none-linux-gnu-ld: params_setup.c:(.text.params_early_setup+0x104): undefined reference to `__stack_chk_guard'
>   aarch64-none-linux-gnu-ld: params_setup.c:(.text.params_early_setup+0x118): undefined reference to `__stack_chk_fail'
>   aarch64-none-linux-gnu-ld: ./build/px30/release/bl31/pmu.o: in function `rockchip_soc_sys_pwr_dm_suspend':
>   pmu.c:(.text.rockchip_soc_sys_pwr_dm_suspend+0xc): undefined reference to `__stack_chk_guard'
>   [...]
> 
> TF-A releases after Nov 2019, that include 7af195e29a4, will circumvent
> these issue by explicitliy and silently disabling the stack protector
> by appending '-fno-stack-protector' to the compiler flags in case
> ENABLE_STACK_PROTECTOR is not set.
> 
> Tested on a Rockchip PX30 based system (TF-A v2.2 and upstream/master).
> 
> Signed-off-by: Christoph Müllner <christoph.muellner@theobroma-systems.com>

 Applied to master, thanks.

 Thank you for the excellent commit message and the research leading up to this v4!

 Regards,
 Arnout

> ---
>  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)
>
Peter Korsgaard Dec. 21, 2020, 2:16 p.m. UTC | #3
>>>>> "Arnout" == Arnout Vandecappelle <arnout@mind.be> writes:

 > On 23/11/2020 14:15, Christoph Müllner wrote:
 >> TF-A supports stack smashing protection (-fstack-protector-*).
 >> However, that feature is currenlty silently disabled because
 >> ENABLE_STACK_PROTECTOR is not set during build time.
 >> 
 >> As documented in the TF-A user guide, the flag ENABLE_STACK_PROTECTOR
 >> is required to enable stack protection support. When enabled the symbols
 >> for the stack protector (e.g. __stack_chk_guard) are built.
 >> This needs to be done because TF-A does not link against an external
 >> library that provides that symbols (e.g. libc).
 >> 
 >> So in case we see that BR2_SSP_* is enabled, let's enable the corresponding
 >> ENABLE_STACK_PROTECTOR build flag for TF-A as documented in the TF-A user guide.
 >> 
 >> This patch also fixes a the following linker errors with older TF-A versions
 >> if BR2_SSP_* is enabled (i.e. -fstack-protector-* is used as compiler flag)
 >> and ENABLE_STACK_PROTECTOR is not set, which are caused by the missing
 >> stack protector symbols:
 >> 
 >> [...]
 >> params_setup.c:(.text.params_early_setup+0xc): undefined reference to `__stack_chk_guard'
 >> aarch64-none-linux-gnu-ld: params_setup.c:(.text.params_early_setup+0x14): undefined reference to `__stack_chk_guard'
 >> aarch64-none-linux-gnu-ld: params_setup.c:(.text.params_early_setup+0x104): undefined reference to `__stack_chk_guard'
 >> aarch64-none-linux-gnu-ld: params_setup.c:(.text.params_early_setup+0x118): undefined reference to `__stack_chk_fail'
 >> aarch64-none-linux-gnu-ld: ./build/px30/release/bl31/pmu.o: in function `rockchip_soc_sys_pwr_dm_suspend':
 >> pmu.c:(.text.rockchip_soc_sys_pwr_dm_suspend+0xc): undefined reference to `__stack_chk_guard'
 >> [...]
 >> 
 >> TF-A releases after Nov 2019, that include 7af195e29a4, will circumvent
 >> these issue by explicitliy and silently disabling the stack protector
 >> by appending '-fno-stack-protector' to the compiler flags in case
 >> ENABLE_STACK_PROTECTOR is not set.
 >> 
 >> Tested on a Rockchip PX30 based system (TF-A v2.2 and upstream/master).
 >> 
 >> Signed-off-by: Christoph Müllner <christoph.muellner@theobroma-systems.com>

 >  Applied to master, thanks.

 >  Thank you for the excellent commit message and the research leading up to this v4!

Indeed, thanks.

Committed to 2020.02.x, 2020.08.x and 2020.11.x, thanks.
diff mbox series

Patch

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)