Add stack validity check and raise an error when it happens.

tannewt · tannewt · commit 7ad2e6ace34a · 2018-12-04T23:26:04.000-08:00
The backtrace cannot be given because it relies on the validity
of the qstr data structures on the heap which may have been
corrupted.

In fact, it still can crash hard when the bytecode itself is
overwritten. To fix, we'd need a way to skip gathering the
backtrace completely.

This also increases the default stack size on M4s so it can
accomodate the stack needed by ASF4s nvm API.
diff --git a/main.c b/main.c
@@ -220,8 +220,8 @@ bool run_code_py(safe_mode_t safe_mode) {
     rgb_status_animation_t animation;
     prep_rgb_status_animation(&result, found_main, safe_mode, &animation);
     while (true) {
-        #ifdef MICROPY_VM_HOOK_LOOP
-            MICROPY_VM_HOOK_LOOP
+        #ifdef CIRCUITPY_SUPERVISOR_BACKGROUND
+            CIRCUITPY_SUPERVISOR_BACKGROUND
         #endif
         if (reload_requested) {
             return true;
diff --git a/ports/atmel-samd/background.c b/ports/atmel-samd/background.c
@@ -29,18 +29,23 @@
 #include "tick.h"
 #include "supervisor/usb.h"
 
+#include "py/runtime.h"
 #include "shared-module/displayio/__init__.h"
 #include "shared-module/network/__init__.h"
+#include "supervisor/shared/stack.h"
 
 volatile uint64_t last_finished_tick = 0;
 
+bool stack_ok_so_far = true;
+
 void run_background_tasks(void) {
     #if (defined(SAMD21) && defined(PIN_PA02)) || defined(SAMD51)
     audio_dma_background();
     #endif
     #ifdef CIRCUITPY_DISPLAYIO
     displayio_refresh_display();
     #endif
+
     #if MICROPY_PY_NETWORK
     network_module_background();
     #endif
@@ -49,6 +54,12 @@ void run_background_tasks(void) {
     last_finished_tick = ticks_ms;
 }
 
+void run_background_vm_tasks(void) {
+    assert_heap_ok();
+    run_background_tasks();
+    assert_heap_ok();
+}
+
 bool background_tasks_ok(void) {
     return ticks_ms - last_finished_tick < 1000;
 }
diff --git a/ports/atmel-samd/background.h b/ports/atmel-samd/background.h
@@ -30,6 +30,7 @@
 #include <stdbool.h>
 
 void run_background_tasks(void);
+void run_background_vm_tasks(void);
 bool background_tasks_ok(void);
 
 #endif  // MICROPY_INCLUDED_ATMEL_SAMD_BACKGROUND_H
diff --git a/ports/atmel-samd/common-hal/nvm/ByteArray.c b/ports/atmel-samd/common-hal/nvm/ByteArray.c
@@ -28,6 +28,8 @@
 
 #include "hal_flash.h"
 
+#include "supervisor/shared/stack.h"
+
 #include <stdint.h>
 #include <string.h>
 
@@ -42,6 +44,7 @@ bool common_hal_nvm_bytearray_set_bytes(nvm_bytearray_obj_t *self,
     struct flash_descriptor desc;
     desc.dev.hw = NVMCTRL;
     flash_write(&desc, (uint32_t) self->start_address + start_index, values, len);
+    assert_heap_ok();
     return true;
 }
 
diff --git a/ports/atmel-samd/mpconfigport.h b/ports/atmel-samd/mpconfigport.h
@@ -182,7 +182,7 @@ typedef long mp_off_t;
 #define MICROPY_PY_SYS_PLATFORM                     "MicroChip SAMD51"
 #define PORT_HEAP_SIZE                              (0x20000) // 128KiB
 #define SPI_FLASH_MAX_BAUDRATE 24000000
-#define CIRCUITPY_DEFAULT_STACK_SIZE                8192
+#define CIRCUITPY_DEFAULT_STACK_SIZE                0x6000
 #define MICROPY_CPYTHON_COMPAT                      (1)
 #define MICROPY_MODULE_WEAK_LINKS                   (1)
 #define MICROPY_PY_BUILTINS_NOTIMPLEMENTED          (1)
@@ -432,10 +432,6 @@ extern const struct _mp_obj_module_t wiznet_module;
 
 #define MP_STATE_PORT MP_STATE_VM
 
-void run_background_tasks(void);
-#define MICROPY_VM_HOOK_LOOP run_background_tasks();
-#define MICROPY_VM_HOOK_RETURN run_background_tasks();
-
 #include "peripherals/samd/dma.h"
 
 #include "supervisor/flash_root_pointers.h"
@@ -454,6 +450,11 @@ void run_background_tasks(void);
     mp_obj_t gamepad_singleton; \
     NETWORK_ROOT_POINTERS \
 
+void run_background_tasks(void);
+void run_background_vm_tasks(void);
+#define MICROPY_VM_HOOK_LOOP run_background_vm_tasks();
+#define MICROPY_VM_HOOK_RETURN run_background_vm_tasks();
+#define CIRCUITPY_SUPERVISOR_BACKGROUND run_background_tasks();
 
 #define CIRCUITPY_AUTORELOAD_DELAY_MS 500
 #define CIRCUITPY_BOOT_OUTPUT_FILE "/boot_out.txt"
diff --git a/ports/atmel-samd/supervisor/port.c b/ports/atmel-samd/supervisor/port.c
@@ -80,7 +80,7 @@ extern volatile bool mp_msc_enabled;
 #define TRACE_BUFFER_SIZE (1 << (TRACE_BUFFER_MAGNITUDE_PACKETS + 1))
 // Size in bytes. 4 bytes per uint32_t.
 #define TRACE_BUFFER_SIZE_BYTES (TRACE_BUFFER_SIZE << 2)
-__attribute__((__aligned__(TRACE_BUFFER_SIZE_BYTES))) uint32_t mtb[TRACE_BUFFER_SIZE];
+__attribute__((__aligned__(TRACE_BUFFER_SIZE_BYTES))) uint32_t mtb[TRACE_BUFFER_SIZE] = {0};
 #endif
 
 safe_mode_t port_init(void) {
@@ -285,6 +285,19 @@ void reset_to_bootloader(void) {
  */
 __attribute__((used)) void HardFault_Handler(void)
 {
+#ifdef ENABLE_MICRO_TRACE_BUFFER
+    // Turn off the micro trace buffer so we don't fill it up in the infinite
+    // loop below.
+    REG_MTB_MASTER = 0x00000000 + 6;
+#endif
+#ifdef CIRCUITPY_CANARY_WORD
+    // If the canary is intact, then kill it and reset so we have a chance to
+    // read our files.
+    if (_ezero == CIRCUITPY_CANARY_WORD) {
+        _ezero = CIRCUITPY_SAFE_RESTART_WORD;
+        NVIC_SystemReset();
+    }
+#endif
     while (true) {
         asm("");
     }
diff --git a/py/obj.c b/py/obj.c
@@ -33,10 +33,12 @@
 #include "py/objtype.h"
 #include "py/objint.h"
 #include "py/objstr.h"
+#include "py/qstr.h"
 #include "py/runtime.h"
 #include "py/stackctrl.h"
 #include "py/stream.h" // for mp_obj_print
 
+#include "supervisor/shared/stack.h"
 #include "supervisor/shared/translate.h"
 
 mp_obj_type_t *mp_obj_get_type(mp_const_obj_t o_in) {
@@ -81,7 +83,7 @@ void mp_obj_print(mp_obj_t o_in, mp_print_kind_t kind) {
 
 // helper function to print an exception with traceback
 void mp_obj_print_exception(const mp_print_t *print, mp_obj_t exc) {
-    if (mp_obj_is_exception_instance(exc)) {
+    if (mp_obj_is_exception_instance(exc) && stack_ok()) {
         size_t n, *values;
         mp_obj_exception_get_traceback(exc, &n, &values);
         if (n > 0) {
diff --git a/supervisor/shared/stack.c b/supervisor/shared/stack.c
@@ -27,6 +27,7 @@
 #include "stack.h"
 
 #include "py/mpconfig.h"
+#include "py/runtime.h"
 #include "supervisor/cpu.h"
 
 extern uint32_t _estack;
@@ -37,6 +38,8 @@ supervisor_allocation* stack_alloc = NULL;
 
 #define EXCEPTION_STACK_SIZE 1024
 
+#define STACK_CANARY_VALUE 0x017829ef
+
 void allocate_stack(void) {
     mp_uint_t regs[10];
     mp_uint_t sp = cpu_get_regs_and_sp(regs);
@@ -50,6 +53,19 @@ void allocate_stack(void) {
     } else {
         current_stack_size = next_stack_size;
     }
+    *stack_alloc->ptr = STACK_CANARY_VALUE;
+}
+
+inline bool stack_ok(void) {
+    return *stack_alloc->ptr == STACK_CANARY_VALUE;
+}
+
+inline void assert_heap_ok(void) {
+    if (!stack_ok()) {
+        asm("nop");
+        while(true) {}
+        mp_raise_RuntimeError(translate("Stack clobbered heap."));
+    }
 }
 
 void stack_init(void) {
@@ -58,6 +74,7 @@ void stack_init(void) {
 
 void stack_resize(void) {
     if (next_stack_size == current_stack_size) {
+        *stack_alloc->ptr = STACK_CANARY_VALUE;
         return;
     }
     free_memory(stack_alloc);
diff --git a/supervisor/shared/stack.h b/supervisor/shared/stack.h
@@ -37,5 +37,10 @@ void stack_init(void);
 void stack_resize(void);
 void set_next_stack_size(uint32_t size);
 uint32_t get_current_stack_size(void);
+bool stack_ok(void);
+
+// Use this after any calls into a library which may use a lot of stack. This will raise a Python
+// exception when the stack has likely overwritten a portio of the heap.
+void assert_heap_ok(void);
 
 #endif  // MICROPY_INCLUDED_SUPERVISOR_STACK_H
