Skip to content

audio: eq_iir: register IPC-time blob validator #10942

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
singalsu wants to merge 2 commits into
base: main
Choose a base branch
from
+105 −44
Open

audio: eq_iir: register IPC-time blob validator #10942

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
442b1d4
audio: eq_iir: register IPC-time blob validator
singalsu Jun 18, 2026
ca65a29
audio: eq_iir: Clear lookup tail with memset
singalsu Jun 29, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 18 additions & 15 deletions src/audio/eq_iir/eq_iir.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,16 +107,6 @@ static int eq_iir_get_config(struct processing_module *mod,
return comp_data_blob_get_cmd(cd->model_handler, cdata, fragment_size);
}

static int eq_iir_check_blob_size(struct comp_dev *dev, size_t size)
{
if (size < sizeof(struct sof_eq_iir_config) || size > SOF_EQ_IIR_MAX_SIZE) {
comp_err(dev, "invalid configuration blob, size %zu", size);
return -EINVAL;
}

return 0;
}

static int eq_iir_process(struct processing_module *mod,
struct input_stream_buffer *input_buffers, int num_input_buffers,
struct output_stream_buffer *output_buffers, int num_output_buffers)
Expand All @@ -127,11 +117,15 @@ static int eq_iir_process(struct processing_module *mod,
uint32_t frame_count = input_buffers[0].size;
int ret;

/* Check for changed configuration */
/* Check for changed configuration. The IPC-time validator installed
* in eq_iir_prepare() has already structurally validated the blob, so
* only NULL needs to be guarded here.
*/
if (comp_is_new_data_blob_available(cd->model_handler)) {
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)
if (!cd->config)
return -EINVAL;

ret = eq_iir_new_blob(mod, audio_stream_get_frm_fmt(source),
audio_stream_get_frm_fmt(sink),
audio_stream_get_channels(source));
Expand Down Expand Up @@ -203,9 +197,11 @@ static int eq_iir_prepare(struct processing_module *mod,
eq_iir_set_passthrough_func(cd, source_format, sink_format);

/* Initialize EQ */
if (cd->config && cd->config_size > 0) {
if (eq_iir_check_blob_size(dev, cd->config_size) < 0)
return -EINVAL;
if (cd->config) {
ret = eq_iir_validate_config(dev, cd->config, cd->config_size);
if (ret < 0)
return ret;

ret = eq_iir_new_blob(mod, source_format, sink_format, channels);
if (ret)
return ret;
Expand All @@ -216,6 +212,11 @@ static int eq_iir_prepare(struct processing_module *mod,
ret = -EINVAL;
}

/* Reject malformed blobs at IPC time so a bad run-time update cannot
* replace the working configuration.
*/
comp_data_blob_set_validator(cd->model_handler, eq_iir_validate_config);

return ret;
}

Expand All @@ -224,6 +225,8 @@ static int eq_iir_reset(struct processing_module *mod)
struct comp_data *cd = module_get_private_data(mod);
int i;

comp_data_blob_set_validator(cd->model_handler, NULL);

eq_iir_free_delaylines(mod);

cd->eq_iir_func = NULL;
Expand Down
2 changes: 2 additions & 0 deletions src/audio/eq_iir/eq_iir.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -71,5 +71,7 @@ void eq_iir_pass(struct processing_module *mod, struct input_stream_buffer *bsou

int eq_iir_setup(struct processing_module *mod, int nch);

int eq_iir_validate_config(struct comp_dev *dev, void *new_data, uint32_t new_data_size);

void eq_iir_free_delaylines(struct processing_module *mod);
#endif /* __SOF_AUDIO_EQ_IIR_EQ_IIR_H__ */
114 changes: 85 additions & 29 deletions src/audio/eq_iir/eq_iir_generic.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -239,59 +239,115 @@ static int eq_iir_init_response(struct comp_dev *dev, int idx,
return 0;
}

static int eq_iir_init_coef(struct processing_module *mod, int nch)
/* Validate the config blob layout and, if lookup is non-NULL, populate it
* with pointers to each response header. Pass lookup = NULL to validate only.
*/
static int eq_iir_walk_config(struct comp_dev *dev,
struct sof_eq_iir_config *config,
size_t config_size,
struct sof_eq_iir_header **lookup)
{
struct comp_data *cd = module_get_private_data(mod);
struct sof_eq_iir_config *config = cd->config;
struct iir_state_df1 *iir = cd->iir;
struct sof_eq_iir_header *lookup[SOF_EQ_IIR_MAX_RESPONSES];
struct sof_eq_iir_header *eq;
uint32_t coef_words_max;
int32_t *assign_response;
int32_t *coef_data;
int size_sum = 0;
int resp = 0;
int ret;
int i;
uint32_t j;
int s;
int ret;

comp_info(mod->dev, "%u responses, %u channels, stream %d channels",
config->number_of_responses, config->channels_in_config, nch);

/* Sanity checks */
if (nch > PLATFORM_MAX_CHANNELS ||
config->channels_in_config > PLATFORM_MAX_CHANNELS ||
if (config->channels_in_config > PLATFORM_MAX_CHANNELS ||
!config->channels_in_config) {
comp_err(mod->dev, "invalid channels count");
comp_err(dev, "invalid channels_in_config %u", config->channels_in_config);
return -EINVAL;
}
if (config->number_of_responses > SOF_EQ_IIR_MAX_RESPONSES) {
comp_err(mod->dev, "# of resp exceeds max");
comp_err(dev, "# of resp %u exceeds max", config->number_of_responses);
return -EINVAL;
}

ret = eq_iir_blob_words_max(mod->dev, config, cd->config_size, &coef_words_max);
ret = eq_iir_blob_words_max(dev, config, config_size, &coef_words_max);
if (ret < 0)
return ret;

/* Collect index of response start positions in all_coefficients[] */
j = 0;
assign_response = ASSUME_ALIGNED(&config->data[0], 4);
coef_data = ASSUME_ALIGNED(&config->data[config->channels_in_config], 4);
for (i = 0; i < SOF_EQ_IIR_MAX_RESPONSES; i++) {
if (i < config->number_of_responses) {
ret = eq_iir_init_response(mod->dev, i, coef_data,
coef_words_max, &j, &eq);
if (ret < 0)
return ret;
if (lookup)
memset(lookup, 0, SOF_EQ_IIR_MAX_RESPONSES * sizeof(*lookup));

@lyakh lyakh Jun 30, 2026

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.

ah, now you "waste cycles" by first setting it all to 0 and then overwriting a part of it... I thought you'd do something like memset(lookup + i, 0, (SOF_EQ_IIR_MAX_RESPONSES - i) * sizeof(lookup[0])) after the loop. And you can do it directly in the previous commit

@singalsu singalsu Jun 30, 2026

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.

Waste code size vs. waste cycles ;) OK, I can do this way.

@singalsu singalsu Jun 30, 2026

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.

Also, it's a very small array...

@singalsu singalsu Jun 30, 2026

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.

Size for memset() is unsigned, so less than zero size would be huge size, though it should never happen, but handling for zero is needed. The man page for memset isn't mentioning what happens if called with zero size.

@singalsu singalsu Jun 30, 2026 •
edited
Loading

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.

OK, seems zero size is legal, but I still wonder if the risk for negative size -> huge positive size risk could happen.


for (i = 0; i < config->number_of_responses; i++) {
ret = eq_iir_init_response(dev, i, coef_data, coef_words_max, &j, &eq);
if (ret < 0)
return ret;
if (lookup)
lookup[i] = eq;
} else {
lookup[i] = NULL;
}

return 0;
}

int eq_iir_validate_config(struct comp_dev *dev, void *new_data, uint32_t new_data_size)
{
struct sof_eq_iir_config *config = new_data;
int32_t *assign_response;
int32_t resp;
int ret;
int i;

if (new_data_size < sizeof(struct sof_eq_iir_config) ||
new_data_size > SOF_EQ_IIR_MAX_SIZE) {
comp_err(dev, "invalid configuration blob, size %u", new_data_size);
return -EINVAL;
}

ret = eq_iir_walk_config(dev, config, new_data_size, NULL);
if (ret < 0)
return ret;

/* Validate every assign_response[] entry that the per-channel loop in
* eq_iir_init_coef() could pick up. Entries beyond channels_in_config
* reuse the last assigned value, so checking [0, channels_in_config)
* covers all reachable nch.
*/
assign_response = ASSUME_ALIGNED(&config->data[0], 4);
for (i = 0; i < config->channels_in_config; i++) {
resp = assign_response[i];
if (resp >= 0 && resp >= config->number_of_responses) {
comp_err(dev, "assign_response[%d] = %d exceeds %u",
i, resp, config->number_of_responses);
return -EINVAL;
}
}

return 0;
}

static int eq_iir_init_coef(struct processing_module *mod, int nch)
{
struct comp_data *cd = module_get_private_data(mod);
struct sof_eq_iir_config *config = cd->config;
struct iir_state_df1 *iir = cd->iir;
struct sof_eq_iir_header *lookup[SOF_EQ_IIR_MAX_RESPONSES];
struct sof_eq_iir_header *eq;
int32_t *assign_response;
int size_sum = 0;
int resp = 0;
int i;
int s;
int ret;

comp_info(mod->dev, "%u responses, %u channels, stream %d channels",
config->number_of_responses, config->channels_in_config, nch);

if (nch > PLATFORM_MAX_CHANNELS) {
comp_err(mod->dev, "invalid stream channels %d", nch);
return -EINVAL;
}

ret = eq_iir_walk_config(mod->dev, config, cd->config_size, lookup);
if (ret < 0)
return ret;

/* Initialize 1st phase */
assign_response = ASSUME_ALIGNED(&config->data[0], 4);
for (i = 0; i < nch; i++) {
/* Check for not reading past blob response to channel assign
* map. The previous channel response is assigned for any
Expand Down
Loading