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

From: Baruch Siach via buildroot <buildroot@buildroot.org>
To: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
Cc: buildroot@busybox.net, Julien Olivain <ju.o@free.fr>,
	Fabio Estevam <festevam@gmail.com>,
	Heiko Thiery <heiko.thiery@gmail.com>,
	Sergey Matyukevich <geomatsi@gmail.com>
Subject: Re: [Buildroot] [PATCH] boot/arm-trusted-firmware: don't enable SSP by default
Date: Sun, 13 Nov 2022 20:03:15 +0200	[thread overview]
Message-ID: <87h6z2u3lx.fsf@tarshish> (raw)
In-Reply-To: <20221111211809.5cd3802e@windsurf>

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