* [PATCH] lib: sbi: Add runtime stack overrun detection
@ 2025-11-18 4:19 Xiang W
2025-11-18 4:25 ` Xiang W
2025-11-18 8:43 ` Bo Gan
0 siblings, 2 replies; 4+ messages in thread
From: Xiang W @ 2025-11-18 4:19 UTC (permalink / raw)
To: opensbi; +Cc: Xiang W, Xiang W
Implement lightweight stack overrun detection using toolchain's
-finstrument-functions instrumentation
Reviewed-by: Xiang W <wangxiang@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));
+ 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)
+{
+}
--
2.47.3
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [PATCH] lib: sbi: Add runtime stack overrun detection
2025-11-18 4:19 [PATCH] lib: sbi: Add runtime stack overrun detection Xiang W
@ 2025-11-18 4:25 ` Xiang W
2025-11-18 8:43 ` Bo Gan
1 sibling, 0 replies; 4+ messages in thread
From: Xiang W @ 2025-11-18 4:25 UTC (permalink / raw)
To: opensbi
在 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)
> +{
> +}
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] lib: sbi: Add runtime stack overrun detection
2025-11-18 4:19 [PATCH] lib: sbi: Add runtime stack overrun detection Xiang W
2025-11-18 4:25 ` Xiang W
@ 2025-11-18 8:43 ` Bo Gan
2025-11-18 9:25 ` Andreas Schwab
1 sibling, 1 reply; 4+ messages in thread
From: Bo Gan @ 2025-11-18 8:43 UTC (permalink / raw)
To: Xiang W, opensbi; +Cc: Xiang W
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@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)
> +{
> +}
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [PATCH] lib: sbi: Add runtime stack overrun detection
2025-11-18 8:43 ` Bo Gan
@ 2025-11-18 9:25 ` Andreas Schwab
0 siblings, 0 replies; 4+ messages in thread
From: Andreas Schwab @ 2025-11-18 9:25 UTC (permalink / raw)
To: Bo Gan; +Cc: Xiang W, opensbi, Xiang W
On Nov 18 2025, Bo Gan wrote:
> Hi Xiang,
>
> On 11/17/25 20:19, Xiang W wrote:
>> + 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.
The GCC docs makes this explicitly undefined:
Another restriction is that the clobber list should not contain the
stack pointer register. This is because the compiler requires the value
of the stack pointer to be the same after an 'asm' statement as it was
on entry to the statement. However, previous versions of GCC did not
enforce this rule and allowed the stack pointer to appear in the list,
with unclear semantics. This behavior is deprecated and listing the
stack pointer may become an error in future versions of GCC.
--
Andreas Schwab, SUSE Labs, schwab@suse.de
GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7
"And now for something completely different."
--
opensbi mailing list
opensbi@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/opensbi
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2025-11-18 9:25 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-11-18 4:19 [PATCH] lib: sbi: Add runtime stack overrun detection Xiang W
2025-11-18 4:25 ` Xiang W
2025-11-18 8:43 ` Bo Gan
2025-11-18 9:25 ` Andreas Schwab
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox