ISSUE-4332: Fix boot-path stack overflow risk and reduce update stack footprint#4350
Open
herbenderbler wants to merge 3 commits intoflipperdevices:devfrom
Open
ISSUE-4332: Fix boot-path stack overflow risk and reduce update stack footprint#4350herbenderbler wants to merge 3 commits intoflipperdevices:devfrom
herbenderbler wants to merge 3 commits intoflipperdevices:devfrom
Conversation
…ices#4332) - Increase stack size from 1024 → 2048 bytes (0x400 → 0x800) - Provides 1096 bytes headroom for boot update operations - Resolves potential overflow in flipper_update_process_manifest - Enable -fstack-usage for static stack analysis
…lipperdevices#4332) Further optimization of boot update stack usage by moving large FatFS structures (FIL, FILINFO, path buffers) from stack to heap allocation. Refactored functions: 1. flipper_update_get_manifest_path Before: ~1160 bytes (FIL + FILINFO + 256B buffer) After: ~50 bytes (pointers only) 2. flipper_update_load_stage Before: ~928 bytes (FIL + FILINFO) After: ~100 bytes (pointers + variables) 3. flipper_update_process_manifest Before: ~920 bytes (FIL + FILINFO) After: ~80 bytes (pointers + variables) Total stack reduction: ~2778 bytes on critical boot path Benefits: - Estimated total boot path stack: ~230 bytes (was ~952 bytes) - Leaves ~1818 bytes headroom in 2048-byte stack - Follows embedded systems best practices - Proper error handling for malloc() failures at each call site - No performance impact - allocations occur once per boot Implementation details: - All FatFS structures now heap-allocated with proper cleanup - Added NULL checks for malloc() failures - Maintained cleanup paths in error conditions - Added clarifying comments explaining stack requirements Files changed: - targets/f7/src/update.c: Three functions refactored Testing: Build validation successful, firmware size unchanged Relates to: flipperdevices#4332 (stack overflow fix)
Disable -fstack-usage compiler flag that was enabled for stack analysis during the fix for issue flipperdevices#4332. Stack analysis is complete and should only be enabled during future stack usage investigations.
herbenderbler
requested review from
DrZlo13,
gsurkov,
hedger and
nminaylov
as code owners
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR resolves a potential stack overflow in the boot update path (issue #4332) with a two-part fix:
It also restores production build settings by disabling temporary stack-usage instrumentation after analysis.
Problem
The boot update path could consume too much stack in constrained scenarios, especially around manifest handling and stage loading. The original configuration used a 1024-byte stack, which provided insufficient safety margins for deep call paths and interrupt context.
What Changed
1) Stack size increase (safety fix)
_stack_sizefrom0x400to0x800in:2) Stack-footprint refactor in boot update code
FIL,FILINFO, and path buffer where applicable), with allocation-failure handling and cleanup:3) Build config cleanup
-fstack-usageonly for analysis phase, then disabled for normal production builds:Commit Breakdown
de0fa1db—fix(boot): increase main thread stack to prevent overflow (#4332)5908589a—refactor(boot): heap-allocate large FatFS structures in update path (#4332)421c1751—build: disable -fstack-usage in production buildsValidation Performed
-fstack-usage.firmware.elf,firmware.bin,firmware.dfu).Risk / Impact
Related Issue
Checklist (For Reviewer)