ipc4: base_fw: fix OOB reads and input validation in LargeConfig handlers by tmleman · Pull Request #10757 · thesofproject/sof

tmleman · 2026-05-08T13:44:10Z

Three bugs in the IPC4 base firmware LargeConfig handlers found during a static analysis audit (CodeQL, cppcheck, semgrep).

base_fw: libraries_info_get: fix sizeof(pointer) and index confusion
base_fw: validate IPC payload size before dereference in set handlers
ipc4: basefw_set_fw_config: validate TLV size

The first commit fixes a sizeof(pointer) misuse (CWE-467) and a mixed-index bug causing uninitialized fields in the library info reply.

The second commit adds minimum payload size guards to six LargeConfigSet handlers that dereferenced the IPC payload without checking the host-supplied data_off_size (CWE-125 / CWE-20).

The third commit validates the TLV header and value length in basefw_set_fw_config() before accessing any TLV fields (CWE-125).

Two bugs in basefw_libraries_info_get() are fixed: 1. sizeof(pointer) misuse (CWE-467): The reply size was computed with sizeof(libs_info) where libs_info is a pointer to struct ipc4_libraries_info. On 32-bit Xtensa targets the pointer and the struct header are both 4 bytes, so the bug was latent. On 64-bit builds (host simulation, future platforms) sizeof(pointer)=8 while sizeof(struct ipc4_libraries_info)=4, causing the reported reply size to be overstated by 4 bytes and potentially leaking uninitialized buffer content to the host. Fix: use sizeof(*libs_info) to always dereference to the struct size. 2. Mixed-index confusion causing uninitialized field writes: The .id field was written using libs_info->libraries[lib_id] while all other fields (name, version, num_module_entries) were written using libs_info->libraries[lib_counter]. When any intermediate library slot is absent (desc == NULL), lib_id advances past lib_counter, so the .id write lands in a different slot than the remaining fields. The result is a reply entry with an uninitialized .id (stale buffer content returned to the host) and an orphaned write into a slot beyond library_count. Fix: use lib_counter consistently for all field writes inside the loop. Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>

Copilot

Pull request overview

This PR hardens IPC4 base firmware LargeConfig handling by fixing incorrect size calculations/indexing and adding input-size validation to prevent out-of-bounds reads when parsing host-supplied payloads.

Changes:

Add minimum payload-size guards for several LargeConfigSet handlers before dereferencing incoming data buffers.
Fix library info reply population (indexing bug) and correct a sizeof(pointer) misuse in response sizing.
Add TLV header/value length validation in FW_CONFIG handling and improve the “unhandled type” warning message.

Reviewed changes

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

File	Description
src/audio/base_fw.c	Adds payload-size validation to multiple set handlers; fixes library info indexing and response size calculation.
src/audio/base_fw_intel.c	Adds TLV size validation for FW_CONFIG and input-size validation for mic privacy state change; improves warning message text.

+	if (data_offset < sizeof(struct sof_tlv) + tlv->length) {
+		tr_err(&basefw_comp_tr, "FW_CONFIG TLV value truncated: need %zu, have %u",
+		       sizeof(struct sof_tlv) + tlv->length, data_offset);


Multiple LargeConfigSet handlers for module_id=0 dereferenced the payload pointer without checking the host-supplied data_off_size. When data_off_size=0 the dcache invalidation is a no-op and handlers read stale mailbox content from the previous transaction. The fix is the same pattern across all affected handlers: add a minimum-size guard against the reported payload length before the first memory access, and return IPC4_ERROR_INVALID_PARAM on undersize input. Three handlers (set_perf_meas_state, io_perf_monitor_state_set, basefw_notification_mask_info) also had data_offset dropped by the dispatcher; those call sites are updated to pass it through. An additional sign-extension defect in basefw_mic_priv_state_changed() is fixed by casting *data through uint8_t before widening to uint32_t. Affected handlers and their minimum required sizes: set_perf_meas_state 1 byte (enum state) io_perf_monitor_state_set 1 byte (enum state) basefw_register_kcps 4 bytes (int32_t kcps) basefw_notification_mask_info 8 bytes (ipc4_notification_mask_info) basefw_resource_allocation_request 8 bytes (ipc4_resource_request) basefw_mic_priv_state_changed 1 byte (uint8_t, mic privacy only) Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>

basefw_set_fw_config() cast the IPC payload directly to struct sof_tlv* and read tlv->type without checking that the reported payload length (data_offset) was sufficient. A zero-length or undersized LargeConfigSet with param_id=IPC4_FW_CONFIG could cause the handler to access memory beyond the valid payload (CWE-125 / CWE-20). Three guards are added before any TLV field access: 1. data_offset < sizeof(struct sof_tlv) Rejects payloads too short to hold the type+length header (8 B). 2. data_offset < sizeof(struct sof_tlv) + tlv->length Rejects payloads where the declared value length exceeds the actual buffer, preventing OOB reads of tlv->value[]. 3. tlv->length < sizeof(uint32_t) for IPC4_DMI_FORCE_L1_EXIT Rejects a TLV whose value field is too small to contain the force flag read by fw_config_set_force_l1_exit(). The TLV pointer cast is moved to after the header-size check so it is never formed against an undersized buffer. The warning log for unhandled types is updated to include the type value to aid diagnostics. Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>

lyakh · 2026-05-11T06:38:42Z

 	libs_info->library_count = lib_counter;
 	*data_offset =
-		sizeof(libs_info) + libs_info->library_count * sizeof(libs_info->libraries[0]);
+		sizeof(*libs_info) + libs_info->library_count * sizeof(libs_info->libraries[0]);


I suppose this IPC4_LIBRARIES_INFO_GET IPC has never been used until now

lyakh · 2026-05-11T06:42:42Z

 }

-__cold int set_perf_meas_state(const char *data)
+__cold int set_perf_meas_state(uint32_t data_size, const char *data)


maybe change the type in these functions to const uint8_t *

lyakh · 2026-05-11T06:43:31Z


 #ifdef CONFIG_SOF_TELEMETRY
-	enum ipc4_perf_measurements_state_set state = *data;
+	if (data_size < sizeof(uint8_t)) {


and then use sizeof(*data) or sizeof(data[0])

kv2019i

Good fixes! I'd look at the enum-casting in patch 2. Not an issue added in this PR, but given the cast is modified here, better do it cleanly.

kv2019i · 2026-05-11T10:14:37Z

+		return IPC4_ERROR_INVALID_PARAM;
+	}
+
+	enum ipc4_perf_measurements_state_set state = *(const uint8_t *)data;


And is this right, forced cast to uint8_t and then enum? OTOH, the old code is suspect too, this is assuming stuff about "enum" that I don't think can be assumed.

kv2019i · 2026-05-11T10:16:33Z

 	}
-	tr_warn(&basefw_comp_tr, "returning success for Set FW_CONFIG without handling it");
+
+	tr_warn(&basefw_comp_tr, "Set FW_CONFIG: no handler for type %u", tlv->type);


If you add a linefeed above, I'd keep one empty line between function body and return to conform with codebase style.

Copilot AI review requested due to automatic review settings May 8, 2026 13:44

tmleman requested review from dbaluta, kv2019i, lbetlej, lgirdwood, mmaka1 and plbossart as code owners May 8, 2026 13:44

Copilot started reviewing on behalf of tmleman May 8, 2026 13:46 View session

Copilot AI reviewed May 8, 2026

View reviewed changes

Comment thread src/audio/base_fw_intel.c

Comment on lines +485 to +487

if (data_offset < sizeof(struct sof_tlv) + tlv->length) {

tr_err(&basefw_comp_tr, "FW_CONFIG TLV value truncated: need %zu, have %u",

sizeof(struct sof_tlv) + tlv->length, data_offset);

tmleman added 2 commits May 8, 2026 16:22

lyakh reviewed May 11, 2026

View reviewed changes

lyakh approved these changes May 11, 2026

View reviewed changes

kv2019i reviewed May 11, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ipc4: base_fw: fix OOB reads and input validation in LargeConfig handlers#10757

ipc4: base_fw: fix OOB reads and input validation in LargeConfig handlers#10757
tmleman wants to merge 3 commits intothesofproject:mainfrom
tmleman:topic/upstream/pr/ipc4/fix/base_fw

tmleman commented May 8, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

lyakh May 11, 2026

Uh oh!

lyakh May 11, 2026

Uh oh!

lyakh May 11, 2026

Uh oh!

kv2019i left a comment

Uh oh!

kv2019i May 11, 2026

Uh oh!

kv2019i May 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

tmleman commented May 8, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

lyakh May 11, 2026

Choose a reason for hiding this comment

Uh oh!

lyakh May 11, 2026

Choose a reason for hiding this comment

Uh oh!

lyakh May 11, 2026

Choose a reason for hiding this comment

Uh oh!

kv2019i left a comment

Choose a reason for hiding this comment

Uh oh!

kv2019i May 11, 2026

Choose a reason for hiding this comment

Uh oh!

kv2019i May 11, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants