From 8efd7e3dddaebb6dea4db6295cc32bedef989d76 Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Thu, 11 Jun 2026 17:32:20 +0300 Subject: [PATCH 1/2] audio: eq_iir: Improve robustness for invalid configuration 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 --- src/audio/eq_iir/eq_iir.c | 21 ++++++-- src/audio/eq_iir/eq_iir.h | 1 + src/audio/eq_iir/eq_iir_generic.c | 82 ++++++++++++++++++++++++++++--- src/math/iir_df1.c | 8 ++- src/math/iir_df2t.c | 8 ++- 5 files changed, 105 insertions(+), 15 deletions(-) diff --git a/src/audio/eq_iir/eq_iir.c b/src/audio/eq_iir/eq_iir.c index 016c2c8caf82..2c90855ab061 100644 --- a/src/audio/eq_iir/eq_iir.c +++ b/src/audio/eq_iir/eq_iir.c @@ -107,6 +107,16 @@ 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) @@ -119,7 +129,9 @@ static int eq_iir_process(struct processing_module *mod, /* Check for changed configuration */ 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 || eq_iir_check_blob_size(mod->dev, cd->config_size) < 0) + 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)); @@ -158,7 +170,6 @@ static int eq_iir_prepare(struct processing_module *mod, struct comp_dev *dev = mod->dev; enum sof_ipc_frame source_format; enum sof_ipc_frame sink_format; - size_t data_size; int channels; int ret = 0; @@ -183,7 +194,7 @@ static int eq_iir_prepare(struct processing_module *mod, source_format = audio_stream_get_frm_fmt(&sourceb->stream); 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); /* Initialize EQ */ comp_info(dev, "source_format=%d, sink_format=%d", @@ -192,7 +203,9 @@ static int eq_iir_prepare(struct processing_module *mod, eq_iir_set_passthrough_func(cd, source_format, sink_format); /* Initialize EQ */ - if (cd->config && data_size > 0) { + if (cd->config && cd->config_size > 0) { + if (eq_iir_check_blob_size(dev, cd->config_size) < 0) + return -EINVAL; ret = eq_iir_new_blob(mod, source_format, sink_format, channels); if (ret) return ret; diff --git a/src/audio/eq_iir/eq_iir.h b/src/audio/eq_iir/eq_iir.h index 12d888d66594..aa325a913005 100644 --- a/src/audio/eq_iir/eq_iir.h +++ b/src/audio/eq_iir/eq_iir.h @@ -39,6 +39,7 @@ struct comp_data { struct comp_data_blob_handler *model_handler; struct sof_eq_iir_config *config; int32_t *iir_delay; /**< pointer to allocated RAM */ + size_t config_size; /**< configuration size */ size_t iir_delay_size; /**< allocated size */ eq_iir_func eq_iir_func; /**< processing function */ }; diff --git a/src/audio/eq_iir/eq_iir_generic.c b/src/audio/eq_iir/eq_iir_generic.c index fd3485a28eca..6eebcc4e0ed3 100644 --- a/src/audio/eq_iir/eq_iir_generic.c +++ b/src/audio/eq_iir/eq_iir_generic.c @@ -180,6 +180,65 @@ void eq_iir_s32_default(struct processing_module *mod, struct input_stream_buffe } #endif /* CONFIG_FORMAT_S32LE */ +static int eq_iir_blob_words_max(struct comp_dev *dev, + const struct sof_eq_iir_config *config, + size_t blob_size, + uint32_t *coef_words_max) +{ + size_t payload_bytes; + + /* Compute the size of the coefficient area in int32_t words from the + * framework-reported blob size. The blob layout is: + * sizeof(*config) header bytes + * channels_in_config int32_t assign_response[] + * coefficient data[] + * channels_in_config is bounded above, so the multiply fits in size_t. + * The blob's self-declared config->size is cross-checked against the + * authoritative blob_size so all later parsing stays within the buffer. + */ + if (blob_size < sizeof(*config) || config->size != blob_size) { + comp_err(dev, "blob size %zu / header size %u mismatch or too small", + blob_size, config->size); + return -EINVAL; + } + payload_bytes = blob_size - sizeof(*config); + if (payload_bytes % sizeof(int32_t) || + payload_bytes < (size_t)config->channels_in_config * sizeof(int32_t)) { + comp_err(dev, "blob size %zu misaligned or too small", blob_size); + return -EINVAL; + } + *coef_words_max = payload_bytes / sizeof(int32_t) - config->channels_in_config; + return 0; +} + +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) +{ + struct sof_eq_iir_header *eq; + uint32_t header_end = *j + SOF_EQ_IIR_NHEADER; + uint32_t section_end; + + /* Header must fit before reading num_sections */ + if (header_end > coef_words_max) { + comp_err(dev, "response %d header out of bounds", idx); + return -EINVAL; + } + eq = (struct sof_eq_iir_header *)&coef_data[*j]; + /* Bound num_sections so the multiply cannot overflow and the section + * data stays within the blob. + */ + section_end = header_end + (uint32_t)SOF_EQ_IIR_NBIQUAD * eq->num_sections; + if (eq->num_sections > SOF_EQ_IIR_BIQUADS_MAX || section_end > coef_words_max) { + comp_err(dev, "response %d num_sections %u out of bounds", + idx, eq->num_sections); + return -EINVAL; + } + *eq_out = eq; + *j = section_end; + return 0; +} + static int eq_iir_init_coef(struct processing_module *mod, int nch) { struct comp_data *cd = module_get_private_data(mod); @@ -187,13 +246,15 @@ static int eq_iir_init_coef(struct processing_module *mod, int nch) 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 i; - int j; + 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); @@ -210,17 +271,21 @@ static int eq_iir_init_coef(struct processing_module *mod, int nch) return -EINVAL; } + ret = eq_iir_blob_words_max(mod->dev, config, cd->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); + 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) { - eq = (struct sof_eq_iir_header *)&coef_data[j]; + ret = eq_iir_init_response(mod->dev, i, coef_data, + coef_words_max, &j, &eq); + if (ret < 0) + return ret; lookup[i] = eq; - j += SOF_EQ_IIR_NHEADER - + SOF_EQ_IIR_NBIQUAD * eq->num_sections; } else { lookup[i] = NULL; } @@ -318,7 +383,10 @@ int eq_iir_setup(struct processing_module *mod, int nch) /* Free existing IIR channels data if it was allocated */ eq_iir_free_delaylines(mod); - /* Set coefficients for each channel EQ from coefficient blob */ + /* Set coefficients for each channel EQ from coefficient blob. + * eq_iir_init_coef() / eq_iir_blob_words_max() perform all blob size + * sanity checks, including config->size vs cd->config_size. + */ delay_size = eq_iir_init_coef(mod, nch); if (delay_size < 0) return delay_size; /* Contains error code */ diff --git a/src/math/iir_df1.c b/src/math/iir_df1.c index 861c5cd2cf19..5e4d13b8cc70 100644 --- a/src/math/iir_df1.c +++ b/src/math/iir_df1.c @@ -16,9 +16,13 @@ int iir_delay_size_df1(struct sof_eq_iir_header *config) { - int n = config->num_sections; /* One section uses two unit delays */ + uint32_t n = config->num_sections; /* One section uses four unit delays */ - if (n > SOF_EQ_IIR_BIQUADS_MAX || n < 1) + if (!n || n > SOF_EQ_IIR_BIQUADS_MAX) + return -EINVAL; + + if (!config->num_sections_in_series || + config->num_sections_in_series > n) return -EINVAL; return 4 * n * sizeof(int32_t); diff --git a/src/math/iir_df2t.c b/src/math/iir_df2t.c index b2adb69be03f..ed2b50a21a4e 100644 --- a/src/math/iir_df2t.c +++ b/src/math/iir_df2t.c @@ -16,9 +16,13 @@ int iir_delay_size_df2t(struct sof_eq_iir_header *config) { - int n = config->num_sections; /* One section uses two unit delays */ + uint32_t n = config->num_sections; /* One section uses two unit delays */ - if (n > SOF_EQ_IIR_BIQUADS_MAX || n < 1) + if (!n || n > SOF_EQ_IIR_BIQUADS_MAX) + return -EINVAL; + + if (!config->num_sections_in_series || + config->num_sections_in_series > n) return -EINVAL; return 2 * n * sizeof(int64_t); From db4e65971f7339c964a26e2c050180be33ddd4df Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Thu, 18 Jun 2026 14:22:46 +0300 Subject: [PATCH 2/2] audio: eq_iir: register IPC-time blob validator Hook an eq_iir blob validator into the model handler so a corrupted run-time configuration update is rejected before it can replace the working blob. Playback or capture then continues with the previously set coefficients instead of being interrupted by a bad IPC. The validator parses the blob layout end to end: channel and response counts, each response header and biquad section bounds, and the assign_response[] entries reachable by the per-channel loop. The walk is factored into a shared helper that eq_iir_init_coef() also reuses at setup time, so IPC-time and prepare-time use exactly the same checks. Signed-off-by: Seppo Ingalsuo --- src/audio/eq_iir/eq_iir.c | 24 ++++++- src/audio/eq_iir/eq_iir.h | 4 ++ src/audio/eq_iir/eq_iir_generic.c | 108 ++++++++++++++++++++++-------- 3 files changed, 107 insertions(+), 29 deletions(-) diff --git a/src/audio/eq_iir/eq_iir.c b/src/audio/eq_iir/eq_iir.c index 2c90855ab061..e54fc52a9b37 100644 --- a/src/audio/eq_iir/eq_iir.c +++ b/src/audio/eq_iir/eq_iir.c @@ -117,6 +117,17 @@ static int eq_iir_check_blob_size(struct comp_dev *dev, size_t size) return 0; } +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) @@ -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) @@ -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; } @@ -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; diff --git a/src/audio/eq_iir/eq_iir.h b/src/audio/eq_iir/eq_iir.h index aa325a913005..0508888438a3 100644 --- a/src/audio/eq_iir/eq_iir.h +++ b/src/audio/eq_iir/eq_iir.h @@ -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__ */ diff --git a/src/audio/eq_iir/eq_iir_generic.c b/src/audio/eq_iir/eq_iir_generic.c index 6eebcc4e0ed3..0c0f32450909 100644 --- a/src/audio/eq_iir/eq_iir_generic.c +++ b/src/audio/eq_iir/eq_iir_generic.c @@ -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; + } + + 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