arch/x86: Add breakpoint to stack canary

In order to debug stack smashing add a write breakpoint to the
stack canary (at _stack or _car_stack) and print the IP when the
stack canary is written.

TEST: Wrote to address _stack in ramstage and got the EIP of the
      code that smashed the stack canary.

Change-Id: I8adf07a8425856795a4a71da5c41bec2244b02a8
Signed-off-by: Patrick Rudolph <patrick.rudolph@9elements.com>
Reviewed-on: https://review.coreboot.org/c/coreboot/+/84833
Reviewed-by: Angel Pons <th3fanbus@gmail.com>
Reviewed-by: Shuo Liu <shuo.liu@intel.com>
Reviewed-by: Jérémy Compostella <jeremy.compostella@intel.com>
Reviewed-by: Subrata Banik <subratabanik@google.com>
Tested-by: build bot (Jenkins) <no-reply@coreboot.org>

src/arch/x86/Kconfig

@@ -368,6 +368,18 @@ config DEBUG_NULL_DEREF_BREAKPOINTS_IN_ALL_STAGES
 	default y
 	depends on DEBUG_NULL_DEREF_BREAKPOINTS && DEBUG_HW_BREAKPOINTS_IN_ALL_STAGES
 
+config DEBUG_STACK_OVERFLOW_BREAKPOINTS
+	bool
+	default y
+	depends on DEBUG_HW_BREAKPOINTS
+	help
+	  Enable support for catching stack overflows
+
+config DEBUG_STACK_OVERFLOW_BREAKPOINTS_IN_ALL_STAGES
+	bool
+	default y
+	depends on DEBUG_STACK_OVERFLOW_BREAKPOINTS && DEBUG_HW_BREAKPOINTS_IN_ALL_STAGES
+
 # Some EC need an "EC firmware pointer" (a data structure hinting the address
 # of its firmware blobs) being put at a fixed position. Its space
 # (__section__(".ecfw_ptr")) should be reserved if it lies in the range of a

src/arch/x86/Makefile.mk

@@ -69,6 +69,7 @@ bootblock-$(CONFIG_ARCH_BOOTBLOCK_X86_64) += memmove_64.S
 bootblock-$(CONFIG_COLLECT_TIMESTAMPS_TSC) += timestamp.c
 bootblock-$(CONFIG_X86_TOP4G_BOOTMEDIA_MAP) += mmap_boot.c
 bootblock-$(CONFIG_DEBUG_NULL_DEREF_BREAKPOINTS_IN_ALL_STAGES) += null_breakpoint.c
+bootblock-$(CONFIG_DEBUG_STACK_OVERFLOW_BREAKPOINTS_IN_ALL_STAGES) += stack_canary_breakpoint.c
 bootblock-$(CONFIG_BOOTBLOCK_NORMAL) += bootblock_normal.c
 bootblock-y += gdt_init.S
 bootblock-y += id.S
@@ -116,6 +117,7 @@ verstage-$(CONFIG_ARCH_VERSTAGE_X86_32) += memmove_32.c
 verstage-$(CONFIG_ARCH_VERSTAGE_X86_64) += memmove_64.S
 verstage-$(CONFIG_X86_TOP4G_BOOTMEDIA_MAP) += mmap_boot.c
 verstage-$(CONFIG_DEBUG_NULL_DEREF_BREAKPOINTS_IN_ALL_STAGES) += null_breakpoint.c
+verstage-$(CONFIG_DEBUG_STACK_OVERFLOW_BREAKPOINTS_IN_ALL_STAGES) += stack_canary_breakpoint.c
 # If verstage is a separate stage it means there's no need
 # for a chipset-specific car_stage_entry() so use the generic one
 # which just calls verstage().
@@ -152,6 +154,7 @@ romstage-$(CONFIG_ARCH_ROMSTAGE_X86_64) += memmove_64.S
 romstage-y += memset.c
 romstage-$(CONFIG_X86_TOP4G_BOOTMEDIA_MAP) += mmap_boot.c
 romstage-$(CONFIG_DEBUG_NULL_DEREF_BREAKPOINTS_IN_ALL_STAGES) += null_breakpoint.c
+romstage-$(CONFIG_DEBUG_STACK_OVERFLOW_BREAKPOINTS_IN_ALL_STAGES) += stack_canary_breakpoint.c
 romstage-y += postcar_loader.c
 romstage-$(CONFIG_COLLECT_TIMESTAMPS_TSC) += timestamp.c
 romstage-$(CONFIG_HAVE_CF9_RESET) += cf9_reset.c
@@ -194,6 +197,7 @@ postcar-$(CONFIG_ARCH_POSTCAR_X86_64) += memmove_64.S
 postcar-y += memset.c
 postcar-$(CONFIG_X86_TOP4G_BOOTMEDIA_MAP) += mmap_boot.c
 postcar-$(CONFIG_DEBUG_NULL_DEREF_BREAKPOINTS_IN_ALL_STAGES) += null_breakpoint.c
+postcar-$(CONFIG_DEBUG_STACK_OVERFLOW_BREAKPOINTS_IN_ALL_STAGES) += stack_canary_breakpoint.c
 postcar-y += postcar.c
 postcar-$(CONFIG_COLLECT_TIMESTAMPS_TSC) += timestamp.c
 postcar-$(CONFIG_HAVE_CF9_RESET) += cf9_reset.c
@@ -238,6 +242,7 @@ ramstage-y += memset.c
 ramstage-$(CONFIG_X86_TOP4G_BOOTMEDIA_MAP) += mmap_boot.c
 ramstage-$(CONFIG_GENERATE_MP_TABLE) += mpspec.c
 ramstage-$(CONFIG_DEBUG_NULL_DEREF_BREAKPOINTS) += null_breakpoint.c
+ramstage-$(CONFIG_DEBUG_STACK_OVERFLOW_BREAKPOINTS) += stack_canary_breakpoint.c
 ramstage-$(CONFIG_GENERATE_PIRQ_TABLE) += pirq_routing.c
 ramstage-y += rdrand.c
 ramstage-$(CONFIG_GENERATE_SMBIOS_TABLES) += smbios.c

src/arch/x86/exception.c

@@ -2,6 +2,7 @@
 #include <arch/cpu.h>
 #include <arch/breakpoint.h>
 #include <arch/null_breakpoint.h>
+#include <arch/stack_canary_breakpoint.h>
 #include <arch/exception.h>
 #include <arch/registers.h>
 #include <commonlib/helpers.h>
@@ -665,4 +666,5 @@ asmlinkage void exception_init(void)
 	load_idt(idt, sizeof(idt));
 
 	null_breakpoint_init();
+	stack_canary_breakpoint_init();
 }

src/arch/x86/include/arch/stack_canary_breakpoint.h

@@ -0,0 +1,21 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+#ifndef _STACK_CANARY_BREAKPOINT_H_
+#define _STACK_CANARY_BREAKPOINT_H_
+
+#if CONFIG(DEBUG_STACK_OVERFLOW_BREAKPOINTS) && \
+    (CONFIG(DEBUG_STACK_OVERFLOW_BREAKPOINTS_IN_ALL_STAGES) || ENV_RAMSTAGE)
+
+/* Places a data breakpoint at stack canary. */
+void stack_canary_breakpoint_init(void);
+void stack_canary_breakpoint_disable(void);
+#else
+static inline void stack_canary_breakpoint_init(void)
+{
+	/* Not implemented */
+}
+static inline void stack_canary_breakpoint_disable(void)
+{
+	/* Not implemented */
+}
+#endif
+#endif /* _STACK_CANARY_BREAKPOINT_H_ */

src/arch/x86/stack_canary_breakpoint.c

@@ -0,0 +1,63 @@
+/* SPDX-License-Identifier: GPL-2.0-only */
+
+#include <arch/breakpoint.h>
+#include <arch/stack_canary_breakpoint.h>
+#include <arch/symbols.h>
+#include <bootstate.h>
+#include <console/console.h>
+#include <types.h>
+#include <symbols.h>
+
+static struct breakpoint_handle stack_canary_bp;
+
+static int handle_stack_canary_written(struct breakpoint_handle handle, struct eregs *regs)
+{
+#if ENV_X86_64
+	printk(BIOS_ERR, "Stack corruption detected at rip: 0x%llx\n", regs->rip);
+#else
+	printk(BIOS_ERR, "Stack corruption detected at eip: 0x%x\n", regs->eip);
+#endif
+	return 0;
+}
+
+static void create_stack_canary_breakpoint(uintptr_t *addr)
+{
+	enum breakpoint_result res =
+		breakpoint_create_data(&stack_canary_bp, addr, sizeof(uintptr_t), true);
+
+	if (res != BREAKPOINT_RES_OK) {
+		printk(BIOS_ERR, "Failed to create stack canary breakpoint\n");
+		return;
+	}
+
+	breakpoint_set_handler(stack_canary_bp, &handle_stack_canary_written);
+	breakpoint_enable(stack_canary_bp, true);
+}
+
+void stack_canary_breakpoint_init(void)
+{
+	uintptr_t *addr;
+
+	if (CONFIG(RESET_VECTOR_IN_RAM)) {
+		addr = (uintptr_t *)_earlyram_stack;
+	} else if (ENV_CACHE_AS_RAM) {
+		addr = (uintptr_t *)_car_stack;
+	} else {
+		addr = (uintptr_t *)_stack;
+	}
+
+	create_stack_canary_breakpoint(addr);
+}
+
+void stack_canary_breakpoint_disable(void)
+{
+	breakpoint_remove(stack_canary_bp);
+}
+
+static void stack_canary_breakpoint_disable_hook(void *unused)
+{
+	stack_canary_breakpoint_disable();
+}
+
+BOOT_STATE_INIT_ENTRY(BS_OS_RESUME, BS_ON_ENTRY, stack_canary_breakpoint_disable_hook, NULL);
+BOOT_STATE_INIT_ENTRY(BS_PAYLOAD_BOOT, BS_ON_ENTRY, stack_canary_breakpoint_disable_hook, NULL);

src/drivers/intel/fsp2_0/memory_init.c

@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later */
 
 #include <arch/null_breakpoint.h>
+#include <arch/stack_canary_breakpoint.h>
 #include <arch/symbols.h>
 #include <assert.h>
 #include <cbfs.h>
@@ -412,6 +413,7 @@ static void do_fsp_memory_init(const struct fspm_context *context, bool s3wake)
 
 	/* FSP disables the interrupt handler so remove debug exceptions temporarily  */
 	null_breakpoint_disable();
+	stack_canary_breakpoint_disable();
 	post_code(POSTCODE_FSP_MEMORY_INIT);
 	timestamp_add_now(TS_FSP_MEMORY_INIT_START);
 	if (ENV_X86_64 && CONFIG(PLATFORM_USES_FSP2_X86_32))
@@ -421,6 +423,7 @@ static void do_fsp_memory_init(const struct fspm_context *context, bool s3wake)
 	else
 		status = fsp_raminit(&fspm_upd, fsp_get_hob_list_ptr());
 	null_breakpoint_init();
+	stack_canary_breakpoint_init();
 
 	post_code(POSTCODE_FSP_MEMORY_EXIT);
 	timestamp_add_now(TS_FSP_MEMORY_INIT_END);

src/drivers/intel/fsp2_0/notify.c

@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later */
 
 #include <arch/null_breakpoint.h>
+#include <arch/stack_canary_breakpoint.h>
 #include <bootstate.h>
 #include <console/console.h>
 #include <cpu/x86/mtrr.h>
@@ -78,11 +79,13 @@ static void fsp_notify(enum fsp_notify_phase phase)
 
 	/* FSP disables the interrupt handler so remove debug exceptions temporarily  */
 	null_breakpoint_disable();
+	stack_canary_breakpoint_disable();
 	if (ENV_X86_64 && CONFIG(PLATFORM_USES_FSP2_X86_32))
 		ret = protected_mode_call_1arg(fspnotify, (uintptr_t)&notify_params);
 	else
 		ret = fspnotify(&notify_params);
 	null_breakpoint_init();
+	stack_canary_breakpoint_init();
 
 	timestamp_add_now(data->timestamp_after);
 	post_code(data->post_code_after);

src/drivers/intel/fsp2_0/silicon_init.c

@@ -1,6 +1,7 @@
 /* SPDX-License-Identifier: GPL-2.0-or-later */
 
 #include <arch/null_breakpoint.h>
+#include <arch/stack_canary_breakpoint.h>
 #include <bootsplash.h>
 #include <bootstate.h>
 #include <cbfs.h>
@@ -132,11 +133,13 @@ static void do_silicon_init(struct fsp_header *hdr)
 
 	/* FSP disables the interrupt handler so remove debug exceptions temporarily  */
 	null_breakpoint_disable();
+	stack_canary_breakpoint_disable();
 	if (ENV_X86_64 && CONFIG(PLATFORM_USES_FSP2_X86_32))
 		status = protected_mode_call_1arg(silicon_init, (uintptr_t)upd);
 	else
 		status = silicon_init(upd);
 	null_breakpoint_init();
+	stack_canary_breakpoint_init();
 
 	fsp_printk(status, BIOS_INFO, "FSPS");
 

src/include/symbols.h

@@ -82,6 +82,7 @@ DECLARE_OPTIONAL_REGION(opensbi)
 DECLARE_OPTIONAL_REGION(bl31)
 DECLARE_REGION(transfer_buffer)
 DECLARE_OPTIONAL_REGION(watchdog_tombstone)
+DECLARE_OPTIONAL_REGION(earlyram_stack)
 
 /* Returns true when pre-RAM symbols are known to the linker.
  * (Does not necessarily mean that the memory is accessible.) */