Merge branch 'master' into wifioff

dok-net · web-flow · commit 2d1cb1cc9c90 · 2023-05-13T11:27:47.000+02:00
diff --git a/cores/esp8266/core_esp8266_postmortem.cpp b/cores/esp8266/core_esp8266_postmortem.cpp
@@ -181,7 +181,13 @@ static void postmortem_report(uint32_t sp_dump) {
             exccause, epc1, rst_info.epc2, rst_info.epc3, rst_info.excvaddr, rst_info.depc);
     }
     else if (rst_info.reason == REASON_SOFT_WDT_RST) {
-        ets_printf_P(PSTR("\nSoft WDT reset\n"));
+        ets_printf_P(PSTR("\nSoft WDT reset"));
+        const char infinite_loop[] = { 0x06, 0xff, 0xff };  // loop: j loop
+        if (0 == memcmp_P(infinite_loop, (PGM_VOID_P)rst_info.epc1, 3u)) {
+            // The SDK is riddled with these. They are usually preceded by an ets_printf.
+            ets_printf_P(PSTR(" - deliberate infinite loop detected"));
+        }
+        ets_putc('\n');
         ets_printf_P(PSTR("\nException (%d):\nepc1=0x%08x epc2=0x%08x epc3=0x%08x excvaddr=0x%08x depc=0x%08x\n"),
             rst_info.exccause, /* Address executing at time of Soft WDT level-1 interrupt */ rst_info.epc1, 0, 0, 0, 0);
     }
diff --git a/cores/esp8266/umm_malloc/Notes.h b/cores/esp8266/umm_malloc/Notes.h
@@ -341,5 +341,16 @@ Enhancement ideas:
       * For multiple Heaps builds, add a dedicated function that always reports
         DRAM results.
 
+
+  April 22, 2023
+
+  The umm_poison logic runs outside the UMM_CRITICAL_* umbrella. When interrupt
+  routines do alloc calls, it is possible to interrupt an in-progress allocation
+  just before the poison is set, with a new alloc request resulting in a false
+  "poison check fail" against the in-progress allocation. The SDK does mallocs
+  from ISRs. SmartConfig can illustrate this issue.
+
+  Move get_poisoned() within UMM_CRITICAL_* in umm_malloc() and umm_realloc().
+
 */
 #endif
diff --git a/cores/esp8266/umm_malloc/umm_local.c b/cores/esp8266/umm_malloc/umm_local.c
@@ -142,8 +142,11 @@ void *umm_poison_realloc_fl(void *ptr, size_t size, const char *file, int line)
 
     add_poison_size(&size);
     ret = umm_realloc(ptr, size);
-
-    ret = get_poisoned(ret, size);
+    /*
+      "get_poisoned" is now called from umm_realloc while still in a critical
+      section. Before umm_realloc returned, the pointer offset was adjusted to
+      the start of the requested buffer.
+    */
 
     return ret;
 }
diff --git a/cores/esp8266/umm_malloc/umm_malloc.cpp b/cores/esp8266/umm_malloc/umm_malloc.cpp
@@ -909,6 +909,8 @@ void *umm_malloc(size_t size) {
 
     ptr = umm_malloc_core(_context, size);
 
+    ptr = POISON_CHECK_SET_POISON(ptr, size);
+
     UMM_CRITICAL_EXIT(id_malloc);
 
     return ptr;
@@ -1068,7 +1070,7 @@ void *umm_realloc(void *ptr, size_t size) {
         //  Case 2 - block + next block fits EXACTLY
     } else if ((blockSize + nextBlockSize) == blocks) {
         DBGLOG_DEBUG("exact realloc using next block - %i\n", blocks);
-        umm_assimilate_up(c);
+        umm_assimilate_up(_context, c);
         STATS__FREE_BLOCKS_UPDATE(-nextBlockSize);
         blockSize += nextBlockSize;
 
@@ -1087,8 +1089,9 @@ void *umm_realloc(void *ptr, size_t size) {
         STATS__FREE_BLOCKS_UPDATE(-prevBlockSize);
         STATS__FREE_BLOCKS_ISR_MIN();
         blockSize += prevBlockSize;
+        POISON_CHECK_SET_POISON((void *)&UMM_DATA(c), size);  // Fix allocation so ISR poison check is good
         UMM_CRITICAL_SUSPEND(id_realloc);
-        memmove((void *)&UMM_DATA(c), ptr, curSize);
+        UMM_POISON_MEMMOVE((void *)&UMM_DATA(c), ptr, curSize);
         ptr = (void *)&UMM_DATA(c);
         UMM_CRITICAL_RESUME(id_realloc);
         //  Case 5 - prev block + block + next block fits
@@ -1108,8 +1111,9 @@ void *umm_realloc(void *ptr, size_t size) {
         #else
         blockSize += (prevBlockSize + nextBlockSize);
         #endif
+        POISON_CHECK_SET_POISON((void *)&UMM_DATA(c), size);
         UMM_CRITICAL_SUSPEND(id_realloc);
-        memmove((void *)&UMM_DATA(c), ptr, curSize);
+        UMM_POISON_MEMMOVE((void *)&UMM_DATA(c), ptr, curSize);
         ptr = (void *)&UMM_DATA(c);
         UMM_CRITICAL_RESUME(id_realloc);
 
@@ -1119,8 +1123,9 @@ void *umm_realloc(void *ptr, size_t size) {
         void *oldptr = ptr;
         if ((ptr = umm_malloc_core(_context, size))) {
             DBGLOG_DEBUG("realloc %i to a bigger block %i, copy, and free the old\n", blockSize, blocks);
+            POISON_CHECK_SET_POISON((void *)&UMM_DATA(c), size);
             UMM_CRITICAL_SUSPEND(id_realloc);
-            memcpy(ptr, oldptr, curSize);
+            UMM_POISON_MEMCPY(ptr, oldptr, curSize);
             UMM_CRITICAL_RESUME(id_realloc);
             umm_free_core(_context, oldptr);
         } else {
@@ -1181,8 +1186,9 @@ void *umm_realloc(void *ptr, size_t size) {
             blockSize = blocks;
             #endif
         }
+        POISON_CHECK_SET_POISON((void *)&UMM_DATA(c), size);
         UMM_CRITICAL_SUSPEND(id_realloc);
-        memmove((void *)&UMM_DATA(c), ptr, curSize);
+        UMM_POISON_MEMMOVE((void *)&UMM_DATA(c), ptr, curSize);
         ptr = (void *)&UMM_DATA(c);
         UMM_CRITICAL_RESUME(id_realloc);
     } else if (blockSize >= blocks) { // 2
@@ -1198,8 +1204,9 @@ void *umm_realloc(void *ptr, size_t size) {
         void *oldptr = ptr;
         if ((ptr = umm_malloc_core(_context, size))) {
             DBGLOG_DEBUG("realloc %d to a bigger block %d, copy, and free the old\n", blockSize, blocks);
+            POISON_CHECK_SET_POISON((void *)&UMM_DATA(c), size);
             UMM_CRITICAL_SUSPEND(id_realloc);
-            memcpy(ptr, oldptr, curSize);
+            UMM_POISON_MEMCPY(ptr, oldptr, curSize);
             UMM_CRITICAL_RESUME(id_realloc);
             umm_free_core(_context, oldptr);
         } else {
@@ -1223,8 +1230,9 @@ void *umm_realloc(void *ptr, size_t size) {
         void *oldptr = ptr;
         if ((ptr = umm_malloc_core(_context, size))) {
             DBGLOG_DEBUG("realloc %d to a bigger block %d, copy, and free the old\n", blockSize, blocks);
+            POISON_CHECK_SET_POISON((void *)&UMM_DATA(c), size);
             UMM_CRITICAL_SUSPEND(id_realloc);
-            memcpy(ptr, oldptr, curSize);
+            UMM_POISON_MEMCPY(ptr, oldptr, curSize);
             UMM_CRITICAL_RESUME(id_realloc);
             umm_free_core(_context, oldptr);
         } else {
@@ -1250,6 +1258,8 @@ void *umm_realloc(void *ptr, size_t size) {
 
     STATS__FREE_BLOCKS_MIN();
 
+    ptr = POISON_CHECK_SET_POISON(ptr, size);
+
     /* Release the critical section... */
     UMM_CRITICAL_EXIT(id_realloc);
 
@@ -1258,6 +1268,7 @@ void *umm_realloc(void *ptr, size_t size) {
 
 /* ------------------------------------------------------------------------ */
 
+#if !defined(UMM_POISON_CHECK) && !defined(UMM_POISON_CHECK_LITE)
 void *umm_calloc(size_t num, size_t item_size) {
     void *ret;
 
@@ -1273,6 +1284,7 @@ void *umm_calloc(size_t num, size_t item_size) {
 
     return ret;
 }
+#endif
 
 /* ------------------------------------------------------------------------ */
 
diff --git a/cores/esp8266/umm_malloc/umm_malloc_cfg.h b/cores/esp8266/umm_malloc/umm_malloc_cfg.h
@@ -618,6 +618,9 @@ extern bool  umm_poison_check(void);
 // Local Additions to better report location in code of the caller.
 void *umm_poison_realloc_fl(void *ptr, size_t size, const char *file, int line);
 void  umm_poison_free_fl(void *ptr, const char *file, int line);
+#define POISON_CHECK_SET_POISON(p, s) get_poisoned(p, s)
+#define UMM_POISON_SKETCH_PTR(p) ((void*)((uintptr_t)p + sizeof(UMM_POISONED_BLOCK_LEN_TYPE) + UMM_POISON_SIZE_BEFORE))
+#define UMM_POISON_SKETCH_PTRSZ(s) (s - sizeof(UMM_POISONED_BLOCK_LEN_TYPE) - UMM_POISON_SIZE_BEFORE  - UMM_POISON_SIZE_AFTER)
 #if defined(UMM_POISON_CHECK_LITE)
 /*
     * We can safely do individual poison checks at free and realloc and stay
@@ -637,8 +640,13 @@ void  umm_poison_free_fl(void *ptr, const char *file, int line);
 #else
 #define POISON_CHECK() 1
 #define POISON_CHECK_NEIGHBORS(c) do {} while (false)
+#define POISON_CHECK_SET_POISON(p, s) (p)
+#define UMM_POISON_SKETCH_PTR(p) (p)
+#define UMM_POISON_SKETCH_PTRSZ(s) (s)
 #endif
 
+#define UMM_POISON_MEMMOVE(t, p, s) memmove(UMM_POISON_SKETCH_PTR(t), UMM_POISON_SKETCH_PTR(p), UMM_POISON_SKETCH_PTRSZ(s))
+#define UMM_POISON_MEMCPY(t, p, s) memcpy(UMM_POISON_SKETCH_PTR(t), UMM_POISON_SKETCH_PTR(p), UMM_POISON_SKETCH_PTRSZ(s))
 
 #if defined(UMM_POISON_CHECK) || defined(UMM_POISON_CHECK_LITE)
 /*
diff --git a/cores/esp8266/umm_malloc/umm_poison.c b/cores/esp8266/umm_malloc/umm_poison.c
@@ -163,8 +163,11 @@ void *umm_poison_malloc(size_t size) {
     add_poison_size(&size);
 
     ret = umm_malloc(size);
-
-    ret = get_poisoned(ret, size);
+    /*
+      "get_poisoned" is now called from umm_malloc while still in a critical
+      section. Before umm_malloc returned, the pointer offset was adjusted to
+      the start of the requested buffer.
+    */
 
     return ret;
 }
@@ -177,17 +180,16 @@ void *umm_poison_calloc(size_t num, size_t item_size) {
     // Use saturated multiply.
     // Rely on umm_malloc to supply the fail response as needed.
     size_t size = umm_umul_sat(num, item_size);
+    size_t request_sz = size;
 
     add_poison_size(&size);
 
     ret = umm_malloc(size);
 
     if (NULL != ret) {
-        memset(ret, 0x00, size);
+        memset(ret, 0x00, request_sz);
     }
 
-    ret = get_poisoned(ret, size);
-
     return ret;
 }
 
@@ -200,8 +202,11 @@ void *umm_poison_realloc(void *ptr, size_t size) {
 
     add_poison_size(&size);
     ret = umm_realloc(ptr, size);
-
-    ret = get_poisoned(ret, size);
+    /*
+      "get_poisoned" is now called from umm_realloc while still in a critical
+      section. Before umm_realloc returned, the pointer offset was adjusted to
+      the start of the requested buffer.
+    */
 
     return ret;
 }
