[PATCH v2 2/2] firmware: Initial compiler built-in stack protector support

[Peter Lin]

Hi Alvin, Xiang,

On Wed, Jun 25, 2025 at 1:32 PM Xiang W <wxjstz at 126.com> wrote:
>
> 在 2025-06-25三的 11:16 +0800，Alvin Chang写道：
> > Add __stack_chk_fail() and __stack_chk_guard variable which are used by
> > compiler built-in stack protector.
> >
> > This patch just try to support stack-protector so the value of the stack
> > guard variable is simply fixed for now. It could be improved by
> > deriving from a random number generator, such as Zkr extension or any
> > platform-specific random number sources.
> >
> > Introduce three configurations for the stack protector:
> > 1. CONFIG_STACK_PROTECTOR to enable the stack protector feature by
> >    providing "-fstack-protector" compiler flag
> > 2. CONFIG_STACK_PROTECTOR_STRONG to provide "-fstack-protector-strong"
> > 3. CONFIG_STACK_PROTECTOR_ALL to provide "-fstack-protector-all"
> >
> > Instead of fixing the compiler flag of stack-protector feature as
> > "-fstack-protector", we derive it from the introduced Kconfig
> > configurations. The compiler flag "stack-protector-cflags-y" is defined
> > as Makefile "immediately expanded variables" with ":=". Thus, the
> > stronger configuration of the stack protector can overwrite the
> > preceding one.
> >
> > Signed-off-by: Alvin Chang <alvinga at andestech.com>
> > ---
> >  firmware/Kconfig              | 27 +++++++++++++++++++++++++++
> >  firmware/fw_base.S            | 17 +++++++++++++++++
> >  firmware/objects.mk           |  9 +++++++++
> >  firmware/payloads/test_head.S | 17 +++++++++++++++++
> >  4 files changed, 70 insertions(+)
> >
> > diff --git a/firmware/Kconfig b/firmware/Kconfig
> > index d6e0506..c1fee19 100644
> > --- a/firmware/Kconfig
> > +++ b/firmware/Kconfig
> > @@ -1 +1,28 @@
> >  # SPDX-License-Identifier: BSD-2-Clause
> > +
> > +menu "Stack Protector Support"
> > +
> > +config STACK_PROTECTOR
> > +     bool "Stack Protector buffer overflow detection"
> > +     default n
> > +     help
> > +       This option turns on the "stack-protector" compiler feature.
> > +
> > +config STACK_PROTECTOR_STRONG
> > +     bool "Strong Stack Protector"
> > +     depends on STACK_PROTECTOR
> > +     default n
> > +     help
> > +       Turn on the "stack-protector" with "-fstack-protector-strong" option.
> > +       Like -fstack-protector but includes additional functions to be
> > +       protected.
> > +
> > +config STACK_PROTECTOR_ALL
> > +     bool "Almighty Stack Protector"
> > +     depends on STACK_PROTECTOR
> > +     default n
> > +     help
> > +       Turn on the "stack-protector" with "-fstack-protector-all" option.
> > +       Like -fstack-protector except that all functions are protected.
> > +
> > +endmenu
> > diff --git a/firmware/fw_base.S b/firmware/fw_base.S
> > index 2498797..9dc73d0 100644
> > --- a/firmware/fw_base.S
> > +++ b/firmware/fw_base.S
> > @@ -736,6 +736,23 @@ _reset_regs:
> >
> >       ret
> >
> > +     /* This will be called when the stack corruption is detected */
> > +     .section .text
> > +     .align 3
> > +     .globl __stack_chk_fail
> > +     .type __stack_chk_fail, %function
>
> The function can be removed to create an alias, as shown below:
>
> > +__stack_chk_fail:
> > +     wfi
> > +     j       __stack_chk_fail
>
>         .equ  __stack_chk_fail, _start_hang

How about adding an error message as PATCHv1.

+       .section .rodata
+.Lstack_corrupt_msg:
+       .string "stack smashing detected\n"
+
        /* This will be called when the stack corruption is detected */
        .section .text
        .align 3
        .globl __stack_chk_fail
        .type __stack_chk_fail, %function
 __stack_chk_fail:
-       wfi
-       j       __stack_chk_fail
+       la    a0, .Lstack_corrupt_msg
+       call  sbi_panic

Otherwise it looks no different when the stack protector is disabled.

Regards,
Peter Lin

> Regards,
> Xiang W
> > +
> > +     /* Initial value of the stack guard variable */
> > +     .section .data
> > +     .align 3
> > +     .globl __stack_chk_guard
> > +     .type __stack_chk_guard, %object
> > +__stack_chk_guard:
> > +     RISCV_PTR       0x95B5FF5A
> > +
> >  #ifdef FW_FDT_PATH
> >       .section .rodata
> >       .align 4
> > diff --git a/firmware/objects.mk b/firmware/objects.mk
> > index a90485d..bfec467 100644
> > --- a/firmware/objects.mk
> > +++ b/firmware/objects.mk
> > @@ -66,3 +66,12 @@ endif
> >  ifdef FW_OPTIONS
> >  firmware-genflags-y += -DFW_OPTIONS=$(FW_OPTIONS)
> >  endif
> > +
> > +ifeq ($(CONFIG_STACK_PROTECTOR),y)
> > +stack-protector-cflags-$(CONFIG_STACK_PROTECTOR) := -fstack-protector
> > +stack-protector-cflags-$(CONFIG_STACK_PROTECTOR_STRONG) := -fstack-protector-strong
> > +stack-protector-cflags-$(CONFIG_STACK_PROTECTOR_ALL) := -fstack-protector-all
> > +else
> > +stack-protector-cflags-y := -fno-stack-protector
> > +endif
> > +firmware-cflags-y += $(stack-protector-cflags-y)
> > diff --git a/firmware/payloads/test_head.S b/firmware/payloads/test_head.S
> > index 7d25e07..b5fd46b 100644
> > --- a/firmware/payloads/test_head.S
> > +++ b/firmware/payloads/test_head.S
> > @@ -97,3 +97,20 @@ _boot_a0:
> >       RISCV_PTR       0
> >  _boot_a1:
> >       RISCV_PTR       0
> > +
> > +     /* This will be called when the stack corruption is detected */
> > +     .section .text
> > +     .align 3
> > +     .globl __stack_chk_fail
> > +     .type __stack_chk_fail, %function
> > +__stack_chk_fail:
> > +     wfi
> > +     j       __stack_chk_fail
> > +
> > +     /* Initial value of the stack guard variable */
> > +     .section .data
> > +     .align 3
> > +     .globl __stack_chk_guard
> > +     .type __stack_chk_guard, %object
> > +__stack_chk_guard:
> > +     RISCV_PTR       0x95B5FF5A
> > --
> > 2.43.0
> >
>