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

[Bo Gan]

Hi Xiang,

On 11/17/25 20:19, Xiang W wrote:
> Implement lightweight stack overrun detection using toolchain's
> -finstrument-functions instrumentation
> 
> Reviewed-by: Xiang W <wangxiang at iscas.ac.cn>
> ---
>   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));

I don't think it's safe to switch stack in C function. I know you want
to avoid re-entrant, but do it in C function is way too risky. Better
have an asm wapper over this function and do it before invoking the C
portion.

Bo

> +		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)
> +{
> +}