Validate sample with audiosample_check() in all audio play paths - #11138
Merged
Merged
Conversation
The various play() entry points passed the user-supplied object straight to the non-validating audiosample getters (audiosample_get_sample_rate(), audiosample_get_bits_per_sample(), etc.), which cast the object without checking it implements the audiosample protocol. Passing a non-sample object dereferenced arbitrary memory instead of raising. Add audiosample_check() as the first statement in each affected common-hal play path so a bad object raises cleanly, matching the existing RP2040 audiobusio.I2SOut fix. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
2 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.
Nonepassed to I2SOut .play() #11067Add missing calls to
audiosample_check()in places that needed to validate the incoming object was anaudiosample.I found the original bug in the RP2040
I2SOutplay logic (#11067), and then asked Claude to see if there were other missing arg validations foraudiosample-protocol objects. It found a bunch.Rest is written by Claude and slightly edited.
Most
play()entry points passed the user-supplied object straight to the non-validating audiosample getters (audiosample_get_sample_rate(),audiosample_get_bits_per_sample(),audiosample_get_channel_count(),audiosample_get_buffer_structure()), which cast the object toaudiosample_base_t *without checking it implements the audiosample protocol. Passing a non-sample object dereferenced arbitrary memory instead of raising an error.Paths fixed:
port_i2s_play()(mimxrt covers bothI2SOutandPWMAudioOut)AudioOut&I2SOutI2SOut&PWMAudioOutI2SOut,PWMAudioOut,mcp4822PWMAudioOutI2SOutAudioOut(was safe only by call ordering; made explicit for consistency)Already-correct paths (unchanged): stm
AudioOut, all shared-module effects andaudiomixer.Mixer/MixerVoice(viaaudiosample_must_match()), andaudiospeed/audiofilewriter/usb_audio(explicit checks).play()was the only entry point with the missing check.