Re: [PATCH] lib: sbi: Add runtime stack overrun detection

Xiang W
2025-11-18T04:25:59Z

在 2025-11-18二的 12:19 +0800，Xiang W写道：
> Implement lightweight stack overrun detection using toolchain's
> -finstrument-functions instrumentation
>
> Reviewed-by: Xiang W <wangxiang@iscas.ac.cn>

Test in the following way

make CROSS_COMPILE=riscv64-linux-gnu- PLATFORM=generic O=/tmp/opensbi menuconfig
Generic SBI Support
-> Enable Stack Overflow runtime checking

make CROSS_COMPILE=riscv64-linux-gnu- PLATFORM=generic O=/tmp/opensbi DEBUG=1 run
.
.
.
OBJCOPY platform/generic/firmware/fw_payload.bin
qemu-system-riscv64 -M virt -m 256M -nographic -bios /tmp/opensbi/platform/generic/firmware/fw_payload.bin
Stack overflow detected in function 800244f0 (caller 8000465a), sp=800844f0 (80085000 - 80086000)

Regards,
Xiang W
> ---
>  Makefile                      |  4 +++
>  firmware/fw_base.S            |  3 +++
>  firmware/payloads/test_head.S | 16 ++++++++++++
>  lib/sbi/Kconfig               |  4 +++
>  lib/sbi/objects.mk            |  2 ++
>  lib/sbi/sbi_sochk.c           | 47 +++++++++++++++++++++++++++++++++++
>  6 files changed, 76 insertions(+)
>  create mode 100644 lib/sbi/sbi_sochk.c
>
> diff --git a/Makefile b/Makefile
> index 398eabe8..cd7d1ad5 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -449,6 +449,10 @@ else
>  CFLAGS += -O2
>  endif
>
> +ifneq ($(CONFIG_SBI_SOCHK),)
> +CFLAGS += -finstrument-functions
> +endif
> +
>  # Setup functions for compilation
>  define dynamic_flags
>  -I$(shell dirname $(2)) -D__OBJNAME__=$(subst -,_,$(shell basename $(1) .o))
> diff --git a/firmware/fw_base.S b/firmware/fw_base.S
> index 5300ecf2..6cd5725f 100644
> --- a/firmware/fw_base.S
> +++ b/firmware/fw_base.S
> @@ -366,6 +366,9 @@ _start_warm:
>   /* Setup stack */
>   add sp, tp, zero
>
> +#ifdef CONFIG_SBI_SOCHK
> + call sbi_sochk_init
> +#endif
>   /* Setup trap handler */
>   lla a4, _trap_handler
>   csrr a5, CSR_MISA
> diff --git a/firmware/payloads/test_head.S b/firmware/payloads/test_head.S
> index 070ce8aa..de4e334f 100644
> --- a/firmware/payloads/test_head.S
> +++ b/firmware/payloads/test_head.S
> @@ -112,3 +112,19 @@ _boot_a1:
>   .type __stack_chk_guard, %object
>  __stack_chk_guard:
>   RISCV_PTR 0x95B5FF5A
> +
> +#ifdef CONFIG_SBI_SOCHK
> + .section .text
> + .align 3
> + .weak __cyg_profile_func_enter
> + .type __cyg_profile_func_enter, %function
> +__cyg_profile_func_enter:
> + ret
> +
> +.section .text
> + .align 3
> + .weak __cyg_profile_func_exit
> + .type __cyg_profile_func_exit, %function
> +__cyg_profile_func_exit:
> + ret
> +#endif
> diff --git a/lib/sbi/Kconfig b/lib/sbi/Kconfig
> index c6cc04bc..77077991 100644
> --- a/lib/sbi/Kconfig
> +++ b/lib/sbi/Kconfig
> @@ -6,6 +6,10 @@ config CONSOLE_EARLY_BUFFER_SIZE
>   int "Early console buffer size (bytes)"
>   default 256
>
> +config SBI_SOCHK
> + bool "Enable Stack Overflow runtime checking"
> + default n
> +
>  config SBI_ECALL_TIME
>   bool "Timer extension"
>   default y
> diff --git a/lib/sbi/objects.mk b/lib/sbi/objects.mk
> index 8abe1e8e..ed6107ad 100644
> --- a/lib/sbi/objects.mk
> +++ b/lib/sbi/objects.mk
> @@ -64,6 +64,8 @@ libsbi-objs-$(CONFIG_SBI_ECALL_SSE) += sbi_ecall_sse.o
>  carray-sbi_ecall_exts-$(CONFIG_SBI_ECALL_MPXY) += ecall_mpxy
>  libsbi-objs-$(CONFIG_SBI_ECALL_MPXY) += sbi_ecall_mpxy.o
>
> +libsbi-objs-$(CONFIG_SBI_SOCHK) += sbi_sochk.o
> +
>  libsbi-objs-y += sbi_bitmap.o
>  libsbi-objs-y += sbi_bitops.o
>  libsbi-objs-y += sbi_console.o
> diff --git a/lib/sbi/sbi_sochk.c b/lib/sbi/sbi_sochk.c
> new file mode 100644
> index 00000000..950275d1
> --- /dev/null
> +++ b/lib/sbi/sbi_sochk.c
> @@ -0,0 +1,47 @@
> +
> +#include <sbi/sbi_scratch.h>
> +#include <sbi/sbi_platform.h>
> +#include <sbi/sbi_console.h>
> +#include <sbi/sbi_hart.h>
> +
> +extern struct sbi_platform platform;
> +
> +static bool __scratch_init_done = false;
> +
> +__attribute__((no_instrument_function, weak))
> +void sbi_sochk_init(void)
> +{
> + __scratch_init_done = true;
> +}
> +
> +
> +__attribute__((no_instrument_function, weak))
> +void __cyg_profile_func_enter(void *this_func, void *call_site)
> +{
> + struct sbi_scratch * scratch;
> + unsigned long sp, stack_start, stack_end;
> +
> + if (!__scratch_init_done)
> + return;
> +
> + scratch = sbi_scratch_thishart_ptr();
> +
> + asm volatile("mv %0, sp" : "=r"(sp));
> + stack_start = (unsigned long)scratch + SBI_SCRATCH_SIZE - platform.hart_stack_size;
> + stack_end = (unsigned long)scratch;
> +
> + if (sp < stack_start || sp > stack_end) {
> + /* Reset SP to output error messages */
> + asm volatile("mv sp, %0"::"r"(stack_end));
> + sbi_printf("Stack overflow detected in function %p (caller %p), "
> +    "sp=%p (%p - %p)\n",
> +    this_func, call_site, (void*)sp,
> +    (void*)stack_start, (void*)stack_end);
> + sbi_hart_hang();
> + }
> +}
> +
> +__attribute__((no_instrument_function, weak))
> +void __cyg_profile_func_exit(void *this_func, void *call_site)
> +{
> +}