audio: eq_iir: register IPC-time blob validator #10942
+124
−44
There are no files selected for viewing
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
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -211,6 +211,26 @@ static int eq_iir_blob_words_max(struct comp_dev *dev, | |
| return 0; | ||
| } | ||
|
|
||
| /** | ||
| * @brief Parse one response header from the coefficient blob. | ||
| * | ||
| * Reads the response at word offset *j in @p coef_data, checks that both | ||
| * the fixed header and the following biquad sections fit inside the | ||
| * @p coef_words_max blob budget, and advances *j past this response so | ||
| * the next call continues where this one left off. On success @p *eq_out | ||
| * is set to point at the in-place header; callers that only want to walk | ||
| * the blob for validation can discard that pointer. | ||
| * | ||
| * @param dev Component device, used for error logging. | ||
| * @param idx Response index, used in error messages. | ||
| * @param coef_data Base of the coefficient word array in the blob. | ||
| * @param coef_words_max Total words available in @p coef_data. | ||
| * @param j In/out cursor in words; advanced past this response. | ||
| * @param eq_out Out; pointer to the parsed response header. | ||
| * | ||
| * @return 0 on success, -EINVAL if the header or sections would run off | ||
| * the end of the blob or num_sections exceeds the platform limit. | ||
| */ | ||
| static int eq_iir_init_response(struct comp_dev *dev, int idx, | ||
| int32_t *coef_data, uint32_t coef_words_max, | ||
| uint32_t *j, struct sof_eq_iir_header **eq_out) | ||
|
|
@@ -239,59 +259,120 @@ static int eq_iir_init_response(struct comp_dev *dev, int idx, | |
| return 0; | ||
| } | ||
|
|
||
| /* 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 sof_eq_iir_header *eq; | ||
| uint32_t coef_words_max; | ||
| int32_t *coef_data; | ||
| int ret; | ||
| int i; | ||
| uint32_t j; | ||
|
|
||
| if (config->channels_in_config > PLATFORM_MAX_CHANNELS || | ||
| !config->channels_in_config) { | ||
| 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(dev, "# of resp %u exceeds max", config->number_of_responses); | ||
| return -EINVAL; | ||
| } | ||
|
|
||
| ret = eq_iir_blob_words_max(dev, config, config_size, &coef_words_max); | ||
| if (ret < 0) | ||
| return ret; | ||
|
|
||
| j = 0; | ||
| coef_data = ASSUME_ALIGNED(&config->data[config->channels_in_config], 4); | ||
| if (lookup) | ||
| memset(lookup, 0, SOF_EQ_IIR_MAX_RESPONSES * sizeof(*lookup)); | ||
|
|
||
| for (i = 0; i < config->number_of_responses; i++) { | ||
| /* Bounds-check response i, advance the walk cursor j past it, | ||
| * and get a pointer to its in-place header. Stored in lookup[] | ||
| * for later use, or discarded when we are walking the blob | ||
| * only to validate it. | ||
| */ | ||
| ret = eq_iir_init_response(dev, i, coef_data, coef_words_max, &j, &eq); | ||
|
Collaborator
There was a problem hiding this comment. here it's called only to check parameters, right? It doesn't actually initialise anything in this case?
Collaborator
Author
There was a problem hiding this comment. It's more than parameter check, it populates the lookup to used EQ responses. I'm adding comment to the function to avoid it to confuse.
Collaborator
Author
There was a problem hiding this comment. @lyakh Is it OK now? For confidence of this patch, I'm always testing myself locally these with a DUT plus testbench run, with same tests not covered by CI (yet). |
||
| if (ret < 0) | ||
| return ret; | ||
| if (lookup) | ||
| lookup[i] = eq; | ||
| } | ||
|
|
||
| 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 | ||
|
|
||
Oops, something went wrong.
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.
There was a problem hiding this comment.
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 commitThere was a problem hiding this comment.
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.
There was a problem hiding this comment.
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...
There was a problem hiding this comment.
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.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
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.