drivers: input: crsf: harden the ISR parser and contain OOB rx-ready windows - #117402
Open
bperseghetti wants to merge 2 commits into
Open
drivers: input: crsf: harden the ISR parser and contain OOB rx-ready windows#117402bperseghetti wants to merge 2 commits into
bperseghetti wants to merge 2 commits into
Conversation
crsf_process_bytes runs in UART ISR context on whatever chunk the async driver hands up, so it must survive torn and malformed frames from a noisy link without walking off a buffer. Sanity-check the incoming chunk on entry: reject a NULL pointer, an empty chunk, or a chunk larger than one RX DMA buffer before dereferencing it. Bound every store into rd_data against its size, resetting the state machine on any violation so no protocol-invariant slip can write out of bounds. Guard every length decrement: check payload_remaining before decrementing in the TYPE, IGNORE, and DATA states, treating zero-when-a-byte-is-still-expected as a framing error that reframes to SYNC. Route all resets through a single crsf_reset_parser helper to keep the hot ISR loop cheap. Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
A field fault traced to the CRSF RX_RDY path: the parser was handed a buffer window whose base pointer read a wild address, faulting in ISR context. The serial driver's async double-buffer accounting was audited and found self-consistent: the RX_RDY event reports a (buf, offset, len) window derived under lock from a single view, with offset and length bounded by the buffer length, so the wild base pointer cannot come from that accounting alone and the root mechanism could not be pinned in the serial layer. This guard is containment at the buffer owner, not a fix for a proven serial-driver defect. The CRSF driver supplies the two RX DMA buffers, so it can validate the event before trusting it: require the RX_RDY buffer to be one of those two buffers and the offset/len window to stay within CRSF_RX_BUF_SIZE before invalidating cache or parsing. Anything else is dropped so a stale or corrupt window can never reach crsf_process_bytes. Signed-off-by: Benjamin Perseghetti <bperseghetti@rudislabs.com>
7 tasks
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
3 participants
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.
Two hardening commits for the CRSF input driver, motivated by a hard fault observed on hardware where the parser walked a receive window that pointed outside the driver's own RX buffers.
Harden the ISR parser against malformed input. Every parser store is bounds-checked against the DMA buffer size, a chunk larger than one RX DMA buffer is dropped wholesale, and any state where a length counter reaches zero without completing its frame reframes instead of continuing on a slipped stream. Well-formed frames see no behavior change.
Contain out-of-bounds async RX-ready windows. Before the parser touches an async RX-ready event, the event's buffer window must name one of the driver's two RX buffers and lie inside it, or the event is dropped and counted. This is containment at the buffer owner, the malformed window originates below the input driver and the underlying mechanism has not been pinned down, so the driver refuses the corrupt window and keeps a counter so any recurrence is visible rather than fatal.
Validated on NXP i.MX RT1064 hardware with a CRSF transceiver over extended runtime with no recurrence of the fault.