Ipc user handler security fixes#10878
Open
jsarha wants to merge 2 commits into
Open
Conversation
jsarha
requested review from
dbaluta,
kv2019i,
lbetlej,
lgirdwood,
mmaka1 and
plbossart
as code owners
Contributor
There was a problem hiding this comment.
Pull request overview
This PR targets IPC4 user message handling hardening in src/ipc/ipc4/handler-user.c, focusing on safer handling of CHAIN_DMA lifecycle and vendor-config large payload parsing.
Changes:
- Removes an extra
list_item_del()in the CHAIN_DMA path after state processing. - Moves vendor-config
data_off_sizevalidation earlier to guard host-controlled size before use.
Comment on lines
+1109
to
+1111
| /* Validate the host-controlled payload size before any use or arithmetic. */ | ||
| if (data_off_size < sizeof(struct sof_tlv) || data_off_size > MAILBOX_HOSTBOX_SIZE) | ||
| return IPC4_INVALID_CONFIG_DATA_STRUCT; |
kv2019i
requested changes
Jun 11, 2026
kv2019i
left a comment
kv2019i
left a comment
Collaborator
There was a problem hiding this comment.
I think the copilot comment is right, the non-initial configs may be smaller than the tlv header, so this would reject valid IPCs...
added 2 commits
June 12, 2026 11:59
ipc4_set_vendor_config_module_instance() only validated data_off_size in the bursted-config path (init_block && final_block). The else path with init_block == 1 && final_block == 0 unconditionally executed: data += sizeof(struct sof_tlv); data_off_size -= sizeof(struct sof_tlv); data_off_size is a host-controlled 20-bit field taken straight from the IPC message. When it is smaller than sizeof(struct sof_tlv) (8) the subtraction underflows and wraps to a value close to 0xFFFFFFFF, which is then forwarded as the length to the module's set_large_config() handler. The actual backing buffer is only the MAILBOX_HOSTBOX_SIZE mailbox, so a compromised host could trigger out-of-bounds reads of DSP SRAM (and possible corruption depending on the target module) by sending MOD_LARGE_CONFIG_SET with init_block=1, final_block=0 and data_off_size < 8. Hoist the existing "data_off_size < sizeof(struct sof_tlv) || data_off_size > MAILBOX_HOSTBOX_SIZE" bounds check to the top of the function so it runs for every entry, before any pointer or size arithmetic. The duplicate check in the bursted-config branch is removed as it is now covered by the hoisted one. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
ipc4_process_chain_dma() called ipc4_chain_dma_state() and then, on the deallocate path (allocate == 0 && enable == 0), unconditionally executed list_item_del(&cdma_comp->list). However, on that same deallocate path ipc4_chain_dma_state() already unlinks the matching ipc_comp_dev from ipc->comp_list and frees it with rfree(): list_item_del(&icd->list); rfree(icd); Since icd is the same object as cdma_comp, the subsequent list_item_del(&cdma_comp->list) in the caller dereferenced and wrote to already-freed memory (prev->next / next->prev), a use-after-free. With heap grooming a host sending GLB_CHAIN_DMA with allocate=0/enable=0 on an existing chain could turn this into controlled heap corruption. The unlink-before-free is already handled correctly by ipc4_chain_dma_state(), so the duplicate list_item_del() in the caller is both redundant and unsafe. Remove it. Signed-off-by: Jyri Sarha <jyri.sarha@linux.intel.com>
jsarha
force-pushed
the
ipc_user_handler_security_fixes
branch
from
121da6d to
3ca57ac
Compare
lgirdwood
approved these changes
Jun 12, 2026
Comment on lines
+1113
to
+1116
| if (data_off_size > MAILBOX_HOSTBOX_SIZE) | ||
| return IPC4_INVALID_CONFIG_DATA_STRUCT; | ||
| if (init_block && data_off_size < sizeof(struct sof_tlv)) | ||
| return IPC4_INVALID_CONFIG_DATA_STRUCT; |
Member
There was a problem hiding this comment.
Can be fixed later or in update, but we need unique error LOG messages for each failure.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Couple of security fixes to src/ipc/ipc4/handler-user.c .