audio: eq_fir: Improve robustness for invalid configuration#10895
Open
singalsu wants to merge 1 commit into
Open
audio: eq_fir: Improve robustness for invalid configuration#10895singalsu wants to merge 1 commit into
singalsu wants to merge 1 commit into
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Tightens validation of the EQ FIR IPC configuration blob to avoid out-of-bounds reads when parsing coefficient sections.
Changes:
- Track configuration blob size in
comp_dataand validate it on prepare and on runtime updates. - Pass blob size into
eq_fir_init_coef()and add size/structure bounds checks while walking responses. - Update validator path to supply
new_data_sizefor cross-checking blob self-declared size.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/audio/eq_fir/eq_fir.h | Store the configuration blob size alongside the config pointer. |
| src/audio/eq_fir/eq_fir.c | Thread blob size through init/validation paths and add bounds checks while parsing FIR response data. |
Comment on lines
+369
to
+376
| if (comp_is_new_data_blob_available(cd->model_handler)) { | ||
| cd->config = comp_get_data_blob(cd->model_handler, NULL, NULL); | ||
| cd->config = comp_get_data_blob(cd->model_handler, &cd->config_size, NULL); | ||
| if (!cd->config || cd->config_size < sizeof(*cd->config) || | ||
| cd->config_size > SOF_EQ_FIR_MAX_SIZE) { | ||
| comp_err(mod->dev, "invalid configuration blob, size %zu", | ||
| cd->config_size); | ||
| return -EINVAL; | ||
| } |
Collaborator
Author
There was a problem hiding this comment.
This is something to address later.
lyakh
approved these changes
Jun 12, 2026
Harden the EQ FIR setup path against malformed IPC configuration blobs. The blob length returned by comp_get_data_blob() is now stored and checked against the expected range every time a new blob is taken, and the blob's self-declared size is cross-checked against it before use. The per-response walk that previously trusted the FIR length field from the blob now bounds the header and coefficient data against the blob, and rejects lengths that are non-positive, exceed SOF_FIR_MAX_LENGTH or are not a multiple of four. The IPC validator applies the same blob length bounds before calling into eq_fir_init_coef(), so an oversized or truncated blob is rejected up front rather than at prepare or process time. Also a blob that has an odd channels_in_config is rejected to ensure the coefficients are aligned per current ASSUME_ALIGNED() constraint. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
Comment on lines
+105
to
+108
| if (config->size != config_size) { | ||
| comp_err(dev, "Incorrect configuration blob size"); | ||
| return -EINVAL; | ||
| } |
Comment on lines
+116
to
+124
| /* channels_in_config indexes into a int16_t array. An odd count would | ||
| * leave the coefficient area at a 2-byte alignment, breaking the | ||
| * 4-byte aligned int32_t loads in the optimized FIR kernels. | ||
| */ | ||
| if (config->channels_in_config & 0x1) { | ||
| comp_err(dev, "channels_in_config %u must be even", | ||
| config->channels_in_config); | ||
| return -EINVAL; | ||
| } |
Comment on lines
+281
to
+286
| static int eq_fir_check_blob_size(struct comp_dev *dev, size_t size) | ||
| { | ||
| if (size < sizeof(struct sof_eq_fir_config) || size > SOF_EQ_FIR_MAX_SIZE) { | ||
| comp_err(dev, "invalid configuration blob, size %zu", size); | ||
| return -EINVAL; | ||
| } |
Comment on lines
470
to
476
| cd->eq_fir_func = eq_fir_passthrough; | ||
| cd->config = comp_get_data_blob(cd->model_handler, &data_size, NULL); | ||
| if (cd->config && data_size > 0) { | ||
| cd->config = comp_get_data_blob(cd->model_handler, &cd->config_size, NULL); | ||
| if (cd->config) { | ||
| if (eq_fir_check_blob_size(dev, cd->config_size) < 0) | ||
| return -EINVAL; | ||
|
|
||
| ret = eq_fir_setup(mod, channels); |
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.
Validate the IPC configuration blob to prevent out-of-bounds reads when walking FIR coefficient sections: