Skip to content

audio: eq_iir: Improve robustness for invalid configuration#10892

Open
singalsu wants to merge 1 commit into
thesofproject:mainfrom
singalsu:iir_updates
Open

audio: eq_iir: Improve robustness for invalid configuration#10892
singalsu wants to merge 1 commit into
thesofproject:mainfrom
singalsu:iir_updates

Conversation

@singalsu

@singalsu singalsu commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator

Validate the EQ IIR configuration blob and IIR header fields to prevent out-of-bounds reads and malformed-state setup.

@singalsu singalsu marked this pull request as ready for review June 12, 2026 08:30
Copilot AI review requested due to automatic review settings June 12, 2026 08:30

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR improves robustness of the EQ IIR module by validating configuration blob sizes and tightening bounds checks while parsing IIR coefficient data, aiming to prevent out-of-bounds reads and malformed configuration setup.

Changes:

  • Track and validate the configuration blob length (config_size) when fetching model data, both in prepare and on-the-fly update paths.
  • Harden coefficient parsing by deriving coefficient-area limits from the blob’s declared size and bounds-checking each response header/biquad block.
  • Add range checks for num_sections_in_series and reject blobs whose self-declared size doesn’t match the received blob length.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/math/iir_df2t.c Adds validation for num_sections_in_series in DF2T delay sizing.
src/math/iir_df1.c Adds validation for num_sections_in_series in DF1 delay sizing and corrects the delay comment.
src/audio/eq_iir/eq_iir.h Adds config_size to store the blob length returned by comp_get_data_blob().
src/audio/eq_iir/eq_iir.c Validates blob size on prepare and runtime update before applying new configuration.
src/audio/eq_iir/eq_iir_generic.c Adds coefficient-area bounds checks and enforces config->size == blob length in eq_iir_setup().

Comment thread src/math/iir_df1.c Outdated
Comment thread src/math/iir_df2t.c Outdated
Comment thread src/audio/eq_iir/eq_iir_generic.c
Comment thread src/audio/eq_iir/eq_iir.c Outdated
comp_err(mod->dev, "invalid configuration blob, size %zu",
cd->config_size);
return -EINVAL;
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

at least from the commit description it looks like this should be 4 commits

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Too detailed description ... it's all about blob size check and not reading past it.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Comment thread src/audio/eq_iir/eq_iir_generic.c Outdated
Comment thread src/audio/eq_iir/eq_iir.c Outdated
Comment thread src/audio/eq_iir/eq_iir_generic.c
Harden the EQ IIR 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 num_sections from the
blob now bounds the header and biquad data against the blob, so a bad
length can no longer push the lookup pointer past the allocation.

The df1 and df2t delay-size helpers also gained a range check on
num_sections_in_series, which strides the delay line and was
previously unchecked.

Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 5 comments.

Comment thread src/audio/eq_iir/eq_iir.c
Comment on lines +132 to +134
cd->config = comp_get_data_blob(cd->model_handler, &cd->config_size, NULL);
if (!cd->config || eq_iir_check_blob_size(mod->dev, cd->config_size) < 0)
return -EINVAL;
Comment thread src/audio/eq_iir/eq_iir.c
sink_format = audio_stream_get_frm_fmt(&sinkb->stream);

cd->config = comp_get_data_blob(cd->model_handler, &data_size, NULL);
cd->config = comp_get_data_blob(cd->model_handler, &cd->config_size, NULL);
Comment thread src/audio/eq_iir/eq_iir.c
Comment on lines +206 to +208
if (cd->config && cd->config_size > 0) {
if (eq_iir_check_blob_size(dev, cd->config_size) < 0)
return -EINVAL;
Comment on lines +379 to +382
if (cd->config->size != cd->config_size) {
comp_err(mod->dev, "Incorrect configuration blob size");
return -EINVAL;
}
Comment on lines +196 to +200
if (config->size < sizeof(*config)) {
comp_err(dev, "config size %u too small", config->size);
return -EINVAL;
}
payload_bytes = config->size - sizeof(*config);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants