Fix intermittent RadioLib static SPI buffer overflow#2708
Open
VBart wants to merge 1 commit into
Open
Conversation
When RADIOLIB_STATIC_ONLY=1 is set, RadioLib's SPItransferStream() allocates two fixed-size stack buffers (buffOut and buffIn) of RADIOLIB_STATIC_ARRAY_SIZE bytes each, instead of heap-allocating exactly the right size. The default value of RADIOLIB_STATIC_ARRAY_SIZE is 256. When receiving a maximum-size LoRa packet (255 bytes, equal to MAX_TRANS_UNIT), SX126x::readBuffer() passes a 3-byte SPI command header (CMD_READ_BUFFER + offset + NOP) plus 255 bytes of payload to SPItransferStream(), for a total buffLen of 258 bytes. This overflows the 256-byte stack buffers by 2 bytes, corrupting adjacent locals and occasionally the stack canary, triggering __stack_chk_fail. The overflow is small (2 bytes on the read path, 1 byte on the write path), so it only intermittently reaches the stack canary depending on compiler-generated stack frame layout. Set RADIOLIB_STATIC_ARRAY_SIZE=260 to eliminate the overflow, with 2 bytes of margin on the read path (258 < 260). The value is placed in [arduino_base] so it applies to all target platforms.
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.
When RADIOLIB_STATIC_ONLY=1 is set, RadioLib's SPItransferStream() allocates two fixed-size stack buffers (buffOut and buffIn) of RADIOLIB_STATIC_ARRAY_SIZE bytes each, instead of heap-allocating exactly the right size.
The default value of RADIOLIB_STATIC_ARRAY_SIZE is 256. When receiving a maximum-size LoRa packet (255 bytes, equal to MAX_TRANS_UNIT), SX126x::readBuffer() passes a 3-byte SPI command header (CMD_READ_BUFFER + offset + NOP) plus 255 bytes of payload to SPItransferStream(), for a total buffLen of 258 bytes. This overflows the 256-byte stack buffers by 2 bytes, corrupting adjacent locals and occasionally the stack canary, triggering __stack_chk_fail.
The overflow is small (2 bytes on the read path, 1 byte on the write path), so it only intermittently reaches the stack canary depending on compiler-generated stack frame layout.
Set RADIOLIB_STATIC_ARRAY_SIZE=260 to eliminate the overflow, with 2 bytes of margin on the read path (258 < 260). The value is placed in [arduino_base] so it applies to all target platforms.