Audio: Fix the init() in modules to not initialize bytes control with invalid data by singalsu · Pull Request #10646 · thesofproject/sof

singalsu · 2026-03-23T17:13:27Z

No description provided.

This patch moves the configuration blob out of the IPC init message in create_eq_iir_comp_ipc(). It adds a new eq_iir_send_config() function that sends the blob via the module's set_configuration() callback and calls it from setup() after component creation. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

Move the configuration blob out of the IPC init message in create_eq_fir_comp_ipc(). Add a new eq_fir_send_config() function that sends the blob via the module's set_configuration() callback, and call it from setup() after component creation. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

lyakh · 2026-03-24T11:58:39Z

src/audio/eq_iir/eq_iir.c

-	if (ret < 0) {
-		comp_err(dev, "comp_init_data_blob() failed with error: %d", ret);
-		goto err;
+		mod_data_blob_handler_free(mod, cd->model_handler);


doesn't look right - the allocation failed, so no need to free?

Oops! Good catch!

lyakh · 2026-03-24T12:04:16Z

src/audio/multiband_drc/multiband_drc.c

 		comp_err(dev, "comp_data_blob_handler_new() failed.");
-		ret = -ENOMEM;
-		goto cd_fail;
+		mod_data_blob_handler_free(mod, cd->model_handler);


ditto - allocation failed

Copilot

Pull request overview

This PR adjusts how IPC4 testbench/topology and several audio modules handle bytes-control configuration blobs, preventing modules from attempting to initialize/apply invalid or absent binary data during init() and ensuring configuration is only applied when a valid blob exists.

Changes:

Testbench IPC4: skip sending bytes controls when the parsed bytes payload is missing/too small to contain an ABI header.
Modules: stop initializing data-blob handlers from md->cfg during init() and instead gate configuration application in prepare() based on retrieved blob size.
Cmocka EQ tests: stop embedding coefficients in the component IPC creation and instead send configuration via set_configuration() before prepare().

Reviewed changes

Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
tools/testbench/utils_ipc4.c	Avoids sending bytes-control IPC when `ctl->data` is NULL.
tools/testbench/topology_ipc4.c	Only assigns `ctl->data` for bytes controls when at least an ABI header is present.
test/cmocka/src/audio/eq_iir/eq_iir_process.c	Sends EQ IIR config via control path instead of embedding it into init IPC.
test/cmocka/src/audio/eq_fir/eq_fir_process.c	Sends EQ FIR config via control path instead of embedding it into init IPC.
src/audio/tdfb/tdfb.c	Removes init-time blob initialization; attempts to gate blob usage in `prepare()`.
src/audio/multiband_drc/multiband_drc.c	Removes init-time blob initialization; applies config only when a blob is present in `prepare()`.
src/audio/mfcc/mfcc.c	Removes init-time blob initialization; applies config only when a blob is present in `prepare()`.
src/audio/eq_iir/eq_iir.c	Removes init-time blob initialization; applies config only when a blob is present in `prepare()`.
src/audio/eq_fir/eq_fir.c	Removes init-time blob initialization; applies config only when a blob is present in `prepare()`.
src/audio/drc/drc.c	Removes init-time blob initialization; applies config only when a blob is present in `prepare()`.
src/audio/dcblock/dcblock.c	Removes init-time blob initialization; uses passthrough when no blob is present in `prepare()`.
src/audio/crossover/crossover.c	Removes init-time blob initialization; validates/applies config in `prepare()` when present, otherwise passthrough.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-24T12:12:27Z

src/audio/tdfb/tdfb.c

-	cd->config = comp_get_data_blob(cd->model_handler, NULL, NULL);
-	if (!cd->config) {
+	cd->config = comp_get_data_blob(cd->model_handler, &data_size, NULL);
+	if (!cd->config && data_size > 0) {


In tdfb_prepare(), cd->config can be NULL after comp_get_data_blob(). The new guard if (!cd->config && data_size > 0) will never catch the “missing config” case because comp_get_data_blob() sets *size to 0 when returning NULL, so the code proceeds to tdfb_setup() and later dereferences cd->config (e.g., in tdfb_init_coef()/logging), risking a NULL-deref. Please either treat missing config as an error (return -EINVAL) before calling tdfb_setup(), or add an explicit passthrough/disabled path when no blob is present.

Suggested change

if (!cd->config && data_size > 0) {

if (!cd->config) {

comp_err(dev, "tdfb_prepare(): missing config data");

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-24T13:41:17Z

tools/testbench/topology_ipc4.c

+		if (tplg_bytes->priv.size >= sizeof(struct sof_abi_hdr))
+			ctl->data = tplg_bytes->priv.data;


The new guard only checks tplg_bytes->priv.size >= sizeof(struct sof_abi_hdr) before storing ctl->data, but later tb_send_bytes_data() trusts abi->size and copies abi->size bytes from abi->data. If the topology contains a malformed ABI header where abi->size is larger than the actual priv.size - sizeof(struct sof_abi_hdr), this will read past the bytes-control buffer. Please also validate that tplg_bytes->priv.size >= sizeof(struct sof_abi_hdr) + abi->size (and possibly that abi->size is within TB_MAX_BYTES_DATA_SIZE) before accepting the control data.

Suggested change

if (tplg_bytes->priv.size >= sizeof(struct sof_abi_hdr))

ctl->data = tplg_bytes->priv.data;

if (tplg_bytes->priv.size >= sizeof(struct sof_abi_hdr)) {

struct sof_abi_hdr *abi =

(struct sof_abi_hdr *)tplg_bytes->priv.data;

if (abi->size > TB_MAX_BYTES_DATA_SIZE) {

fprintf(stderr,

"Error: ABI payload size %u exceeds max %d\n",

abi->size, TB_MAX_BYTES_DATA_SIZE);

return -EINVAL;

}

if (tplg_bytes->priv.size <

sizeof(struct sof_abi_hdr) + abi->size) {

fprintf(stderr,

"Error: bytes data size %d is smaller than ABI header + payload (%zu + %u)\n",

tplg_bytes->priv.size,

sizeof(struct sof_abi_hdr), abi->size);

return -EINVAL;

}

ctl->data = tplg_bytes->priv.data;

}

Copilot · 2026-03-24T13:41:18Z

src/audio/crossover/crossover.c

 		/* If config is invalid then delete it */
 		comp_err(dev, "invalid binary config format");
 		crossover_free_config(&cd->config);


cd->config comes directly from comp_get_data_blob(cd->model_handler, ...), so its lifetime is owned by the data-blob handler. Calling crossover_free_config(&cd->config) here rfree()s that buffer, leaving the blob handler holding a dangling pointer and risking UAF/double-free on the next comp_get_data_blob()/handler cleanup. Instead of freeing cd->config directly, either (a) just set cd->config = NULL and ignore the invalid blob, or (b) clear it via the blob-handler API (e.g., comp_init_data_blob(cd->model_handler, 0, NULL)), and consider adding a blob validator so invalid configs are rejected before becoming current.

Suggested change

/* If config is invalid then delete it */

comp_err(dev, "invalid binary config format");

crossover_free_config(&cd->config);

/* If config is invalid then delete it via blob handler */

comp_err(dev, "invalid binary config format");

comp_init_data_blob(cd->model_handler, 0, NULL);

cd->config = NULL;

It's better to fail prepare().

…data The change in removing bytes control blob in module init() triggered an issue in sof-testbench4. It resulted in valgrind fail with scripts/host-testbench.sh run with error "Invalid read of size 1" in "memcpy(msg + sizeof(config), (char *)abi->data + offset, chunk_size)". The invalid read happens when abi->data doesn't have chunk_size bytes available. The fix is to skip bytes controls with no private data to avoid reading garbage abi->size from adjacent topology buffer data, which causes invalid memory reads in tb_send_bytes_data(). Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

The pass of bytes control blob from topology in init() was used in some early IPC3 kernels but with add of support for multiple controls blob it was changed to use normal controls IPC. With IPC4 the module configuration data is not for ALSA controls while some modules still handled it as such. If a topology does not contain a blob to initialize the control, an invalid blob is attempted to be used in prepare(). The prepare() then fails with invalid blob detected while the modules would normally result to pass-through mode when not configured. In addition to code removal a check is added to prepare() for the data_size from comp_get_data_blob() to ensure the configuration data is not zero size. This patch fixes the similar issue in crossover, dcblock, drc, fir, iir, mfcc, multiband-drc, and tdfb. Signed-off-by: Seppo Ingalsuo <seppo.ingalsuo@linux.intel.com>

Copilot

Pull request overview

Copilot reviewed 12 out of 12 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (2)

src/audio/mfcc/mfcc.c:201

In mfcc_prepare(), if the configuration blob is missing/empty then mfcc_setup() is skipped, but mfcc_process() will still call mfcc_s16_default() via cd->mfcc_func. mfcc_s16_default() uses fields/buffers initialized by mfcc_setup() (e.g., FFT plan/buffers), so proceeding without a valid blob risks NULL-deref or invalid memory access. Consider failing prepare() (e.g., -EINVAL) when no valid config is present, or explicitly switching to a safe “disabled” processing path that does not rely on mfcc_setup()-initialized state.

	cd->config = comp_get_data_blob(cd->model_handler, &data_size, NULL);

	/* Initialize MFCC, max_frames is set to dev->frames + 4 */
	if (cd->config && data_size > 0) {
		ret = mfcc_setup(mod, dev->frames + 4, audio_stream_get_rate(&sourceb->stream),
				 audio_stream_get_channels(&sourceb->stream));
		if (ret < 0) {
			comp_err(dev, "setup failed.");
			goto err;
		}
	}

src/audio/multiband_drc/multiband_drc.c:378

multiband_drc_prepare() currently continues when the config blob is missing/empty (cd->config == NULL or size == 0), but multiband_drc_process() will call cd->multiband_drc_func when process_enabled is true. The default multiband_drc_* processing functions dereference cd->config unconditionally (e.g., cd->config->num_bands), so running without a valid blob can crash. Suggest either failing prepare() when no blob is present, or forcing a safe bypass path when config is absent (e.g., set process_enabled=false and/or select the *_pass function map).

	/* Initialize DRC */
	comp_dbg(dev, "source_format=%d, sink_format=%d",
		 cd->source_format, cd->source_format);
	cd->config = comp_get_data_blob(cd->model_handler, &data_size, NULL);
	if (cd->config && data_size > 0) {
		ret = multiband_drc_setup(mod, channels, rate);
		if (ret < 0) {
			comp_err(dev, "error: multiband_drc_setup failed.");
			return ret;
		}
	}

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

singalsu · 2026-03-24T15:26:35Z

Note: Related kernel patch is thesofproject/linux#5708 .

singalsu added 2 commits March 24, 2026 13:23

singalsu force-pushed the fix_modules_no_blob_in_init branch from f9b0c16 to fc8e5cb Compare March 24, 2026 11:58

singalsu marked this pull request as ready for review March 24, 2026 12:02

singalsu requested review from a team, dbaluta, kv2019i, lbetlej, lgirdwood, mmaka1, plbossart and ranj063 as code owners March 24, 2026 12:02

Copilot AI review requested due to automatic review settings March 24, 2026 12:02

lyakh reviewed Mar 24, 2026

View reviewed changes

Copilot started reviewing on behalf of singalsu March 24, 2026 12:05 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

singalsu force-pushed the fix_modules_no_blob_in_init branch from fc8e5cb to 87fa65d Compare March 24, 2026 13:25

singalsu requested a review from Copilot March 24, 2026 13:25

Copilot started reviewing on behalf of singalsu March 24, 2026 13:31 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

singalsu added 2 commits March 24, 2026 16:55

singalsu force-pushed the fix_modules_no_blob_in_init branch 2 times, most recently from 87fa65d to a7ffffa Compare March 24, 2026 14:57

singalsu requested a review from Copilot March 24, 2026 14:57

Copilot started reviewing on behalf of singalsu March 24, 2026 15:00 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

singalsu requested a review from lyakh March 24, 2026 15:23

	if (!cd->config && data_size > 0) {
	if (!cd->config) {
	comp_err(dev, "tdfb_prepare(): missing config data");

		if (tplg_bytes->priv.size >= sizeof(struct sof_abi_hdr))
		ctl->data = tplg_bytes->priv.data;

-		if (tplg_bytes->priv.size >= sizeof(struct sof_abi_hdr))
-			ctl->data = tplg_bytes->priv.data;
+		if (tplg_bytes->priv.size >= sizeof(struct sof_abi_hdr)) {
+			struct sof_abi_hdr *abi =
+				(struct sof_abi_hdr *)tplg_bytes->priv.data;
+			if (abi->size > TB_MAX_BYTES_DATA_SIZE) {
+				fprintf(stderr,
+					"Error: ABI payload size %u exceeds max %d\n",
+					abi->size, TB_MAX_BYTES_DATA_SIZE);
+				return -EINVAL;
+			}
+			if (tplg_bytes->priv.size <
+			    sizeof(struct sof_abi_hdr) + abi->size) {
+				fprintf(stderr,
+					"Error: bytes data size %d is smaller than ABI header + payload (%zu + %u)\n",
+					tplg_bytes->priv.size,
+					sizeof(struct sof_abi_hdr), abi->size);
+				return -EINVAL;
+			}
+			ctl->data = tplg_bytes->priv.data;
+		}

Conversation

singalsu commented Mar 23, 2026

Uh oh!

lyakh Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

singalsu Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

lyakh Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

singalsu Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

singalsu Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

singalsu Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

singalsu commented Mar 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants