Merge branch 'bugfix/core_dump_data_struct_ovf' into 'master'

Fixes core dump data overwriting

See merge request espressif/esp-idf!8611

components/espcoredump/include_core_dump/esp_core_dump_port.h

@@ -42,7 +42,7 @@ extern "C" {
 #define COREDUMP_TCB_SIZE   sizeof(StaticTask_t)
 
 // Gets RTOS tasks snapshot
-uint32_t esp_core_dump_get_tasks_snapshot(core_dump_task_header_t* const tasks,
+uint32_t esp_core_dump_get_tasks_snapshot(core_dump_task_header_t** const tasks,
                         const uint32_t snapshot_size);
 
 // Checks TCB consistency

components/espcoredump/src/core_dump_common.c

@@ -103,7 +103,7 @@ static esp_err_t esp_core_dump_save_mem_segment(core_dump_write_config_t* write_
 static esp_err_t esp_core_dump_write_binary(void *frame, core_dump_write_config_t *write_cfg)
 {
     esp_err_t err;
-    static core_dump_task_header_t tasks[CONFIG_ESP32_CORE_DUMP_MAX_TASKS_NUM];
+    static core_dump_task_header_t *tasks[CONFIG_ESP32_CORE_DUMP_MAX_TASKS_NUM];
     uint32_t task_num, tcb_sz = esp_core_dump_get_tcb_len();
     uint32_t data_len = 0, task_id;
     int curr_task_index = COREDUMP_CURR_TASK_NOT_FOUND;
@@ -116,7 +116,7 @@ static esp_err_t esp_core_dump_write_binary(void *frame, core_dump_write_config_
     // Verifies all tasks in the snapshot
     for (task_id = 0; task_id < task_num; task_id++) {
         bool is_current_task = false, stack_is_valid = false;
-        bool tcb_is_valid = esp_core_dump_check_task(frame, &tasks[task_id], &is_current_task, &stack_is_valid);
+        bool tcb_is_valid = esp_core_dump_check_task(frame, tasks[task_id], &is_current_task, &stack_is_valid);
         // Check if task tcb or stack is corrupted
         if (!tcb_is_valid || !stack_is_valid) {
             // If tcb or stack for task is corrupted count task as broken
@@ -126,26 +126,26 @@ static esp_err_t esp_core_dump_write_binary(void *frame, core_dump_write_config_
             curr_task_index = task_id; // save current crashed task index in the snapshot
             ESP_COREDUMP_LOG_PROCESS("Task #%d (TCB:%x) is first crashed task.",
                                         task_id,
-                                        tasks[task_id].tcb_addr);
+                                        tasks[task_id]->tcb_addr);
         }
         // Increase core dump size by task stack size
         uint32_t stk_vaddr, stk_len;
-        esp_core_dump_get_stack(&tasks[task_id], &stk_vaddr, &stk_len);
+        esp_core_dump_get_stack(tasks[task_id], &stk_vaddr, &stk_len);
         data_len += esp_core_dump_get_stack_len(stk_vaddr, stk_vaddr+stk_len);
         // Add tcb size
         data_len += (tcb_sz + sizeof(core_dump_task_header_t));
     }
 
     if (esp_core_dump_in_isr_context()) {
-        interrupted_task_stack.start = tasks[curr_task_index].stack_start;
-        interrupted_task_stack.size = esp_core_dump_get_stack_len(tasks[curr_task_index].stack_start, tasks[curr_task_index].stack_end);
+        interrupted_task_stack.start = tasks[curr_task_index]->stack_start;
+        interrupted_task_stack.size = esp_core_dump_get_stack_len(tasks[curr_task_index]->stack_start, tasks[curr_task_index]->stack_end);
         // size of the task's stack has been already taken into account, also addresses have also been checked
         data_len += sizeof(core_dump_mem_seg_header_t);
-        tasks[curr_task_index].stack_start = (uint32_t)frame;
-        tasks[curr_task_index].stack_end = esp_core_dump_get_isr_stack_end();
-        ESP_COREDUMP_LOG_PROCESS("Add ISR stack %lu to %lu", tasks[curr_task_index].stack_end - tasks[curr_task_index].stack_start, data_len);
+        tasks[curr_task_index]->stack_start = (uint32_t)frame;
+        tasks[curr_task_index]->stack_end = esp_core_dump_get_isr_stack_end();
+        ESP_COREDUMP_LOG_PROCESS("Add ISR stack %lu to %lu", tasks[curr_task_index]->stack_end - tasks[curr_task_index]->stack_start, data_len);
         // take into account size of the ISR stack
-        data_len += esp_core_dump_get_stack_len(tasks[curr_task_index].stack_start, tasks[curr_task_index].stack_end);
+        data_len += esp_core_dump_get_stack_len(tasks[curr_task_index]->stack_start, tasks[curr_task_index]->stack_end);
     }
 
     // Check if current task TCB is broken
@@ -193,10 +193,10 @@ static esp_err_t esp_core_dump_write_binary(void *frame, core_dump_write_config_
     }
 
     // Write first crashed task data first (not always first task in the snapshot)
-    err = esp_core_dump_save_task(write_cfg, &tasks[curr_task_index]);
+    err = esp_core_dump_save_task(write_cfg, tasks[curr_task_index]);
     if (err != ESP_OK) {
         ESP_COREDUMP_LOGE("Failed to save first crashed task #%d (TCB:%x), error=%d!",
-                            curr_task_index, tasks[curr_task_index].tcb_addr, err);
+                            curr_task_index, tasks[curr_task_index]->tcb_addr, err);
         return err;
     }
 
@@ -206,10 +206,10 @@ static esp_err_t esp_core_dump_write_binary(void *frame, core_dump_write_config_
         if (task_id == curr_task_index) {
             continue;
         }
-        err = esp_core_dump_save_task(write_cfg, &tasks[task_id]);
+        err = esp_core_dump_save_task(write_cfg, tasks[task_id]);
         if (err != ESP_OK) {
             ESP_COREDUMP_LOGE("Failed to save core dump task #%d (TCB:%x), error=%d!",
-                                    task_id, tasks[curr_task_index].tcb_addr, err);
+                                    task_id, tasks[curr_task_index]->tcb_addr, err);
             return err;
         }
     }

components/espcoredump/src/core_dump_elf.c

@@ -302,7 +302,7 @@ static int elf_add_tcb(core_dump_elf_t *self, core_dump_task_header_t *task)
 }
 
 // get index of current crashed task (not always first task in the snapshot)
-static int elf_get_current_task_index(core_dump_task_header_t *tasks,
+static int elf_get_current_task_index(core_dump_task_header_t** tasks,
                                         uint32_t task_num)
 {
     int task_id;
@@ -311,13 +311,13 @@ static int elf_get_current_task_index(core_dump_task_header_t *tasks,
 
     // get index of current crashed task (not always first task in the snapshot)
     for (task_id = 0; task_id < task_num; task_id++) {
-        bool tcb_is_valid = esp_core_dump_tcb_addr_is_sane((uint32_t)tasks[task_id].tcb_addr);
-        bool stack_is_valid = esp_core_dump_check_stack(tasks[task_id].stack_start, tasks[task_id].stack_end);
-        if (stack_is_valid && tcb_is_valid && curr_task_handle == tasks[task_id].tcb_addr) {
+        bool tcb_is_valid = esp_core_dump_tcb_addr_is_sane((uint32_t)tasks[task_id]->tcb_addr);
+        bool stack_is_valid = esp_core_dump_check_stack(tasks[task_id]->stack_start, tasks[task_id]->stack_end);
+        if (stack_is_valid && tcb_is_valid && curr_task_handle == tasks[task_id]->tcb_addr) {
             curr_task_index = task_id; // save current crashed task index in the snapshot
             ESP_COREDUMP_LOG_PROCESS("Task #%d, (TCB:%x) is current crashed task.",
                                         task_id,
-                                        tasks[task_id].tcb_addr);
+                                        tasks[task_id]->tcb_addr);
         }
     }
     return curr_task_index;
@@ -416,7 +416,7 @@ static int elf_process_note_segment(core_dump_elf_t *self, int notes_size)
 }
 
 static int elf_process_tasks_regs(core_dump_elf_t *self, void* frame,
-                                    core_dump_task_header_t* tasks,
+                                    core_dump_task_header_t** tasks,
                                     uint32_t task_num)
 {
     int len = 0;
@@ -428,7 +428,7 @@ static int elf_process_tasks_regs(core_dump_elf_t *self, void* frame,
     }
 
     // place current task dump first
-    int ret = elf_process_task_regdump(self, frame, &tasks[curr_task_index]);
+    int ret = elf_process_task_regdump(self, frame, tasks[curr_task_index]);
     if (self->elf_stage == ELF_STAGE_PLACE_HEADERS) {
         // when writing segments headers this function writes nothing
         ELF_CHECK_ERR((ret >= 0), ret, "Task #%d, PR_STATUS write failed, return (%d).", curr_task_index, ret);
@@ -444,7 +444,7 @@ static int elf_process_tasks_regs(core_dump_elf_t *self, void* frame,
         if (task_id == curr_task_index) {
             continue; // skip current task (already processed)
         }
-        ret = elf_process_task_regdump(self, frame, &tasks[task_id]);
+        ret = elf_process_task_regdump(self, frame, tasks[task_id]);
         if (self->elf_stage == ELF_STAGE_PLACE_HEADERS) {
             // when writing segments headers this function writes nothing
             ELF_CHECK_ERR((ret >= 0), ret, "Task #%d, PR_STATUS write failed, return (%d).", task_id, ret);
@@ -462,12 +462,12 @@ static int elf_process_tasks_regs(core_dump_elf_t *self, void* frame,
             // in this stage we can safely replace task's stack with IRQ's one
             // if task had corrupted stack it was replaced with fake one in HW dependent code called by elf_process_task_regdump()
             // in the "write data" stage registers from ISR's stack will be saved in PR_STATUS
-            self->interrupted_task.stack_start = tasks[curr_task_index].stack_start;
-            self->interrupted_task.stack_end = tasks[curr_task_index].stack_end;
+            self->interrupted_task.stack_start = tasks[curr_task_index]->stack_start;
+            self->interrupted_task.stack_end = tasks[curr_task_index]->stack_end;
             uint32_t isr_stk_end = esp_core_dump_get_isr_stack_end();
             ESP_COREDUMP_LOG_PROCESS("Add ISR stack %lu (%x - %x)", isr_stk_end - (uint32_t)frame, (uint32_t)frame, isr_stk_end);
-            tasks[curr_task_index].stack_start = (uint32_t)frame;
-            tasks[curr_task_index].stack_end = isr_stk_end;
+            tasks[curr_task_index]->stack_start = (uint32_t)frame;
+            tasks[curr_task_index]->stack_end = isr_stk_end;
         }
 
         // actually we write current task's stack here which was replaced by ISR's
@@ -479,7 +479,7 @@ static int elf_process_tasks_regs(core_dump_elf_t *self, void* frame,
 }
 
 static int elf_write_tasks_data(core_dump_elf_t *self, void* frame,
-                                core_dump_task_header_t* tasks,
+                                core_dump_task_header_t** tasks,
                                 uint32_t task_num)
 {
     int elf_len = 0;
@@ -496,11 +496,11 @@ static int elf_write_tasks_data(core_dump_elf_t *self, void* frame,
     // processes all task's stack data and writes segment data into partition
     // if flash configuration is set
     for (task_id = 0; task_id < task_num; task_id++) {
-        ret = elf_process_task_tcb(self, &tasks[task_id]);
+        ret = elf_process_task_tcb(self, tasks[task_id]);
         ELF_CHECK_ERR((ret > 0), ret,
                         "Task #%d, TCB write failed, return (%d).", task_id, ret);
         elf_len += ret;
-        ret = elf_process_task_stack(self, &tasks[task_id]);
+        ret = elf_process_task_stack(self, tasks[task_id]);
         ELF_CHECK_ERR((ret != ELF_PROC_ERR_WRITE_FAIL), ELF_PROC_ERR_WRITE_FAIL,
                         "Task #%d, stack write failed, return (%d).", task_id, ret);
         elf_len += ret;
@@ -546,7 +546,7 @@ static int elf_write_core_dump_info(core_dump_elf_t *self)
 }
 
 static int esp_core_dump_do_write_elf_pass(core_dump_elf_t *self, void* frame,
-                                            core_dump_task_header_t* tasks,
+                                            core_dump_task_header_t** tasks,
                                             uint32_t task_num)
 {
     int tot_len = 0;
@@ -574,7 +574,7 @@ static int esp_core_dump_do_write_elf_pass(core_dump_elf_t *self, void* frame,
 esp_err_t esp_core_dump_write_elf(void *frame, core_dump_write_config_t *write_cfg)
 {
     esp_err_t err = ESP_OK;
-    static core_dump_task_header_t tasks[CONFIG_ESP32_CORE_DUMP_MAX_TASKS_NUM];
+    static core_dump_task_header_t *tasks[CONFIG_ESP32_CORE_DUMP_MAX_TASKS_NUM];
     static core_dump_elf_t self;
     core_dump_header_t dump_hdr;
     uint32_t tcb_sz = COREDUMP_TCB_SIZE, task_num;

components/espcoredump/src/core_dump_port.c

@@ -275,13 +275,21 @@ inline bool esp_core_dump_tcb_addr_is_sane(uint32_t addr)
     return esp_core_dump_mem_seg_is_sane(addr, COREDUMP_TCB_SIZE);
 }
 
-uint32_t esp_core_dump_get_tasks_snapshot(core_dump_task_header_t* const tasks,
+uint32_t esp_core_dump_get_tasks_snapshot(core_dump_task_header_t** const tasks,
                         const uint32_t snapshot_size)
 {
+    static TaskSnapshot_t s_tasks_snapshots[CONFIG_ESP32_CORE_DUMP_MAX_TASKS_NUM];
     uint32_t tcb_sz; // unused
-    uint32_t task_num = (uint32_t)uxTaskGetSnapshotAll((TaskSnapshot_t*)tasks,
+
+    /* implying that TaskSnapshot_t extends core_dump_task_header_t by adding extra fields */
+    _Static_assert(sizeof(TaskSnapshot_t) >= sizeof(core_dump_task_header_t), "FreeRTOS task snapshot binary compatibility issue!");
+
+    uint32_t task_num = (uint32_t)uxTaskGetSnapshotAll(s_tasks_snapshots,
                                                          (UBaseType_t)snapshot_size,
                                                          (UBaseType_t*)&tcb_sz);
+    for (uint32_t i = 0; i < task_num; i++) {
+        tasks[i] = (core_dump_task_header_t *)&s_tasks_snapshots[i];
+    }
     return task_num;
 }
 

components/freertos/Kconfig

@@ -174,6 +174,8 @@ menu "FreeRTOS"
 
     config FREERTOS_ISR_STACKSIZE
         int "ISR stack size"
+        range 2100 32768 if ESP32_COREDUMP_DATA_FORMAT_ELF
+        default 2100 if ESP32_COREDUMP_DATA_FORMAT_ELF
         range 1536 32768
         default 1536
         help