audio: tdfb: Improve robustness for invalid configuration by singalsu · Pull Request #10877 · thesofproject/sof

singalsu · 2026-06-11T13:52:04Z

Add several validity checks against malformed configuration blobs and malformed IPC control payloads.

Copilot

Pull request overview

This PR hardens the TDFB (time-domain fixed beamformer) module against malformed configuration blobs and malformed IPC3 control payloads, aiming to prevent invalid offset arithmetic and out-of-bounds accesses when parsing untrusted inputs.

Changes:

Track and validate configuration blob size end-to-end (cd->config_size) and add multiple config field validity checks (e.g., angle_enum_mult, filter_index, negative channel selects).
Add IPC3 GET handler validation for cdata->num_elems to avoid overrunning the control payload.
Increase the stack array used in theoretical time-difference calculation to support up to SOF_TDFB_MAX_MICROPHONES.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
src/audio/tdfb/tdfb.c	Adds blob-size tracking and additional config sanity checks during coefficient init and runtime blob updates.
src/audio/tdfb/tdfb_ipc3.c	Rejects invalid `num_elems` values in IPC3 GET handling to prevent payload overruns.
src/audio/tdfb/tdfb_direction.c	Enlarges a local array to support up to 16 microphone locations.
src/audio/tdfb/tdfb_comp.h	Adds `config_size` field to persist blob size for validation.

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 <seppo.ingalsuo@linux.intel.com>

Copilot

Pull request overview

Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.

Comments suppressed due to low confidence (1)

src/audio/tdfb/tdfb.c:380

tdfb_filter_seek() walks num_filters coefficient blocks by reading each sof_fir_coef_data->length from the blob, but it has no bounds checking against config->size/cd->config_size. With a malformed blob (e.g., a large length), this can advance coefp past the end of the allocation and then dereference out-of-bounds in the next iteration, before the later end-of-blob size check runs.

	/* Skip filter coefficients */
	num_filters = config->num_filters * (config->num_angles + config->beam_off_defined);
	coefp = tdfb_filter_seek(config, num_filters);

singalsu · 2026-06-12T07:50:08Z

+	if (!config->angle_enum_mult) {
+		comp_err(dev, "invalid angle_enum_mult");
+		return -EINVAL;
+	}


With negative multiplier it's possible to generate a descending angle scale. It could be useful, no need to limit the usage. The angles are negative and positive any way via the angle offset parameter.

singalsu · 2026-06-12T07:48:38Z

+	if (config->size != cd->config_size) {
+		comp_err(dev, "Incorrect configuration blob size");
+		return -EINVAL;
+	}


Since the blobs are created with a tool (not manually) the size mismatch is very unlikely. No need for more details.

singalsu · 2026-06-12T07:51:23Z

 	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;
+		}


I propose to add the blob validator later. I have started the work but it won't be ready before summer vacations that this PR targets for.

ujfalusi · 2026-06-12T08:26:21Z

 	struct tdfb_comp_data *cd = module_get_private_data(mod);

+	if (cdata->num_elems == 0 || cdata->num_elems > SOF_IPC_MAX_CHANNELS)
+		return -EINVAL;


only bytes controls will have num_elems as 0 as the hos does not know the size, for any other controls the host sets num_elems: data->num_elems = scontrol->num_channels;, the num_channels comes from topology and only checked against SND_SOC_TPLG_MAX_CHAN, no check for 0 anywhere, not even in core, a control is allowed to have 0 channels?

I'm not even sure the num_elems strictly means number of stream channels. It might be e.g. graphical EQ bands number that could be larger than max. channels count, e.g. 10 vs. 8. Can the cdata buffer allocation be too small for the num_elems ever?

it is allocated based on the num_channels:
https://github.com/thesofproject/linux/blob/topic/sof-dev/sound/soc/sof/ipc4-control.c#L214

With current implementation of the control the components generally can decide the usage, the control can be per channel, or as "mono" a control for all channels, or with non-associated to channels something else like the graphical EQ bands controls. In this case with the current IIR implementation, the SOF_IPC_MAX_CHANNELS check isn't limiting any functionality so it can be kept.

atm, lets keep the validation in the module for valid channel count in kcontrols as they will be different.

singalsu marked this pull request as ready for review June 11, 2026 14:19

Copilot AI review requested due to automatic review settings June 11, 2026 14:19

Copilot started reviewing on behalf of singalsu June 11, 2026 14:19 View session

Copilot AI reviewed Jun 11, 2026

View reviewed changes

Comment thread src/audio/tdfb/tdfb.c

Comment thread src/audio/tdfb/tdfb.c

Comment thread src/audio/tdfb/tdfb_direction.c

singalsu force-pushed the tdfb_updates branch from 7342818 to 19ca585 Compare June 11, 2026 16:40

singalsu requested a review from Copilot June 11, 2026 16:47

Copilot started reviewing on behalf of singalsu June 11, 2026 16:47 View session

Copilot AI reviewed Jun 11, 2026

View reviewed changes

ujfalusi reviewed Jun 12, 2026

View reviewed changes

kv2019i approved these changes Jun 12, 2026

View reviewed changes

lgirdwood approved these changes Jun 12, 2026

View reviewed changes

lgirdwood merged commit 569e54f into thesofproject:main Jun 12, 2026
45 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

audio: tdfb: Improve robustness for invalid configuration#10877

audio: tdfb: Improve robustness for invalid configuration#10877
lgirdwood merged 1 commit into
thesofproject:mainfrom
singalsu:tdfb_updates

singalsu commented Jun 11, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

singalsu Jun 12, 2026

Uh oh!

singalsu Jun 12, 2026

Uh oh!

singalsu Jun 12, 2026

Uh oh!

ujfalusi Jun 12, 2026

Uh oh!

singalsu Jun 12, 2026

Uh oh!

ujfalusi Jun 12, 2026

Uh oh!

singalsu Jun 12, 2026

Uh oh!

lgirdwood Jun 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Conversation

singalsu commented Jun 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

singalsu commented Jun 11, 2026 •

edited

Loading