From 19ca5857da2ddd41d40172ecb5aef3a9fedd1093 Mon Sep 17 00:00:00 2001 From: Seppo Ingalsuo Date: Thu, 11 Jun 2026 12:17:24 +0300 Subject: [PATCH] audio: tdfb: Improve robustness for invalid configuration Add validity checks against malformed configuration blobs and IPC control payloads: - Bound the blob length reported by comp_get_data_blob() to [sizeof(*cd->config), SOF_TDFB_MAX_SIZE] in both tdfb_prepare and the runtime new-blob path; store it in cd->config_size and require config->size to match in tdfb_init_coef. - Bump SOF_TDFB_MAX_SIZE 4096 -> 16384 to match the topology budget. - Tie SOF_TDFB_MAX_MICROPHONES to PLATFORM_MAX_CHANNELS so num_mic_locations cannot exceed the per-channel direction state arrays in tdfb_comp.h. - Reject num_angles == 0, angle_enum_mult == 0, negative or out-of-range filter_index, and negative input_channel_select[]. - Check cdata->num_elems vs. SOF_IPC_MAX_CHANNELS in the IPC3 GET handler. Signed-off-by: Seppo Ingalsuo --- src/audio/tdfb/tdfb.c | 51 +++++++++++++++++++++++---------- src/audio/tdfb/tdfb.h | 5 ++-- src/audio/tdfb/tdfb_comp.h | 1 + src/audio/tdfb/tdfb_direction.c | 2 +- src/audio/tdfb/tdfb_ipc3.c | 3 ++ 5 files changed, 44 insertions(+), 18 deletions(-) diff --git a/src/audio/tdfb/tdfb.c b/src/audio/tdfb/tdfb.c index 5df4563c3558..1cda385cae84 100644 --- a/src/audio/tdfb/tdfb.c +++ b/src/audio/tdfb/tdfb.c @@ -323,6 +323,11 @@ static int tdfb_init_coef(struct processing_module *mod, int source_nch, int i; /* Sanity checks */ + if (config->size != cd->config_size) { + comp_err(dev, "Incorrect configuration blob size"); + return -EINVAL; + } + if (config->num_output_channels > PLATFORM_MAX_CHANNELS || !config->num_output_channels) { comp_err(dev, "invalid num_output_channels %d", @@ -342,12 +347,21 @@ static int tdfb_init_coef(struct processing_module *mod, int source_nch, return -EINVAL; } - if (config->num_angles > SOF_TDFB_MAX_ANGLES) { + /* In SOF v1.6 - 1.8 based beamformer topologies the multiple angles, mic locations, + * and beam on/off switch were not defined. A most basic supported blob has num_angles + * equal to 1. Mic locations data is optional. + */ + if (config->num_angles == 0 || config->num_angles > SOF_TDFB_MAX_ANGLES) { comp_err(dev, "invalid num_angles %d", config->num_angles); return -EINVAL; } + if (!config->angle_enum_mult) { + comp_err(dev, "invalid angle_enum_mult"); + return -EINVAL; + } + if (config->beam_off_defined > 1) { comp_err(dev, "invalid beam_off_defined %d", config->beam_off_defined); @@ -360,15 +374,6 @@ static int tdfb_init_coef(struct processing_module *mod, int source_nch, return -EINVAL; } - /* In SOF v1.6 - 1.8 based beamformer topologies the multiple angles, mic locations, - * and beam on/off switch were not defined. Return error if such configuration is seen. - * A most basic blob has num_angles equals 1. Mic locations data is optional. - */ - if (config->num_angles == 0 && config->num_mic_locations == 0) { - comp_err(dev, "ABI version less than 3.19.1 is not supported."); - return -EINVAL; - } - /* Skip filter coefficients */ num_filters = config->num_filters * (config->num_angles + config->beam_off_defined); coefp = tdfb_filter_seek(config, num_filters); @@ -423,6 +428,12 @@ static int tdfb_init_coef(struct processing_module *mod, int source_nch, cd->filter_angles[min_delta_idx].azimuth, idx); } + if (idx < 0 || idx + (int)config->num_filters > num_filters) { + comp_err(dev, "invalid filter_index %d for angle %d", + idx, cd->filter_angles[min_delta_idx].azimuth); + return -EINVAL; + } + /* Seek to proper filter for requested angle or beam off configuration */ coefp = tdfb_filter_seek(config, idx); @@ -451,6 +462,11 @@ static int tdfb_init_coef(struct processing_module *mod, int source_nch, for (i = 0; i < config->num_filters; i++) { if (cd->input_channel_select[i] > max_ch) max_ch = cd->input_channel_select[i]; + + if (cd->input_channel_select[i] < 0) { + comp_err(dev, "invalid channel select for filter %d", i); + return -EINVAL; + } } /* The stream must contain at least the number of channels that is @@ -641,7 +657,12 @@ static int tdfb_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 || cd->config_size < sizeof(*cd->config) || + cd->config_size > SOF_TDFB_MAX_SIZE) { + comp_err(dev, "invalid configuration blob, size %zu", cd->config_size); + return -EINVAL; + } ret = tdfb_setup(mod, audio_stream_get_channels(source), audio_stream_get_channels(sink), audio_stream_get_frm_fmt(source)); @@ -705,7 +726,6 @@ static int tdfb_prepare(struct processing_module *mod, struct comp_buffer *sourceb, *sinkb; struct comp_dev *dev = mod->dev; enum sof_ipc_frame frame_fmt; - size_t data_size; int source_channels; int sink_channels; int rate; @@ -735,9 +755,10 @@ static int tdfb_prepare(struct processing_module *mod, rate = audio_stream_get_rate(&sourceb->stream); /* Initialize filter */ - cd->config = comp_get_data_blob(cd->model_handler, &data_size, NULL); - if (!cd->config || !data_size) { - comp_err(dev, "Missing a configuration blob."); + cd->config = comp_get_data_blob(cd->model_handler, &cd->config_size, NULL); + if (!cd->config || cd->config_size < sizeof(*cd->config) || + cd->config_size > SOF_TDFB_MAX_SIZE) { + comp_err(dev, "invalid configuration blob, size %zu", cd->config_size); ret = -EINVAL; goto out; } diff --git a/src/audio/tdfb/tdfb.h b/src/audio/tdfb/tdfb.h index d5a09776301f..c6fd1ff0f97d 100644 --- a/src/audio/tdfb/tdfb.h +++ b/src/audio/tdfb/tdfb.h @@ -8,16 +8,17 @@ #ifndef __USER_TDFB_H__ #define __USER_TDFB_H__ +#include #include #define SOF_TDFB_NUM_INPUT_PINS 1 /* One source */ #define SOF_TDFB_NUM_OUTPUT_PINS 1 /* One sink */ -#define SOF_TDFB_MAX_SIZE 4096 /* Max size for coef data in bytes */ +#define SOF_TDFB_MAX_SIZE 16384 /* Max size for coef data in bytes */ #define SOF_TDFB_FIR_MAX_LENGTH 256 /* Max length for individual filter */ #define SOF_TDFB_FIR_MAX_COUNT 16 /* A blob can define max 16 FIR EQs */ #define SOF_TDFB_MAX_STREAMS 8 /* Support 1..8 sinks */ #define SOF_TDFB_MAX_ANGLES 360 /* Up to 1 degree precision for 360 degrees coverage */ -#define SOF_TDFB_MAX_MICROPHONES 16 /* Up to 16 microphone locations */ +#define SOF_TDFB_MAX_MICROPHONES PLATFORM_MAX_CHANNELS /* Bounded by direction state arrays */ /* The driver assigns running numbers for control index. If there's single control of * type switch, enum, binary they all have index 0. diff --git a/src/audio/tdfb/tdfb_comp.h b/src/audio/tdfb/tdfb_comp.h index ef33f28fa612..9e0727474f4a 100644 --- a/src/audio/tdfb/tdfb_comp.h +++ b/src/audio/tdfb/tdfb_comp.h @@ -88,6 +88,7 @@ struct tdfb_comp_data { int16_t *output_stream_mix; /**< for each FIR define stream */ int16_t az_value; /**< beam steer azimuth as in control enum */ int16_t az_value_estimate; /**< beam steer azimuth as in control enum */ + size_t config_size; /**< size of the configuration blob */ size_t fir_delay_size; /**< allocated size */ unsigned int max_frames; /**< max frames to process */ bool direction_updates:1; /**< set true if direction angle control is updated */ diff --git a/src/audio/tdfb/tdfb_direction.c b/src/audio/tdfb/tdfb_direction.c index 71b544003ee7..bd435fa2ebd3 100644 --- a/src/audio/tdfb/tdfb_direction.c +++ b/src/audio/tdfb/tdfb_direction.c @@ -386,7 +386,7 @@ static int16_t distance_from_source(struct tdfb_comp_data *cd, int mic_n, static void theoretical_time_differences(struct tdfb_comp_data *cd, int16_t az) { - int16_t d[PLATFORM_MAX_CHANNELS]; + int16_t d[SOF_TDFB_MAX_MICROPHONES]; int16_t src_x; int16_t src_y; int16_t sin_az; diff --git a/src/audio/tdfb/tdfb_ipc3.c b/src/audio/tdfb/tdfb_ipc3.c index 544acbe9a41f..1ebdb9641ccf 100644 --- a/src/audio/tdfb/tdfb_ipc3.c +++ b/src/audio/tdfb/tdfb_ipc3.c @@ -112,6 +112,9 @@ static int tdfb_cmd_get_value(struct processing_module *mod, struct sof_ipc_ctrl { struct tdfb_comp_data *cd = module_get_private_data(mod); + if (cdata->num_elems == 0 || cdata->num_elems > SOF_IPC_MAX_CHANNELS) + return -EINVAL; + switch (cdata->cmd) { case SOF_CTRL_CMD_ENUM: comp_dbg(mod->dev, "SOF_CTRL_CMD_ENUM index=%d",