Fix unguarded pointer write in HAL_UART_ErrorCallback - #708
Open
jonwaterschoot wants to merge 1 commit into
Open
Fix unguarded pointer write in HAL_UART_ErrorCallback#708jonwaterschoot wants to merge 1 commit into
jonwaterschoot wants to merge 1 commit into
Conversation
MapInstanceToHandle() explicitly returns NULL for any instance not in its known list of 9 UART/USART peripherals. f7c63ae added an unconditional write through that pointer on every UART error (handle->listener_mode_ = false), with no null check. Found on hardware where a board runs UART MIDI continuously alongside I2C (OLED, touch) and USB: a floating/idle MIDI input generating occasional UART framing noise caused the OLED to stop updating, touch pads to stop responding, and USB enumeration to fail entirely, while audio and switch/knob input kept working normally. Bisected to this exact commit; reverting just these two lines resolves all three symptoms on the affected hardware. Root cause not fully isolated at the register level (could be the null write itself for an unmapped instance, or the added per-interrupt cost under a high error rate starving other peripherals' interrupt servicing), but the fix is verified against the observed failure.
Merged
3 tasks
Author
|
Same as my other PR before this: I was using an LLM to help me make TouchPlaited a firmware for Simple Touch using the mpr121 sensor and after updating libDaisy this issue surfaced. Traced it back to this, however it might be purposefully implemented for another bug. |
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.
MapInstanceToHandle() explicitly returns NULL for any instance not in its known list of 9 UART/USART peripherals. f7c63ae added an unconditional write through that pointer on every UART error (handle->listener_mode_ = false), with no null check.
Found on a board (TouchPlaited) that runs UART MIDI continuously alongside I2C (OLED, capacitive touch) and USB: a floating/idle MIDI input generating occasional UART framing noise caused the OLED to stop updating, touch pads to stop responding, and USB enumeration to fail entirely, while audio and switch/knob input kept working normally. Bisected the regression to this exact commit against a known-good older libDaisy snapshot; reverting just these two lines resolves all three symptoms on the affected hardware.
I haven't fully isolated the mechanism at the register level, it could be the null write itself for an unmapped instance, or the added per-interrupt cost under a high error rate starving other peripherals' interrupt servicing, but the fix is verified against the observed failure, and the missing null check is a real gap regardless of which it is.