Skip to content

Commit ddbb20d

Browse files
mcsprhasenradball
authored andcommitted
Nice stack smashing postmortem message (esp8266#8670)
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. esp8266#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
1 parent 72449e1 commit ddbb20d

7 files changed

+36
-27
lines changed

cores/esp8266/StackThunk.cpp

+6-5
Original file line numberDiff line numberDiff line change
@@ -142,11 +142,12 @@ void stack_thunk_dump_stack()
142142
ets_printf("<<<stack<<<\n");
143143
}
144144

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

152-
};
153+
}

cores/esp8266/StackThunk.h

+2-2
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ extern uint32_t stack_thunk_get_stack_bot();
4141
extern uint32_t stack_thunk_get_cont_sp();
4242
extern uint32_t stack_thunk_get_max_usage();
4343
extern void stack_thunk_dump_stack();
44-
extern void stack_thunk_fatal_overflow();
44+
extern void stack_thunk_fatal_smashing();
4545

4646
// Globals required for thunking operation
4747
extern uint32_t *stack_thunk_ptr;
@@ -75,7 +75,7 @@ thunk_"#fcnToThunk":\n\
7575
l32i.n a15, a15, 0 /* A15 now has contents of last stack entry */\n\
7676
l32r a0, .LC_STACK_VALUE"#fcnToThunk" /* A0 now has the check value */\n\
7777
beq a0, a15, .L1"#fcnToThunk"\n\
78-
call0 stack_thunk_fatal_overflow\n\
78+
call0 stack_thunk_fatal_smashing\n\
7979
.L1"#fcnToThunk":\n\
8080
movi a15, stack_thunk_save /* Restore A1(SP) */\n\
8181
l32i.n a1, a15, 0\n\

cores/esp8266/cont.h

+2-3
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,8 @@ void cont_run(cont_t*, void (*pfn)(void));
6262
// execution state (registers and stack)
6363
void cont_suspend(cont_t*);
6464

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

6968
// Go through stack and check how many bytes are most probably still unchanged
7069
// and thus weren't used by the user code. i.e. that stack space is free. (high water mark)

cores/esp8266/cont_util.cpp

+16-7
Original file line numberDiff line numberDiff line change
@@ -18,14 +18,18 @@
1818
Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
1919
*/
2020

21-
#include "cont.h"
21+
#include <ets_sys.h>
22+
2223
#include <stddef.h>
2324
#include <string.h>
24-
#include "ets_sys.h"
2525

26-
extern "C" {
26+
#include "cont.h"
27+
#include "debug.h"
2728

28-
#define CONT_STACKGUARD 0xfeefeffe
29+
extern "C"
30+
{
31+
32+
static constexpr unsigned int CONT_STACKGUARD { 0xfeefeffe };
2933

3034
void cont_init(cont_t* cont) {
3135
memset(cont, 0, sizeof(cont_t));
@@ -42,10 +46,15 @@ void cont_init(cont_t* cont) {
4246
}
4347
}
4448

45-
int IRAM_ATTR cont_check(cont_t* cont) {
46-
if(cont->stack_guard1 != CONT_STACKGUARD || cont->stack_guard2 != CONT_STACKGUARD) return 1;
49+
void IRAM_ATTR cont_check(cont_t* cont) {
50+
if ((cont->stack_guard1 == CONT_STACKGUARD)
51+
&& (cont->stack_guard2 == CONT_STACKGUARD))
52+
{
53+
return;
54+
}
4755

48-
return 0;
56+
__stack_chk_fail();
57+
__builtin_unreachable();
4958
}
5059

5160
// No need for this to be in IRAM, not expected to be IRQ called

cores/esp8266/core_esp8266_main.cpp

+3-3
Original file line numberDiff line numberDiff line change
@@ -245,21 +245,21 @@ static void loop_wrapper() {
245245
}
246246
loop();
247247
loop_end();
248+
cont_check(g_pcont);
248249
if (serialEventRun) {
249250
serialEventRun();
250251
}
251252
esp_schedule();
252253
}
253254

255+
extern "C" void __stack_chk_fail(void);
256+
254257
static void loop_task(os_event_t *events) {
255258
(void) events;
256259
s_cycles_at_resume = ESP.getCycleCount();
257260
ESP.resetHeap();
258261
cont_run(g_pcont, &loop_wrapper);
259262
ESP.setDramHeap();
260-
if (cont_check(g_pcont) != 0) {
261-
panic();
262-
}
263263
}
264264

265265
extern "C" {

cores/esp8266/core_esp8266_postmortem.cpp

+6-6
Original file line numberDiff line numberDiff line change
@@ -42,9 +42,12 @@ static int s_panic_line = 0;
4242
static const char* s_panic_func = 0;
4343
static const char* s_panic_what = 0;
4444

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

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

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

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

320321
__wrap_system_restart_local();
321322

322-
while (1); // never reached, needed to satisfy "noreturn" attribute
323+
__builtin_unreachable(); // never reached, needed to satisfy "noreturn" attribute
323324
}
324325

325-
326-
};
326+
} // extern "C"

cores/esp8266/debug.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ void hexdump(const void* mem, uint32_t len, uint8_t cols);
2626
extern "C"
2727
{
2828
#endif
29-
29+
void __stack_chk_fail(void) __attribute__((noreturn));
3030
void __unhandled_exception(const char* str) __attribute__((noreturn));
3131
void __panic_func(const char* file, int line, const char* func) __attribute__((noreturn));
3232
#define panic() __panic_func(PSTR(__FILE__), __LINE__, __func__)

0 commit comments

Comments
 (0)