smart_amp: bound host-supplied frame, channel and config sizes#10910
Open
lgirdwood wants to merge 5 commits into
Open
smart_amp: bound host-supplied frame, channel and config sizes#10910lgirdwood wants to merge 5 commits into
lgirdwood wants to merge 5 commits into
Conversation
The feed-forward and feedback paths copy frames * channels samples into fixed intermediate buffers but only checked the frame count. Bound the total sample count against the buffer capacity so an unexpected channel count cannot overflow the buffers. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
lgirdwood
requested review from
dbaluta,
kv2019i,
lbetlej,
mmaka1 and
plbossart
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Hardens smart_amp against malicious/compromised host-provided parameters to prevent out-of-bounds accesses and unintended data exposure, without changing behavior for valid configurations.
Changes:
- Adds platform max channel-count validation in the IPC3 sample component prepare path.
- Tightens bounds checks for buffer sizes (frames × channels), channel map indices, calibration fragment offsets, and get-config copy length.
- Improves error diagnostics for rejected out-of-range inputs.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/samples/audio/smart_amp_test_ipc3.c | Rejects in/out channel counts above PLATFORM_MAX_CHANNELS during prepare. |
| src/audio/smart_amp/smart_amp_maxim_dsm.c | Validates calibration fragment offset/length to prevent reading past calibration data. |
| src/audio/smart_amp/smart_amp_generic.c | Skips unmapped and out-of-range channel-map entries to avoid source-frame OOB. |
| src/audio/smart_amp/smart_amp.c | Bounds get-config copy length by struct size; bounds processing by total samples (frames × channels). |
singalsu
approved these changes
Jun 15, 2026
lyakh
reviewed
Jun 16, 2026
| for (ch = 0; ch < src_mod->channels; ch++) { | ||
| if (chan_map[ch] == -1) | ||
| /* skip unmapped (-1) and out-of-range source channels */ | ||
| if (chan_map[ch] < 0 || chan_map[ch] >= src_ch) |
Collaborator
There was a problem hiding this comment.
this is in the processing channel loop (not the sample loop though)... Would the compiler be smart enough to make a single comparison of it or should we make it explicit (uint8_t)chan_map[ch] >= src_ch?
Member
Author
There was a problem hiding this comment.
Made it explicit per your suggestion — both remap paths now use the single (uint8_t)chan_map[ch] >= src_ch comparison (the cast folds the -1/negative case into the upper-bound check), rather than relying on the compiler to merge the two comparisons.
The remap routines used host-provided channel map entries to index the source frame, guarding only against the -1 unmapped sentinel. Skip any entry outside [0, source channels) so a crafted map cannot read past the source frame. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
The fragmented calibration get path advanced a read offset by a host-controlled index without checking it against the calibration data size, allowing reads past the buffer. Reject offsets at or beyond the data size and fragments that would extend past it. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
The config read-back used the stored config size as the memcpy source length from a fixed-size struct; a host-set oversized size read adjacent heap. Bound the length by the struct size as well as the destination. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.com>
The sample component took its channel counts from the streams and used them to index the platform-sized channel map without bounds. Reject counts above the platform maximum. Signed-off-by: Liam Girdwood <liam.r.girdwood@linux.intel.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
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.
Hardening of host-supplied fields in smart_amp so a compromised host cannot
drive out-of-bounds access in the DSP:
capacity, not just the frame count
source frame
No functional change for valid configurations.