فهرست منبع

espcoredump: Fix bugs related to (fake) stacks

Add support to tasks stacks in RTC DRAM. Before this fix, any stack
in RTC DRAM would have been considered as corrupted, whichi is not
the case.
Fix a bug related to wrong parameters passed to esp_core_dump_get_stack.
Fix a bug reading fake stack memory, triggering a memory violation.

* Closes https://github.com/espressif/esp-idf/issues/6751
* Merges https://github.com/espressif/esp-idf/pull/6750
Omar Chebib 4 سال پیش
والد
کامیت
e533431095

+ 2 - 0
components/espcoredump/component.mk

@@ -4,9 +4,11 @@ COMPONENT_PRIV_INCLUDEDIRS := include_core_dump
 COMPONENT_ADD_LDFRAGMENTS += linker.lf
 
 ifdef CONFIG_IDF_TARGET_ARCH_XTENSA
+	COMPONENT_SRCDIRS += src/port/xtensa
 	COMPONENT_PRIV_INCLUDEDIRS += include_core_dump/port/xtensa
 endif
 
 ifdef CONFIG_IDF_TARGET_ARCH_RISCV
+	COMPONENT_SRCDIRS += src/port/riscv
 	COMPONENT_PRIV_INCLUDEDIRS += include_core_dump/port/riscv
 endif

+ 4 - 1
components/espcoredump/src/core_dump_checksum.c

@@ -128,7 +128,10 @@ uint32_t esp_core_dump_checksum_finish(core_dump_checksum_ctx* cks_ctx, core_dum
 
 #endif
 
-    ESP_COREDUMP_LOG_PROCESS("Total length of hashed data: %d!", cks_ctx->total_bytes_checksum);
+    if (cks_ctx) {
+        ESP_COREDUMP_LOG_PROCESS("Total length of hashed data: %d", cks_ctx->total_bytes_checksum);
+    }
+
     return chs_len;
 }
 

+ 1 - 1
components/espcoredump/src/core_dump_elf.c

@@ -281,7 +281,7 @@ static int elf_add_stack(core_dump_elf_t *self, core_dump_task_header_t *task)
 
     ELF_CHECK_ERR((task), ELF_PROC_ERR_OTHER, "Invalid task pointer.");
 
-    stack_len = esp_core_dump_get_stack(task, &stack_paddr, &stack_vaddr);
+    stack_len = esp_core_dump_get_stack(task, &stack_vaddr, &stack_paddr);
     ESP_COREDUMP_LOG_PROCESS("Add stack for task 0x%x: addr 0x%x, sz %u",
                                 task->tcb_addr, stack_vaddr, stack_len);
     int ret = elf_add_segment(self, PT_LOAD,

+ 9 - 3
components/espcoredump/src/port/riscv/core_dump_port.c

@@ -227,8 +227,14 @@ uint32_t esp_core_dump_get_isr_stack_end(void)
  */
 static inline bool esp_core_dump_task_stack_end_is_sane(uint32_t sp)
 {
-    //TODO: currently core dump supports stacks in DRAM only, external SRAM not supported yet
-    return esp_ptr_in_dram((void *)sp);
+    return esp_ptr_in_dram((void *)sp)
+#if CONFIG_SPIRAM_ALLOW_STACK_EXTERNAL_MEMORY
+        || esp_stack_ptr_in_extram(sp)
+#endif
+#if CONFIG_ESP_SYSTEM_ALLOW_RTC_FAST_MEM_AS_HEAP
+        || esp_ptr_in_rtc_dram_fast((void*) sp)
+#endif
+    ;
 }
 
 bool esp_core_dump_check_stack(core_dump_task_header_t *task)
@@ -329,7 +335,7 @@ uint32_t esp_core_dump_get_task_regs_dump(core_dump_task_header_t *task, void **
 
     ESP_COREDUMP_LOG_PROCESS("Add regs for task 0x%x", task->tcb_addr);
 
-    stack_len = esp_core_dump_get_stack(task, &stack_paddr, &stack_vaddr);
+    stack_len = esp_core_dump_get_stack(task, &stack_vaddr, &stack_paddr);
 
     if (stack_len < sizeof(RvExcFrame)) {
         ESP_COREDUMP_LOGE("Too small stack to keep frame: %d bytes!", stack_len);

+ 36 - 25
components/espcoredump/src/port/xtensa/core_dump_port.c

@@ -299,8 +299,14 @@ uint8_t* esp_core_dump_get_isr_stack_top(void) {
 
 static inline bool esp_core_dump_task_stack_end_is_sane(uint32_t sp)
 {
-    //TODO: currently core dump supports stacks in DRAM only, external SRAM not supported yet
-    return esp_ptr_in_dram((void *)sp);
+    return esp_ptr_in_dram((void *)sp)
+#if CONFIG_SPIRAM_ALLOW_STACK_EXTERNAL_MEMORY
+        || esp_stack_ptr_in_extram(sp)
+#endif
+#if CONFIG_ESP_SYSTEM_ALLOW_RTC_FAST_MEM_AS_HEAP
+        || esp_ptr_in_rtc_dram_fast((void*) sp)
+#endif
+    ;
 }
 
 
@@ -387,28 +393,31 @@ bool esp_core_dump_check_task(core_dump_task_header_t *task)
                                             task->tcb_addr,
                                             task->stack_start,
                                             task->stack_end);
-    }
-    XtSolFrame *sol_frame = (XtSolFrame *)task->stack_start;
-    if (sol_frame->exit == 0) {
-        ESP_COREDUMP_LOG_PROCESS("Task (TCB:%x), EXIT/PC/PS/A0/SP %x %x %x %x %x",
-                                    task->tcb_addr,
-                                    sol_frame->exit,
-                                    sol_frame->pc,
-                                    sol_frame->ps,
-                                    sol_frame->a0,
-                                    sol_frame->a1);
     } else {
-// to avoid warning that 'exc_frame' is unused when ESP_COREDUMP_LOG_PROCESS does nothing
-#if CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH
-        XtExcFrame *exc_frame = (XtExcFrame *)task->stack_start;
-        ESP_COREDUMP_LOG_PROCESS("Task (TCB:%x) EXIT/PC/PS/A0/SP %x %x %x %x %x",
-                                    task->tcb_addr,
-                                    exc_frame->exit,
-                                    exc_frame->pc,
-                                    exc_frame->ps,
-                                    exc_frame->a0,
-                                    exc_frame->a1);
-#endif
+        /* This shall be done only if the stack was correct, else, stack_start
+         * would point to a fake address. */
+        XtSolFrame *sol_frame = (XtSolFrame *)task->stack_start;
+        if (sol_frame->exit == 0) {
+            ESP_COREDUMP_LOG_PROCESS("Task (TCB:%x), EXIT/PC/PS/A0/SP %x %x %x %x %x",
+                                        task->tcb_addr,
+                                        sol_frame->exit,
+                                        sol_frame->pc,
+                                        sol_frame->ps,
+                                        sol_frame->a0,
+                                        sol_frame->a1);
+        } else {
+    // to avoid warning that 'exc_frame' is unused when ESP_COREDUMP_LOG_PROCESS does nothing
+    #if CONFIG_ESP_COREDUMP_ENABLE_TO_FLASH
+            XtExcFrame *exc_frame = (XtExcFrame *)task->stack_start;
+            ESP_COREDUMP_LOG_PROCESS("Task (TCB:%x) EXIT/PC/PS/A0/SP %x %x %x %x %x",
+                                        task->tcb_addr,
+                                        exc_frame->exit,
+                                        exc_frame->pc,
+                                        exc_frame->ps,
+                                        exc_frame->a0,
+                                        exc_frame->a1);
+    #endif
+        }
     }
     return true;
 }
@@ -420,12 +429,14 @@ bool esp_core_dump_check_task(core_dump_task_header_t *task)
  */
 uint32_t esp_core_dump_get_task_regs_dump(core_dump_task_header_t *task, void **reg_dump)
 {
-    uint32_t stack_vaddr, stack_paddr, stack_len;
+    uint32_t stack_vaddr = 0;
+    uint32_t stack_paddr = 0;
+    uint32_t stack_len = 0;
     static xtensa_elf_reg_dump_t s_reg_dump = { 0 };
 
     ESP_COREDUMP_DEBUG_ASSERT(task != NULL && reg_dump != NULL);
 
-    stack_len = esp_core_dump_get_stack(task, &stack_paddr, &stack_vaddr);
+    stack_len = esp_core_dump_get_stack(task, &stack_vaddr, &stack_paddr);
 
     ESP_COREDUMP_LOG_PROCESS("Add regs for task 0x%x", task->tcb_addr);
 

+ 6 - 3
components/soc/include/soc/soc_memory_layout.h

@@ -293,9 +293,12 @@ inline static bool IRAM_ATTR esp_stack_ptr_in_extram(uint32_t sp)
 
 inline static bool IRAM_ATTR esp_stack_ptr_is_sane(uint32_t sp)
 {
+    return esp_stack_ptr_in_dram(sp)
 #if CONFIG_SPIRAM_ALLOW_STACK_EXTERNAL_MEMORY
-    return (esp_stack_ptr_in_dram(sp) || esp_stack_ptr_in_extram(sp));
-#else
-    return esp_stack_ptr_in_dram(sp);
+        || esp_stack_ptr_in_extram(sp)
+#endif
+#if CONFIG_ESP_SYSTEM_ALLOW_RTC_FAST_MEM_AS_HEAP
+        || esp_ptr_in_rtc_dram_fast((void*) sp)
 #endif
+        ;
 }