Closed jonathanperret closed 4 months ago
This update introduces a stack canary mechanism to enhance memory safety by detecting stack buffer overflows. The platformio.ini
file is modified to potentially enable this feature, while the main code in src/ayab/main.cpp
integrates functions for setting up and checking the stack canary. Additionally, the README.md
provides instructions for enabling this security feature. These changes aim to improve the robustness and security of the application, ensuring better protection against runtime vulnerabilities.
Files | Change Summary |
---|---|
platformio.ini | Added a build_flags entry for ENABLE_STACK_CANARY , currently commented out to suggest future enhancement. |
src/ayab/main.cpp | Introduced stack canary functions (stackCanarySetup() and stackCanaryCheck() ) to detect buffer overflows; integrated them into setup() and loop() . |
README.md | Added instructions for enabling stack overflow detection, guiding users on modifying the configuration and monitoring behavior. |
sequenceDiagram
participant User
participant Firmware
participant StackCanary
User->>Firmware: Modify platformio.ini to enable stack canary
User->>Firmware: Build and upload firmware
Firmware->>StackCanary: stackCanarySetup()
StackCanary->>Firmware: Initialize canary
Firmware->>Firmware: Execute main loop
Firmware->>StackCanary: stackCanaryCheck()
alt Canary value intact
StackCanary-->>Firmware: Continue normal execution
else Canary value altered
StackCanary-->>Firmware: Trigger panic mode
Firmware->>User: Indicate stack overflow via LED and serial output
end
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?
Could your write some lines about the ENABLE_STACK_CANARY in the README.mde (basically the description of the PR)? So it's no "hidden feature"
Could your write some lines about the ENABLE_STACK_CANARY in the README.mde (basically the description of the PR)? So it's no "hidden feature"
Sure thing. I added a section to README.md
(https://github.com/AllYarnsAreBeautiful/ayab-firmware/blob/7b77503/README.md#enabling-stack-overflow-detection), let me know if that's what you had in mind.
Note that once #190 is fixed, this check could perhaps remain enabled all the time, to avoid stack overflow bugs creeping back in.
While working on getting the hardware test mode to work, I encountered some erratic behaviours (like the Arduino spontaneously resetting) that made me suspect memory corruption. As the Arduino UNO has only 2048 bytes of RAM, it is very easy to exhaust the available memory in a way that is not detected at compilation time, but results in the program stack (which grows down from the top of memory) overwriting the heap and static data areas.
So I added some code to try to detect such a situation: with the
ENABLE_STACK_CANARY
#define, a "stack canary" is enabled which will detect the stack growing past the top of the heap and enter a "panic" mode to show the problem occurring (by flashing a LED). The canary does consume a few bytes of RAM, so in theory it could itself be the proverbial needle that breaks the camel's back, but if these few bytes are the only margin of safety, a problem probably needs to be addressed.Note that in this PR the canary is disabled by default (in
platformio.ini
), because it actual triggers in normal knitting use, illustrating that there is a problem that needs to be addressed. I have an upcoming PR that offers improvements on this front.Summary by CodeRabbit
New Features
Documentation
Configuration Changes