Skip to content

Commit

Permalink
Nice stack smashing postmortem message (#8670)
Browse files Browse the repository at this point in the history
Wire everything that relies on stack smashing detection to call
`__stack_chk_fail()` (aka what libssp / ssp / stack-protector uses)
Expose it in our debugging header

Rename overflow -> smashing, as these are different things we are trying
to detect (meaning, that we check for things writing there, not some
kind of `alloca` issue or the way `-fstack-check` would have worked)
ref. #8666

`-fstack-protector` continues to work as it always did
CONT replaces `abort()`, also moves its check to the loop wrapper to
avoid dumping otherwise useless SYS context memory
StackThunk replaces a similar `abort()` call
  • Loading branch information
mcspr authored Oct 31, 2022
1 parent 5f94a60 commit bde8ef7
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 27 deletions.
11 changes: 6 additions & 5 deletions cores/esp8266/StackThunk.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,12 @@ void stack_thunk_dump_stack()
ets_printf("<<<stack<<<\n");
}

/* Called when the stack overflow is detected by a thunk. Main memory is corrupted at this point. Do not return. */
void stack_thunk_fatal_overflow()
/* Called when the stack overflow is detected by a thunk. Main memory is corrupted at this point.
* Do not return, use libssp-compatible function to notify postmortem and immediately reboot. */
void stack_thunk_fatal_smashing()
{
ets_printf("FATAL ERROR: BSSL stack overflow\n");
abort();
ets_printf("FATAL ERROR: BSSL stack smashing detected\n");
__stack_chk_fail();
}

};
}
4 changes: 2 additions & 2 deletions cores/esp8266/StackThunk.h
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ extern uint32_t stack_thunk_get_stack_bot();
extern uint32_t stack_thunk_get_cont_sp();
extern uint32_t stack_thunk_get_max_usage();
extern void stack_thunk_dump_stack();
extern void stack_thunk_fatal_overflow();
extern void stack_thunk_fatal_smashing();

// Globals required for thunking operation
extern uint32_t *stack_thunk_ptr;
Expand Down Expand Up @@ -75,7 +75,7 @@ thunk_"#fcnToThunk":\n\
l32i.n a15, a15, 0 /* A15 now has contents of last stack entry */\n\
l32r a0, .LC_STACK_VALUE"#fcnToThunk" /* A0 now has the check value */\n\
beq a0, a15, .L1"#fcnToThunk"\n\
call0 stack_thunk_fatal_overflow\n\
call0 stack_thunk_fatal_smashing\n\
.L1"#fcnToThunk":\n\
movi a15, stack_thunk_save /* Restore A1(SP) */\n\
l32i.n a1, a15, 0\n\
Expand Down
5 changes: 2 additions & 3 deletions cores/esp8266/cont.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,9 +62,8 @@ void cont_run(cont_t*, void (*pfn)(void));
// execution state (registers and stack)
void cont_suspend(cont_t*);

// Check guard bytes around the stack. Return 0 in case everything is ok,
// return 1 if guard bytes were overwritten.
int cont_check(cont_t* cont);
// Check guard bytes around the stack. Immediately panics on failure.
void cont_check(cont_t*);

// Go through stack and check how many bytes are most probably still unchanged
// and thus weren't used by the user code. i.e. that stack space is free. (high water mark)
Expand Down
23 changes: 16 additions & 7 deletions cores/esp8266/cont_util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,18 @@
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
*/

#include "cont.h"
#include <ets_sys.h>

#include <stddef.h>
#include <string.h>
#include "ets_sys.h"

extern "C" {
#include "cont.h"
#include "debug.h"

#define CONT_STACKGUARD 0xfeefeffe
extern "C"
{

static constexpr unsigned int CONT_STACKGUARD { 0xfeefeffe };

void cont_init(cont_t* cont) {
memset(cont, 0, sizeof(cont_t));
Expand All @@ -42,10 +46,15 @@ void cont_init(cont_t* cont) {
}
}

int IRAM_ATTR cont_check(cont_t* cont) {
if(cont->stack_guard1 != CONT_STACKGUARD || cont->stack_guard2 != CONT_STACKGUARD) return 1;
void IRAM_ATTR cont_check(cont_t* cont) {
if ((cont->stack_guard1 == CONT_STACKGUARD)
&& (cont->stack_guard2 == CONT_STACKGUARD))
{
return;
}

return 0;
__stack_chk_fail();
__builtin_unreachable();
}

// No need for this to be in IRAM, not expected to be IRQ called
Expand Down
6 changes: 3 additions & 3 deletions cores/esp8266/core_esp8266_main.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -245,21 +245,21 @@ static void loop_wrapper() {
}
loop();
loop_end();
cont_check(g_pcont);
if (serialEventRun) {
serialEventRun();
}
esp_schedule();
}

extern "C" void __stack_chk_fail(void);

static void loop_task(os_event_t *events) {
(void) events;
s_cycles_at_resume = ESP.getCycleCount();
ESP.resetHeap();
cont_run(g_pcont, &loop_wrapper);
ESP.setDramHeap();
if (cont_check(g_pcont) != 0) {
panic();
}
}

extern "C" {
Expand Down
12 changes: 6 additions & 6 deletions cores/esp8266/core_esp8266_postmortem.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,12 @@ static int s_panic_line = 0;
static const char* s_panic_func = 0;
static const char* s_panic_what = 0;

// Our wiring for abort() and C++ exceptions
static bool s_abort_called = false;
static const char* s_unhandled_exception = NULL;

// Common way to notify about where the stack smashing happened
// (but, **only** if caller uses our handler function)
static uint32_t s_stacksmash_addr = 0;

void abort() __attribute__((noreturn));
Expand Down Expand Up @@ -154,7 +157,7 @@ void __wrap_system_restart_local() {
ets_printf_P(PSTR("\nSoft WDT reset\n"));
}
else if (rst_info.reason == REASON_USER_STACK_SMASH) {
ets_printf_P(PSTR("\nStack overflow detected.\n"));
ets_printf_P(PSTR("\nStack smashing detected.\n"));
ets_printf_P(PSTR("\nException (%d):\nepc1=0x%08x epc2=0x%08x epc3=0x%08x excvaddr=0x%08x depc=0x%08x\n"),
5 /* Alloca exception, closest thing to stack fault*/, s_stacksmash_addr, 0, 0, 0, 0);
}
Expand Down Expand Up @@ -310,17 +313,14 @@ void __panic_func(const char* file, int line, const char* func) {
uintptr_t __stack_chk_guard = 0x08675309 ^ RANDOM_REG32;
void __stack_chk_fail(void) {
s_user_reset_reason = REASON_USER_STACK_SMASH;
ets_printf_P(PSTR("\nPANIC: Stack overrun"));

s_stacksmash_addr = (uint32_t)__builtin_return_address(0);

if (gdb_present())
__asm__ __volatile__ ("syscall"); // triggers GDB when enabled

__wrap_system_restart_local();

while (1); // never reached, needed to satisfy "noreturn" attribute
__builtin_unreachable(); // never reached, needed to satisfy "noreturn" attribute
}


};
} // extern "C"
2 changes: 1 addition & 1 deletion cores/esp8266/debug.h
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ void hexdump(const void* mem, uint32_t len, uint8_t cols);
extern "C"
{
#endif

void __stack_chk_fail(void) __attribute__((noreturn));
void __unhandled_exception(const char* str) __attribute__((noreturn));
void __panic_func(const char* file, int line, const char* func) __attribute__((noreturn));
#define panic() __panic_func(PSTR(__FILE__), __LINE__, __func__)
Expand Down

0 comments on commit bde8ef7

Please sign in to comment.