[Buildroot] [PATCH] boot/arm-trusted-firmware: don't enable SSP by default

[Buildroot] [PATCH] boot/arm-trusted-firmware: don't enable SSP by default

[Baruch Siach via buildroot]

SSP support requires support in ATF platform code. Not all platforms
implement plat_get_stack_protector_canary() hook. The result is build
failure:

(.text.asm.update_stack_protector_canary+0x4): undefined reference to `plat_get_stack_protector_canary'

Commit cf176128ec4 ("boot/arm-trusted-firmware: add SSP option")
originally introduces this issue. But then commit ccac9a5bbbd
("boot/arm-trusted-firmware: don't force ENABLE_STACK_PROTECTOR") hid
the problem by effectively disabling SSP for all platforms. So only
after commit 09acc7cbc91f5 ("boot/arm-trusted-firmware: fix SSP
support") the issue showed up.

Make SSP an opt-in for platform that actually provide the
plat_get_stack_protector_canary() hook.

Cc: Sergey Matyukevich <geomatsi@gmail.com>
Cc: Dick Olsson <hi@senzilla.io>
Tested-by: Heiko Thiery <heiko.thiery@gmail.com>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 boot/arm-trusted-firmware/Config.in | 1 -
 1 file changed, 1 deletion(-)

diff --git a/boot/arm-trusted-firmware/Config.in b/boot/arm-trusted-firmware/Config.in
index 5070849d9b43..a9b9bbcc5a1f 100644
--- a/boot/arm-trusted-firmware/Config.in
+++ b/boot/arm-trusted-firmware/Config.in
@@ -212,7 +212,6 @@ config BR2_TARGET_ARM_TRUSTED_FIRMWARE_NEEDS_ARM32_TOOLCHAIN
 
 config BR2_TARGET_ARM_TRUSTED_FIRMWARE_SSP
 	bool "Build with SSP"
-	default y
 	depends on BR2_TOOLCHAIN_HAS_SSP
 	depends on !BR2_SSP_NONE
 	help
-- 
2.35.1

_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] boot/arm-trusted-firmware: don't enable SSP by default

[Thomas Petazzoni via buildroot]

On Fri, 28 Oct 2022 08:36:27 +0300
Baruch Siach via buildroot <buildroot@buildroot.org> wrote:

> SSP support requires support in ATF platform code. Not all platforms
> implement plat_get_stack_protector_canary() hook. The result is build
> failure:
> 
> (.text.asm.update_stack_protector_canary+0x4): undefined reference to `plat_get_stack_protector_canary'
> 
> Commit cf176128ec4 ("boot/arm-trusted-firmware: add SSP option")
> originally introduces this issue. But then commit ccac9a5bbbd
> ("boot/arm-trusted-firmware: don't force ENABLE_STACK_PROTECTOR") hid
> the problem by effectively disabling SSP for all platforms. So only
> after commit 09acc7cbc91f5 ("boot/arm-trusted-firmware: fix SSP
> support") the issue showed up.
> 
> Make SSP an opt-in for platform that actually provide the
> plat_get_stack_protector_canary() hook.
> 
> Cc: Sergey Matyukevich <geomatsi@gmail.com>
> Cc: Dick Olsson <hi@senzilla.io>
> Tested-by: Heiko Thiery <heiko.thiery@gmail.com>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> ---
>  boot/arm-trusted-firmware/Config.in | 1 -
>  1 file changed, 1 deletion(-)

We had a quick discussion with Peter Korsgaard on IRC a few days ago
about this, and I do agree making this opt-in is the right solution, so
I've applied. We'll see if other maintainers disagree :-)

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] boot/arm-trusted-firmware: don't enable SSP by default

[Peter Korsgaard]

>>>>> "Baruch" == Baruch Siach via buildroot <buildroot@buildroot.org> writes:

 > SSP support requires support in ATF platform code. Not all platforms
 > implement plat_get_stack_protector_canary() hook. The result is build
 > failure:

 > (.text.asm.update_stack_protector_canary+0x4): undefined reference to `plat_get_stack_protector_canary'

 > Commit cf176128ec4 ("boot/arm-trusted-firmware: add SSP option")
 > originally introduces this issue. But then commit ccac9a5bbbd
 > ("boot/arm-trusted-firmware: don't force ENABLE_STACK_PROTECTOR") hid
 > the problem by effectively disabling SSP for all platforms. So only
 > after commit 09acc7cbc91f5 ("boot/arm-trusted-firmware: fix SSP
 > support") the issue showed up.

 > Make SSP an opt-in for platform that actually provide the
 > plat_get_stack_protector_canary() hook.

 > Cc: Sergey Matyukevich <geomatsi@gmail.com>
 > Cc: Dick Olsson <hi@senzilla.io>
 > Tested-by: Heiko Thiery <heiko.thiery@gmail.com>
 > Signed-off-by: Baruch Siach <baruch@tkos.co.il>

Committed to 2022.08.x and 2022.02.x, thanks.

-- 
Bye, Peter Korsgaard
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] boot/arm-trusted-firmware: don't enable SSP by default

[Thomas Petazzoni via buildroot]

Hello Baruch,

On Fri, 28 Oct 2022 08:36:27 +0300
Baruch Siach via buildroot <buildroot@buildroot.org> wrote:

> SSP support requires support in ATF platform code. Not all platforms
> implement plat_get_stack_protector_canary() hook. The result is build
> failure:
> 
> (.text.asm.update_stack_protector_canary+0x4): undefined reference to `plat_get_stack_protector_canary'
> 
> Commit cf176128ec4 ("boot/arm-trusted-firmware: add SSP option")
> originally introduces this issue. But then commit ccac9a5bbbd
> ("boot/arm-trusted-firmware: don't force ENABLE_STACK_PROTECTOR") hid
> the problem by effectively disabling SSP for all platforms. So only
> after commit 09acc7cbc91f5 ("boot/arm-trusted-firmware: fix SSP
> support") the issue showed up.
> 
> Make SSP an opt-in for platform that actually provide the
> plat_get_stack_protector_canary() hook.
> 
> Cc: Sergey Matyukevich <geomatsi@gmail.com>
> Cc: Dick Olsson <hi@senzilla.io>
> Tested-by: Heiko Thiery <heiko.thiery@gmail.com>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>

Unfortunately, it seems like the SSP stuff for TF-A still doesn't work.
We still have build failures on several defconfigs:

https://gitlab.com/buildroot.org/buildroot/-/jobs/3301821171
https://gitlab.com/buildroot.org/buildroot/-/jobs/3301821262
https://gitlab.com/buildroot.org/buildroot/-/jobs/3301821323
https://gitlab.com/buildroot.org/buildroot/-/jobs/3301821325
https://gitlab.com/buildroot.org/buildroot/-/jobs/3301821326
https://gitlab.com/buildroot.org/buildroot/-/jobs/3301821327
https://gitlab.com/buildroot.org/buildroot/-/jobs/3301821374
https://gitlab.com/buildroot.org/buildroot/-/jobs/3301821374
https://gitlab.com/buildroot.org/buildroot/-/jobs/3301821388
https://gitlab.com/buildroot.org/buildroot/-/jobs/3301821583

Since your commit 09acc7cbc91f50305730ca0690a58fb93529034b
boot/arm-trusted-firmware: fix SSP support, we no longer force disable
SSP support when BR2_TARGET_ARM_TRUSTED_FIRMWARE_SSP is disabled.

If one of BR2_SSP_REGULAR, BR2_SSP_STRONG or BR2_SSP_ALL is enabled,
all code gets built with SSP, including the TF-A code.

Prior to commit 09acc7cbc91f50305730ca0690a58fb93529034b, we were
passing ENABLE_STACK_PROTECTOR=0 when
BR2_TARGET_ARM_TRUSTED_FIRMWARE_SSP was disabled, making sure that TF-A
was forcefully disabling SSP, even if it was globally enabled via one
of BR2_SSP_...

So I'm afraid the fix in 09acc7cbc91f50305730ca0690a58fb93529034b does
not work :-/

Thomas
-- 
Thomas Petazzoni, co-owner and CEO, Bootlin
Embedded Linux and Kernel engineering and training
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot

Re: [Buildroot] [PATCH] boot/arm-trusted-firmware: don't enable SSP by default

[Baruch Siach via buildroot]

Hi Thomas,

On Fri, Nov 11 2022, Thomas Petazzoni wrote:
> On Fri, 28 Oct 2022 08:36:27 +0300
> Baruch Siach via buildroot <buildroot@buildroot.org> wrote:
>
>> SSP support requires support in ATF platform code. Not all platforms
>> implement plat_get_stack_protector_canary() hook. The result is build
>> failure:
>> 
>> (.text.asm.update_stack_protector_canary+0x4): undefined reference to `plat_get_stack_protector_canary'
>> 
>> Commit cf176128ec4 ("boot/arm-trusted-firmware: add SSP option")
>> originally introduces this issue. But then commit ccac9a5bbbd
>> ("boot/arm-trusted-firmware: don't force ENABLE_STACK_PROTECTOR") hid
>> the problem by effectively disabling SSP for all platforms. So only
>> after commit 09acc7cbc91f5 ("boot/arm-trusted-firmware: fix SSP
>> support") the issue showed up.
>> 
>> Make SSP an opt-in for platform that actually provide the
>> plat_get_stack_protector_canary() hook.
>> 
>> Cc: Sergey Matyukevich <geomatsi@gmail.com>
>> Cc: Dick Olsson <hi@senzilla.io>
>> Tested-by: Heiko Thiery <heiko.thiery@gmail.com>
>> Signed-off-by: Baruch Siach <baruch@tkos.co.il>
>
> Unfortunately, it seems like the SSP stuff for TF-A still doesn't work.
> We still have build failures on several defconfigs:
>
> https://gitlab.com/buildroot.org/buildroot/-/jobs/3301821171
> https://gitlab.com/buildroot.org/buildroot/-/jobs/3301821262
> https://gitlab.com/buildroot.org/buildroot/-/jobs/3301821323
> https://gitlab.com/buildroot.org/buildroot/-/jobs/3301821325
> https://gitlab.com/buildroot.org/buildroot/-/jobs/3301821326
> https://gitlab.com/buildroot.org/buildroot/-/jobs/3301821327
> https://gitlab.com/buildroot.org/buildroot/-/jobs/3301821374
> https://gitlab.com/buildroot.org/buildroot/-/jobs/3301821374
> https://gitlab.com/buildroot.org/buildroot/-/jobs/3301821388
> https://gitlab.com/buildroot.org/buildroot/-/jobs/3301821583
>
> Since your commit 09acc7cbc91f50305730ca0690a58fb93529034b
> boot/arm-trusted-firmware: fix SSP support, we no longer force disable
> SSP support when BR2_TARGET_ARM_TRUSTED_FIRMWARE_SSP is disabled.
>
> If one of BR2_SSP_REGULAR, BR2_SSP_STRONG or BR2_SSP_ALL is enabled,
> all code gets built with SSP, including the TF-A code.
>
> Prior to commit 09acc7cbc91f50305730ca0690a58fb93529034b, we were
> passing ENABLE_STACK_PROTECTOR=0 when
> BR2_TARGET_ARM_TRUSTED_FIRMWARE_SSP was disabled, making sure that TF-A
> was forcefully disabling SSP, even if it was globally enabled via one
> of BR2_SSP_...
>
> So I'm afraid the fix in 09acc7cbc91f50305730ca0690a58fb93529034b does
> not work :-/

Well, the fix works in the sense that it allows to enable SSP for ATF
while previously it was always disabled.

Failing configs all appear to use ATF version 2.2 or older that lacks
commit 7af195e29a421 ("Disable stack protection explicitly").

The only solution I can think of is to pass 'TF_CFLAGS =
-fno-stack-protector' in the environment when
BR2_TARGET_ARM_TRUSTED_FIRMWARE_SSP is disabled.

I'll give it a test to see how it works.

baruch

-- 
                                                     ~. .~   Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
   - baruch@tkos.co.il - tel: +972.52.368.4656, http://www.tkos.co.il -
_______________________________________________
buildroot mailing list
buildroot@buildroot.org
https://lists.buildroot.org/mailman/listinfo/buildroot