| Message ID | bab58c04544d0247bce855c8b984ce0d5349def4.1666935387.git.baruch@tkos.co.il |
|---|---|
| State | Accepted |
| Headers | show |
| Series | boot/arm-trusted-firmware: don't enable SSP by default | expand |
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
>>>>> "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.
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
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
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
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(-)