Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
24 changes: 23 additions & 1 deletion src/audio/eq_iir/eq_iir.c
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,17 @@ static int eq_iir_check_blob_size(struct comp_dev *dev, size_t size)
return 0;

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.

you never propagate this function's returned error code and always use it as a condition in if clauses. How about making it boolean

static bool eq_iir_blob_size_valid(struct comp_dev *dev, size_t size)
{
	return size >= sizeof(struct sof_eq_iir_config) && size <= SOF_EQ_IIR_MAX_SIZE;
}

}

static int eq_iir_validator(struct comp_dev *dev, void *new_data, uint32_t new_data_size)
{
int ret;

ret = eq_iir_check_blob_size(dev, new_data_size);
if (ret < 0)
return ret;

return eq_iir_validate_config(dev, new_data, new_data_size);
}

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,7 +138,11 @@ 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. Note that the IPC-time validator set
* in eq_iir_prepare() already runs eq_iir_check_blob_size() and
* eq_iir_validate_config() on every blob, so the next check is not
* mandatory.
*/
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)
Expand Down Expand Up @@ -216,6 +231,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_validator);

return ret;
}

Expand All @@ -224,6 +244,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
4 changes: 4 additions & 0 deletions src/audio/eq_iir/eq_iir.h
Original file line number Diff line number Diff line change
Expand Up @@ -71,5 +71,9 @@ 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,
struct sof_eq_iir_config *config,
size_t config_size);

void eq_iir_free_delaylines(struct processing_module *mod);
#endif /* __SOF_AUDIO_EQ_IIR_EQ_IIR_H__ */
108 changes: 80 additions & 28 deletions src/audio/eq_iir/eq_iir_generic.c
Original file line number Diff line number Diff line change
Expand Up @@ -239,59 +239,111 @@ 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;
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 {
}
if (lookup) {
for (; i < SOF_EQ_IIR_MAX_RESPONSES; i++)
lookup[i] = NULL;

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.

memset()

}

return 0;
}

int eq_iir_validate_config(struct comp_dev *dev,
struct sof_eq_iir_config *config,
size_t config_size)
{
int32_t *assign_response;
int32_t resp;
int ret;
int i;

ret = eq_iir_walk_config(dev, config, config_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