| Message ID | 20201120212912.2993985-1-christoph.muellner@theobroma-systems.com |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [1/1] boot/arm-trusted-firmware: Forward stack protection configuration | expand |
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
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
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
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)
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(+)