diff --git a/site_scons/cc.scons b/site_scons/cc.scons index 9960d350c28c..3a295ebd8f31 100644 --- a/site_scons/cc.scons +++ b/site_scons/cc.scons @@ -35,6 +35,7 @@ ENV.AppendUnique( "-fsingle-precision-constant", "-fno-math-errno", # Generates .su files with stack usage information + # Disabled in production - only enable for stack analysis # "-fstack-usage", "-g", ], diff --git a/targets/f7/src/update.c b/targets/f7/src/update.c index d15474e529d2..63280595d887 100644 --- a/targets/f7/src/update.c +++ b/targets/f7/src/update.c @@ -60,31 +60,48 @@ static bool flipper_update_init(void) { return flipper_update_mount_sd(); } +/** + * Load and verify boot loader stage from manifest + * + * FatFS structures (FIL, FILINFO) are heap-allocated to reduce stack usage + * from ~928 bytes to ~100 bytes. This is critical for the boot update path + * which must complete safely within the main thread's 2048-byte stack. + * See issue #4332 for stack overflow analysis. + */ static bool flipper_update_load_stage(const FuriString* work_dir, UpdateManifest* manifest) { - FIL file; - FILINFO stat; + FIL* file = (FIL*)malloc(sizeof(FIL)); // ~400 bytes - MUST be heap-allocated + FILINFO* stat = (FILINFO*)malloc(sizeof(FILINFO)); // ~80 bytes + + // Validate allocations to prevent cascade failures + if(!file || !stat) { + free(file); + free(stat); + return false; + } FuriString* loader_img_path; loader_img_path = furi_string_alloc_set(work_dir); path_append(loader_img_path, furi_string_get_cstr(manifest->staged_loader_file)); - if((f_stat(furi_string_get_cstr(loader_img_path), &stat) != FR_OK) || - (f_open(&file, furi_string_get_cstr(loader_img_path), FA_OPEN_EXISTING | FA_READ) != + if((f_stat(furi_string_get_cstr(loader_img_path), stat) != FR_OK) || + (f_open(file, furi_string_get_cstr(loader_img_path), FA_OPEN_EXISTING | FA_READ) != FR_OK) || - (stat.fsize == 0)) { + (stat->fsize == 0)) { furi_string_free(loader_img_path); + free(file); + free(stat); return false; } furi_string_free(loader_img_path); - void* img = malloc(stat.fsize); + void* img = malloc(stat->fsize); uint32_t read_total = 0; uint16_t read_current = 0; const uint16_t MAX_READ = 0xFFFF; uint32_t crc = 0; do { - if(f_read(&file, img + read_total, MAX_READ, &read_current) != FR_OK) { //-V769 + if(f_read(file, img + read_total, MAX_READ, &read_current) != FR_OK) { //-V769 break; } crc = crc32_calc_buffer(crc, img + read_total, read_current); @@ -92,7 +109,7 @@ static bool flipper_update_load_stage(const FuriString* work_dir, UpdateManifest } while(read_current == MAX_READ); do { - if((read_total != stat.fsize) || (crc != manifest->staged_loader_crc)) { + if((read_total != stat->fsize) || (crc != manifest->staged_loader_crc)) { break; } @@ -107,28 +124,49 @@ static bool flipper_update_load_stage(const FuriString* work_dir, UpdateManifest */ __disable_irq(); - memmove((void*)(SRAM1_BASE), img, stat.fsize); + memmove((void*)(SRAM1_BASE), img, stat->fsize); LL_SYSCFG_SetRemapMemory(LL_SYSCFG_REMAP_SRAM); furi_hal_switch(0x0); + free(file); + free(stat); return true; } while(false); free(img); + free(file); + free(stat); return false; } +/** + * Read manifest path from SD card boot pointer file + * + * FatFS structures and path buffer are heap-allocated to reduce stack usage + * from ~1160 bytes to ~50 bytes. This was the largest single stack consumer + * in the firmware, exceeding the original 1024-byte limit. Heap allocation + * is essential for boot path reliability. See issue #4332. + */ static bool flipper_update_get_manifest_path(FuriString* out_path) { - FIL file; - FILINFO stat; + FIL* file = (FIL*)malloc(sizeof(FIL)); // ~400 bytes + FILINFO* stat = (FILINFO*)malloc(sizeof(FILINFO)); // ~80 bytes + char* manifest_name_buf = (char*)malloc(UPDATE_OPERATION_MAX_MANIFEST_PATH_LEN); // 256 bytes uint16_t size_read = 0; - char manifest_name_buf[UPDATE_OPERATION_MAX_MANIFEST_PATH_LEN] = {0}; + // Critical: Validate all allocations to prevent OOM issues during boot + if(!file || !stat || !manifest_name_buf) { + free(file); + free(stat); + free(manifest_name_buf); + return false; + } + + memset(manifest_name_buf, 0, UPDATE_OPERATION_MAX_MANIFEST_PATH_LEN); furi_string_reset(out_path); - CHECK_FRESULT(f_stat(UPDATE_POINTER_FILE_PATH, &stat)); - CHECK_FRESULT(f_open(&file, UPDATE_POINTER_FILE_PATH, FA_OPEN_EXISTING | FA_READ)); + CHECK_FRESULT(f_stat(UPDATE_POINTER_FILE_PATH, stat)); + CHECK_FRESULT(f_open(file, UPDATE_POINTER_FILE_PATH, FA_OPEN_EXISTING | FA_READ)); do { - if(f_read(&file, manifest_name_buf, UPDATE_OPERATION_MAX_MANIFEST_PATH_LEN, &size_read) != + if(f_read(file, manifest_name_buf, UPDATE_OPERATION_MAX_MANIFEST_PATH_LEN, &size_read) != FR_OK) { break; } @@ -139,24 +177,42 @@ static bool flipper_update_get_manifest_path(FuriString* out_path) { furi_string_set(out_path, manifest_name_buf); furi_string_right(out_path, strlen(STORAGE_EXT_PATH_PREFIX)); } while(0); - f_close(&file); + f_close(file); + free(file); + free(stat); + free(manifest_name_buf); return !furi_string_empty(out_path); } +/** + * Parse and validate firmware update manifest + * + * FatFS structures are heap-allocated to reduce stack usage from ~920 bytes + * to ~80 bytes. These structures are large and not portable from the stack + * into deep call chains. Heap allocation provides safety margin for interrupt + * handlers in the critical boot update path. See issue #4332. + */ static UpdateManifest* flipper_update_process_manifest(const FuriString* manifest_path) { - FIL file; - FILINFO stat; + FIL* file = (FIL*)malloc(sizeof(FIL)); // ~400 bytes + FILINFO* stat = (FILINFO*)malloc(sizeof(FILINFO)); // ~80 bytes + + // Ensure allocations succeed before proceeding + if(!file || !stat) { + free(file); + free(stat); + return NULL; + } - CHECK_FRESULT(f_stat(furi_string_get_cstr(manifest_path), &stat)); - CHECK_FRESULT(f_open(&file, furi_string_get_cstr(manifest_path), FA_OPEN_EXISTING | FA_READ)); + CHECK_FRESULT(f_stat(furi_string_get_cstr(manifest_path), stat)); + CHECK_FRESULT(f_open(file, furi_string_get_cstr(manifest_path), FA_OPEN_EXISTING | FA_READ)); - uint8_t* manifest_data = malloc(stat.fsize); + uint8_t* manifest_data = malloc(stat->fsize); uint32_t bytes_read = 0; const uint16_t MAX_READ = 0xFFFF; do { uint16_t size_read = 0; - if(f_read(&file, manifest_data + bytes_read, MAX_READ, &size_read) != FR_OK) { //-V769 + if(f_read(file, manifest_data + bytes_read, MAX_READ, &size_read) != FR_OK) { //-V769 break; } bytes_read += size_read; @@ -164,7 +220,7 @@ static UpdateManifest* flipper_update_process_manifest(const FuriString* manifes UpdateManifest* manifest = NULL; do { - if(bytes_read != stat.fsize) { + if(bytes_read != stat->fsize) { break; } @@ -175,8 +231,10 @@ static UpdateManifest* flipper_update_process_manifest(const FuriString* manifes } } while(false); - f_close(&file); + f_close(file); free(manifest_data); + free(file); + free(stat); return manifest; } diff --git a/targets/f7/stm32wb55xx_flash.ld b/targets/f7/stm32wb55xx_flash.ld index c31aee863229..321c3ea529a6 100644 --- a/targets/f7/stm32wb55xx_flash.ld +++ b/targets/f7/stm32wb55xx_flash.ld @@ -3,7 +3,7 @@ ENTRY(Reset_Handler) /* Highest address of the user mode stack */ _stack_end = 0x20030000; /* end of RAM */ /* Generate a link error if heap and stack don't fit into RAM */ -_stack_size = 0x400; /* required amount of stack */ +_stack_size = 0x800; /* required amount of stack */ MEMORY { FLASH (rx) : ORIGIN = 0x08000000, LENGTH = 1024K diff --git a/targets/f7/stm32wb55xx_ram_fw.ld b/targets/f7/stm32wb55xx_ram_fw.ld index c3f5f8a6a75e..7eddd8921c22 100644 --- a/targets/f7/stm32wb55xx_ram_fw.ld +++ b/targets/f7/stm32wb55xx_ram_fw.ld @@ -3,7 +3,7 @@ ENTRY(Reset_Handler) /* Highest address of the user mode stack */ _stack_end = 0x20030000; /* end of RAM */ /* Generate a link error if heap and stack don't fit into RAM */ -_stack_size = 0x400; /* required amount of stack */ +_stack_size = 0x800; /* required amount of stack */ MEMORY { FLASH (rx) : ORIGIN = 0x08000000, LENGTH = 1024K