From a573cfe58a4a9159626e7b265ab578f0716ffd62 Mon Sep 17 00:00:00 2001 From: Omar Chebib Date: Tue, 2 Mar 2021 14:17:24 +0800 Subject: [PATCH] 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 --- components/espcoredump/component.mk | 2 + .../espcoredump/src/core_dump_checksum.c | 5 +- components/espcoredump/src/core_dump_elf.c | 2 +- .../src/port/riscv/core_dump_port.c | 12 +++- .../src/port/xtensa/core_dump_port.c | 61 +++++++++++-------- .../soc/include/soc/soc_memory_layout.h | 9 ++- 6 files changed, 58 insertions(+), 33 deletions(-) diff --git a/components/espcoredump/component.mk b/components/espcoredump/component.mk index 64fff02566..8a2c1d439f 100644 --- a/components/espcoredump/component.mk +++ b/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 diff --git a/components/espcoredump/src/core_dump_checksum.c b/components/espcoredump/src/core_dump_checksum.c index a20c2ac53e..53d5b00440 100644 --- a/components/espcoredump/src/core_dump_checksum.c +++ b/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; } diff --git a/components/espcoredump/src/core_dump_elf.c b/components/espcoredump/src/core_dump_elf.c index 80e6034c6c..5c6bf1eb85 100644 --- a/components/espcoredump/src/core_dump_elf.c +++ b/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, diff --git a/components/espcoredump/src/port/riscv/core_dump_port.c b/components/espcoredump/src/port/riscv/core_dump_port.c index 898ddc3557..cbe20e0d4e 100644 --- a/components/espcoredump/src/port/riscv/core_dump_port.c +++ b/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); diff --git a/components/espcoredump/src/port/xtensa/core_dump_port.c b/components/espcoredump/src/port/xtensa/core_dump_port.c index de1953d96b..deb7f313b9 100644 --- a/components/espcoredump/src/port/xtensa/core_dump_port.c +++ b/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); diff --git a/components/soc/include/soc/soc_memory_layout.h b/components/soc/include/soc/soc_memory_layout.h index b967006c95..2da5b729f2 100644 --- a/components/soc/include/soc/soc_memory_layout.h +++ b/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 + ; }