From c3dee9eaba929babe08b73ae8eb149928568737d Mon Sep 17 00:00:00 2001 From: Patrick Rudolph Date: Thu, 12 Dec 2024 10:42:17 +0100 Subject: [PATCH] cpu/intel/car/romstage: Fix false-positive stack corruption Fix regression introduced in commit 03518727313119a8c7a273eb745663c99f8e4d22 ("arch/x86: Add breakpoint to stack canary"). romstage_main writes to the stack-canary, but since that's expected temporarily disable the breakpoint. This only caused a warning on platforms that do select IDT_IN_EVERY_STAGE, since those install the stack canary breakpoint. TEST: No more exceptions are printed in romstage when IDT_IN_EVERY_STAGE is enabled. Change-Id: I7ebf0a5e8eaad49af77ab4d5f6b58fc849013b14 Signed-off-by: Patrick Rudolph Reviewed-on: https://review.coreboot.org/c/coreboot/+/85568 Tested-by: build bot (Jenkins) Reviewed-by: Shuo Liu Reviewed-by: Lean Sheng Tan Reviewed-by: Angel Pons --- src/cpu/intel/car/romstage.c | 18 ++++++++++++++++++ 1 file changed, 18 insertions(+) diff --git a/src/cpu/intel/car/romstage.c b/src/cpu/intel/car/romstage.c index 7df512dce0..63a83abbd7 100644 --- a/src/cpu/intel/car/romstage.c +++ b/src/cpu/intel/car/romstage.c @@ -3,6 +3,7 @@ #include #include #include +#include #include #include #include @@ -35,9 +36,26 @@ void __noreturn romstage_main(void) stack_base = (u32 *)(_ecar_stack - size); + /* Disable breakpoint since stack is intentionally corrupted */ + stack_canary_breakpoint_remove(); + + /* + * The "stack guard" and the "stack canary breakpoint" can both detect excessive + * stack usage. Excessive stack usage can corrupt data and lead to undefined + * (and thus hard to debug) behaviour. + * The stack guard will be checked later on, assuming the corruption wasn't to + * severe and allowed romstage to run. It's useful to detect problems when + * HW breakpoints were disabled. + * + * When HW breakpoints are used and enabled, the stack canary breakpoint will + * report the instruction pointer immediately, which can hint at which function + * may be using too much stack. FSP might disable HW breakpoints, though. + */ for (i = 0; i < num_guards; i++) stack_base[i] = stack_guard; + stack_canary_breakpoint_init(); + if (CONFIG(VBOOT_EARLY_EC_SYNC)) vboot_sync_ec();