[coreboot] New patch to review for coreboot: 831243e Implement stack overflow checking for the BSP
Stefan Reinauer (stefan.reinauer@coreboot.org)
gerrit at coreboot.org
Mon Jul 23 23:21:08 CEST 2012
Stefan Reinauer (stefan.reinauer at coreboot.org) just uploaded a new patch set to gerrit, which you can find at http://review.coreboot.org/1286
-gerrit
commit 831243ee6459d48974caa0adc739eef740377122
Author: Ronald G. Minnich <rminnich at chromium.org>
Date: Tue Jun 12 16:29:32 2012 -0700
Implement stack overflow checking for the BSP
Previous patches implemented stack overflow checking for the APs.
This patch builds on the BSP stack poisoning patch to implement
stack overflow checking for the BSP, and also prints out maximum
stack usage. It reveals that our 32K stack is ridiculously oversized,
especially now that the lzma decoder doesn't use a giant 16K on-stack
array.
Break the stack checking out into a separate function, which
we will later use for the APs.
CPU0: stack from 00180000 to 00188000:Lowest stack address 00187ad8
To test failure, change the DEADBEEF stack poison value in c_start.S
to something else. Then we should get an error like this:
Stack overrun on BSP.Increase stack from current 32768 bytes
CPU0: stack from 00180000 to 00188000:Lowest stack address 00180000
Separate the act of loading from the act of starting the payload. This
allows us better error management and reporting of stack use. Now we
see:
CPU0: stack from 00180000 to 00188000:Lowest stack address 00187ad8
Tested for both success and failure on Link. At the same time, feel free
to carefully check my manipulation of _estack.
Change-Id: Ibb09738b15ec6a5510ac81e45dd82756bfa5aac2
Signed-off-by: Ronald G. Minnich <rminnich at chromium.org>
---
src/boot/hardwaremain.c | 17 +++++++++++++-
src/boot/selfboot.c | 10 +-------
src/include/cbfs.h | 1 +
src/include/lib.h | 8 ++++++-
src/lib/Makefile.inc | 1 +
src/lib/stack.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++
6 files changed, 77 insertions(+), 11 deletions(-)
diff --git a/src/boot/hardwaremain.c b/src/boot/hardwaremain.c
index d78b859..bb7f264 100644
--- a/src/boot/hardwaremain.c
+++ b/src/boot/hardwaremain.c
@@ -35,6 +35,7 @@ it with the version available from LANL.
#include <boot/tables.h>
#include <boot/elf.h>
#include <cbfs.h>
+#include <lib.h>
#if CONFIG_HAVE_ACPI_RESUME
#include <arch/acpi.h>
#endif
@@ -143,7 +144,19 @@ void hardwaremain(int boot_complete)
lb_mem = write_tables();
timestamp_add_now(TS_LOAD_PAYLOAD);
- cbfs_load_payload(lb_mem, CONFIG_CBFS_PREFIX "/payload");
- printk(BIOS_ERR, "Boot failed.\n");
+
+ void *payload;
+ payload = cbfs_load_payload(lb_mem, CONFIG_CBFS_PREFIX "/payload");
+ if (! payload)
+ die("Could not find a payload\n");
+
+ printk(BIOS_DEBUG, "Got a payload\n");
+ /* Before we go off to run the payload, see if
+ * we stayed within our bounds.
+ */
+ checkstack(&_estack, 0);
+
+ selfboot(lb_mem, payload);
+ printk(BIOS_EMERG, "Boot failed");
}
diff --git a/src/boot/selfboot.c b/src/boot/selfboot.c
index 3c31023..fd5b382 100644
--- a/src/boot/selfboot.c
+++ b/src/boot/selfboot.c
@@ -494,7 +494,7 @@ static int load_self_segments(
return 1;
}
-static int selfboot(struct lb_memory *mem, struct cbfs_payload *payload)
+int selfboot(struct lb_memory *mem, struct cbfs_payload *payload)
{
u32 entry=0;
struct segment head;
@@ -532,13 +532,7 @@ void *cbfs_load_payload(struct lb_memory *lb_mem, const char *name)
struct cbfs_payload *payload;
payload = (struct cbfs_payload *)cbfs_find_file(name, CBFS_TYPE_PAYLOAD);
- if (payload == NULL)
- return (void *) -1;
- printk(BIOS_DEBUG, "Got a payload\n");
- selfboot(lb_mem, payload);
- printk(BIOS_EMERG, "SELFBOOT RETURNED!\n");
-
- return (void *) -1;
+ return payload;
}
diff --git a/src/include/cbfs.h b/src/include/cbfs.h
index 1483177..b33e932 100644
--- a/src/include/cbfs.h
+++ b/src/include/cbfs.h
@@ -57,5 +57,6 @@ void *cbfs_load_stage(const char *name);
int cbfs_execute_stage(const char *name);
void *cbfs_load_optionrom(u16 vendor, u16 device, void * dest);
int run_address(void *f);
+int selfboot(struct lb_memory *mem, struct cbfs_payload *payload);
#endif
diff --git a/src/include/lib.h b/src/include/lib.h
index ea09887..b2f38a8 100644
--- a/src/include/lib.h
+++ b/src/include/lib.h
@@ -21,7 +21,7 @@
#ifndef __LIB_H__
#define __LIB_H__
-
+#include <stdint.h>
#ifndef __ROMCC__ /* romcc doesn't support prototypes. */
#ifndef __PRE_RAM__ /* Conflicts with romcc_io.h */
@@ -40,6 +40,12 @@ void ram_check(unsigned long start, unsigned long stop);
int ram_check_nodie(unsigned long start, unsigned long stop);
void quick_ram_check(void);
+/* Defined in src/lib/stack.c */
+int checkstack(void *top_of_stack, int stacksize);
+
+/* currently defined by a ldscript */
+extern u8 _estack;
+
/* Defined in romstage.c */
#if CONFIG_CPU_AMD_GEODE_LX
void cache_as_ram_main(void);
diff --git a/src/lib/Makefile.inc b/src/lib/Makefile.inc
index 7c081ac..a3235a2 100644
--- a/src/lib/Makefile.inc
+++ b/src/lib/Makefile.inc
@@ -39,6 +39,7 @@ ramstage-y += version.c
ramstage-y += cbfs.c
ramstage-y += lzma.c
#ramstage-y += lzmadecode.c
+ramstage-y += stack.c
ramstage-y += gcc.c
ramstage-y += clog2.c
ramstage-y += cbmem.c
diff --git a/src/lib/stack.c b/src/lib/stack.c
new file mode 100644
index 0000000..3f04b63
--- /dev/null
+++ b/src/lib/stack.c
@@ -0,0 +1,51 @@
+/*
+This software and ancillary information (herein called SOFTWARE )
+called LinuxBIOS is made available under the terms described
+here. The SOFTWARE has been approved for release with associated
+LA-CC Number 00-34 . Unless otherwise indicated, this SOFTWARE has
+been authored by an employee or employees of the University of
+California, operator of the Los Alamos National Laboratory under
+Contract No. W-7405-ENG-36 with the U.S. Department of Energy. The
+U.S. Government has rights to use, reproduce, and distribute this
+SOFTWARE. The public may copy, distribute, prepare derivative works
+and publicly display this SOFTWARE without charge, provided that this
+Notice and any statement of authorship are reproduced on all copies.
+Neither the Government nor the University makes any warranty, express
+or implied, or assumes any liability or responsibility for the use of
+this SOFTWARE. If SOFTWARE is modified to produce derivative works,
+such modified SOFTWARE should be clearly marked, so as not to confuse
+it with the version available from LANL.
+ */
+/* Copyright 2000, Ron Minnich, Advanced Computing Lab, LANL
+ * rminnich at lanl.gov
+ */
+
+#include <lib.h>
+#include <console/console.h>
+
+int checkstack(void *top_of_stack, int core)
+{
+ int i;
+ u32 *stack = (u32 *) (top_of_stack - CONFIG_STACK_SIZE);
+
+ if (stack[0] != 0xDEADBEEF){
+ printk(BIOS_ERR, "Stack overrun on CPU%d."
+ "Increase stack from current %d bytes\n",
+ CONFIG_STACK_SIZE, core);
+ return -1;
+ }
+
+ for(i = 0; i < CONFIG_STACK_SIZE/sizeof(stack[0]); i++){
+ if (stack[i] == 0xDEADBEEF)
+ continue;
+ printk(BIOS_SPEW, "CPU%d: stack from %p to %p:",
+ core,
+ stack,
+ &stack[CONFIG_STACK_SIZE/sizeof(stack[0])]);
+ printk(BIOS_SPEW, "Lowest stack address %p\n", &stack[i]);
+ return -1;
+ }
+
+ return 0;
+
+}
More information about the coreboot
mailing list