Skip to content

Commit 2ac1c68

Browse files
committed
audio: eq_fir: Improve robustness for invalid configuration
Harden the EQ FIR 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 the FIR length field from the blob now bounds the header and coefficient data against the blob, and rejects lengths that are non-positive, exceed SOF_FIR_MAX_LENGTH or are not a multiple of four. The IPC validator applies the same blob length bounds before calling into eq_fir_init_coef(), so an oversized or truncated blob is rejected up front rather than at prepare or process time. Also a blob that has an odd channels_in_config is rejected to ensure the coefficients are aligned per current ASSUME_ALIGNED() constraint. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>
1 parent bd296e4 commit 2ac1c68

2 files changed

Lines changed: 75 additions & 10 deletions

File tree

src/audio/eq_fir/eq_fir.c

Lines changed: 74 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -72,12 +72,13 @@ static void eq_fir_free_delaylines(struct processing_module *mod)
7272
}
7373

7474
static int eq_fir_init_coef(struct comp_dev *dev, struct sof_eq_fir_config *config,
75-
struct fir_state_32x16 *fir, int nch)
75+
size_t config_size, struct fir_state_32x16 *fir, int nch)
7676
{
7777
struct sof_fir_coef_data *lookup[SOF_EQ_FIR_MAX_RESPONSES];
7878
struct sof_fir_coef_data *eq;
7979
int16_t *assign_response;
8080
int16_t *coef_data;
81+
size_t coef_words_max;
8182
size_t size_sum = 0;
8283
int resp = 0;
8384
int i;
@@ -101,27 +102,70 @@ static int eq_fir_init_coef(struct comp_dev *dev, struct sof_eq_fir_config *conf
101102
config->number_of_responses, config->channels_in_config, nch);
102103

103104
/* Sanity checks */
105+
if (config->size != config_size) {
106+
comp_err(dev, "Incorrect configuration blob size");
107+
return -EINVAL;
108+
}
109+
104110
if (nch > PLATFORM_MAX_CHANNELS ||
105111
config->channels_in_config > PLATFORM_MAX_CHANNELS ||
106112
!config->channels_in_config) {
107113
comp_err(dev, "invalid channels count");
108114
return -EINVAL;
109115
}
116+
/* channels_in_config indexes into a int16_t array. An odd count would
117+
* leave the coefficient area at a 2-byte alignment, breaking the
118+
* 4-byte aligned int32_t loads in the optimized FIR kernels.
119+
*/
120+
if (config->channels_in_config & 0x1) {
121+
comp_err(dev, "channels_in_config %u must be even",
122+
config->channels_in_config);
123+
return -EINVAL;
124+
}
110125
if (config->number_of_responses > SOF_EQ_FIR_MAX_RESPONSES) {
111126
comp_err(dev, "# of resp exceeds max");
112127
return -EINVAL;
113128
}
114129

130+
/* Compute the size of the coefficient area in int16_t words from the
131+
* blob's self-declared size. The blob layout is:
132+
* sizeof(*config) header bytes
133+
* channels_in_config int16_t assign_response[]
134+
* coefficient data[]
135+
*/
136+
if (config->size < sizeof(*config) ||
137+
config->size - sizeof(*config) <
138+
(size_t)config->channels_in_config * sizeof(int16_t)) {
139+
comp_err(dev, "config size %u too small", config->size);
140+
return -EINVAL;
141+
}
142+
coef_words_max = (config->size - sizeof(*config)) / sizeof(int16_t) -
143+
config->channels_in_config;
144+
115145
/* Collect index of response start positions in all_coefficients[] */
116146
j = 0;
117147
assign_response = ASSUME_ALIGNED(&config->data[0], 4);
118-
coef_data = ASSUME_ALIGNED(&config->data[config->channels_in_config],
119-
4);
148+
coef_data = ASSUME_ALIGNED(&config->data[config->channels_in_config], 4);
120149
for (i = 0; i < SOF_EQ_FIR_MAX_RESPONSES; i++) {
121150
if (i < config->number_of_responses) {
151+
/* Header must fit before reading length */
152+
if (j + SOF_FIR_COEF_NHEADER > coef_words_max) {
153+
comp_err(dev, "response %d header out of bounds", i);
154+
return -EINVAL;
155+
}
122156
eq = (struct sof_fir_coef_data *)&coef_data[j];
157+
/* Bound length so it is valid and the coefficient data
158+
* stays within the blob.
159+
*/
160+
if (eq->length <= 0 || eq->length > SOF_FIR_MAX_LENGTH ||
161+
(eq->length & 0x3) ||
162+
j + SOF_FIR_COEF_NHEADER + eq->length > coef_words_max) {
163+
comp_err(dev, "response %d length %d out of bounds",
164+
i, eq->length);
165+
return -EINVAL;
166+
}
123167
lookup[i] = eq;
124-
j += SOF_FIR_COEF_NHEADER + coef_data[j];
168+
j += SOF_FIR_COEF_NHEADER + eq->length;
125169
} else {
126170
lookup[i] = NULL;
127171
}
@@ -209,7 +253,7 @@ static int eq_fir_setup(struct processing_module *mod, int nch)
209253
cd->nch = nch;
210254

211255
/* Set coefficients for each channel EQ from coefficient blob */
212-
delay_size = eq_fir_init_coef(dev, cd->config, cd->fir, nch);
256+
delay_size = eq_fir_init_coef(dev, cd->config, cd->config_size, cd->fir, nch);
213257
if (delay_size < 0)
214258
return delay_size; /* Contains error code */
215259

@@ -234,9 +278,25 @@ static int eq_fir_setup(struct processing_module *mod, int nch)
234278
return 0;
235279
}
236280

281+
static int eq_fir_check_blob_size(struct comp_dev *dev, size_t size)
282+
{
283+
if (size < sizeof(struct sof_eq_fir_config) || size > SOF_EQ_FIR_MAX_SIZE) {
284+
comp_err(dev, "invalid configuration blob, size %zu", size);
285+
return -EINVAL;
286+
}
287+
288+
return 0;
289+
}
290+
237291
static int eq_fir_validator(struct comp_dev *dev, void *new_data, uint32_t new_data_size)
238292
{
239-
return eq_fir_init_coef(dev, new_data, NULL, -1);
293+
int ret;
294+
295+
ret = eq_fir_check_blob_size(dev, new_data_size);
296+
if (ret < 0)
297+
return ret;
298+
299+
return eq_fir_init_coef(dev, new_data, new_data_size, NULL, -1);
240300
}
241301

242302
/*
@@ -332,7 +392,9 @@ static int eq_fir_process(struct processing_module *mod,
332392

333393
/* Check for changed configuration */
334394
if (comp_is_new_data_blob_available(cd->model_handler)) {
335-
cd->config = comp_get_data_blob(cd->model_handler, NULL, NULL);
395+
cd->config = comp_get_data_blob(cd->model_handler, &cd->config_size, NULL);
396+
if (!cd->config || eq_fir_check_blob_size(mod->dev, cd->config_size) < 0)
397+
return -EINVAL;
336398
ret = eq_fir_setup(mod, audio_stream_get_channels(source));
337399
if (ret < 0) {
338400
comp_err(mod->dev, "failed FIR setup");
@@ -384,7 +446,6 @@ static int eq_fir_prepare(struct processing_module *mod,
384446
int channels;
385447
enum sof_ipc_frame frame_fmt;
386448
int ret = 0;
387-
size_t data_size;
388449

389450
comp_dbg(dev, "entry");
390451

@@ -407,8 +468,11 @@ static int eq_fir_prepare(struct processing_module *mod,
407468
frame_fmt = audio_stream_get_frm_fmt(&sourceb->stream);
408469

409470
cd->eq_fir_func = eq_fir_passthrough;
410-
cd->config = comp_get_data_blob(cd->model_handler, &data_size, NULL);
411-
if (cd->config && data_size > 0) {
471+
cd->config = comp_get_data_blob(cd->model_handler, &cd->config_size, NULL);
472+
if (cd->config) {
473+
if (eq_fir_check_blob_size(dev, cd->config_size) < 0)
474+
return -EINVAL;
475+
412476
ret = eq_fir_setup(mod, channels);
413477
if (ret < 0)
414478
comp_err(dev, "eq_fir_setup failed.");

src/audio/eq_fir/eq_fir.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ struct comp_data {
3636
struct comp_data_blob_handler *model_handler;
3737
struct sof_eq_fir_config *config;
3838
int32_t *fir_delay; /**< pointer to allocated RAM */
39+
size_t config_size; /**< configuration size */
3940
size_t fir_delay_size; /**< allocated size */
4041
void (*eq_fir_func)(struct fir_state_32x16 fir[],
4142
struct input_stream_buffer *bsource,

0 commit comments

Comments
 (0)