Conversation
tmleman
requested review from
dbaluta,
kv2019i,
lbetlej,
lgirdwood,
mmaka1 and
plbossart
as code owners
September 15, 2026 11:27
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The oversized-read issue is addressed with no unresolved blocking concerns.
Pull request overview
Hardens IPC3 smart amplifier config retrieval by bounding host-controlled copy lengths to the configuration struct size.
Changes:
- Rejects oversized configuration lengths.
- Prevents out-of-bounds source reads.
File summaries
| File | Description |
|---|---|
src/samples/audio/smart_amp_test_ipc3.c |
Adds source-size validation before copying configuration data. |
Review details
- Files reviewed: 1/1 changed files
- Comments generated: 0
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
PR 11202: test resultsRun date: 2026-09-15 12:23 UTC Tested commit: 326a04f5884830590c8230e3fb621db74592028c
|
tmleman
requested review from
abonislawski,
serhiy-katsyuba-intel,
softwarecki and
wjablon1
September 15, 2026 12:47
smart_amp_get_config() copies sad->config.size (bs) bytes from &sad->config into the host reply buffer, but only validated bs against the destination bound (size, the IPC max_data_size), never against the actual size of the source: sizeof(struct sof_smart_amp_config). sad->config.size is host-influenced: smart_amp_new() copies a create-time blob (including the embedded size field) into sad->config without validating that field, so a crafted blob can store an arbitrary size. A crafted blob (config.size = 247) followed by a binary GET_DATA request makes the DSP read 223 bytes past the 24-byte config, reported by AddressSanitizer as a use-after-poison read. Fix by validating bs against sizeof(struct sof_smart_amp_config) too, at the point of use, rejecting any oversized .size before it is used as a read length. This is the same pattern used for the channel-count bound added in commit 1007de2 ("smart_amp_test: bound channel counts to platform max"). Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
serhiy-katsyuba-intel
approved these changes
Sep 15, 2026
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.
smart_amp_get_config() copies sad->config.size (bs) bytes from &sad->config into the host reply buffer, but only validated bs against the destination bound (size, the IPC max_data_size), never against the actual size of the source: sizeof(struct sof_smart_amp_config).
sad->config.size is host-influenced: smart_amp_new() copies a create-time blob (including the embedded size field) into sad->config without validating that field, so a crafted blob can store an arbitrary size. A crafted blob (config.size = 247) followed by a binary GET_DATA request makes the DSP read 223 bytes past the 24-byte config, reported by AddressSanitizer as a use-after-poison read.
Fix by validating bs against sizeof(struct sof_smart_amp_config) too, at the point of use, rejecting any oversized .size before it is used as a read length. This is the same pattern used for the channel-count bound added in commit 1007de2 ("smart_amp_test: bound channel counts to platform max").