audio: module_adapter: fix sizeof(pointer) and underflow in module_ext_init_decode#10748
Open
tmleman wants to merge 1 commit intothesofproject:mainfrom
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes a vulnerability chain in IPC4 module extended-init decoding that could underflow spec->size and allow out-of-bounds mailbox reads during module adapter initialization.
Changes:
- Validate
spec->sizeagainst the fullext_initheader (sizeof(*ext_init)) before dereferencing any fields. - Prevent unsigned underflow by checking the computed consumed-byte count before adjusting
spec->size. - Add a defense-in-depth upper bound in
module_adapter_init_data()to reject configs larger thanMAILBOX_HOSTBOX_SIZE.
Comment on lines
+39
to
+42
| /* Validate size before dereferencing ext_init pointer */ | ||
| if (spec->size < sizeof(*ext_init)) { | ||
| comp_cl_err(drv, "Size too small for ext init %zu < %zu", | ||
| spec->size, sizeof(ext_init)); | ||
| spec->size, sizeof(*ext_init)); |
Comment on lines
+114
to
+117
| if (consumed > spec->size) { | ||
| comp_cl_err(drv, "ext_init consumed more than spec->size (%zu > %zu)", | ||
| consumed, spec->size); | ||
| return -EINVAL; |
…t_init_decode Three weaknesses compose into a single chain in module_ext_init_decode() that allows a crafted IPC4 ModuleInit payload to corrupt spec->size and spec->data before they are consumed by module_adapter_init_data(). The size guard used the wrong sizeof operand: if (spec->size < sizeof(ext_init)) /* sizeof(pointer) = 4, not 12 */ This accepted any payload >= 4 bytes even though the struct header is 12 bytes. Additionally, ext_init->data_obj_array was dereferenced before the guard ran, allowing the object-walk loop to be skipped with no size validation. When the loop is skipped, the unconditional spec->size adjustment: spec->size -= (unsigned char *)obj - spec->data; /* obj = data + 12 */ produces an unsigned underflow for spec->size in [4, 11], yielding values around 0xFFFFFFFC. The corrupted spec is then passed to module_adapter_init_data() where the inflated size bypasses the base_cfg guard and dst->base_cfg is populated from mailbox bytes beyond the declared payload boundary. Found by semgrep static analysis, confirmed by manual review of the caller chain through module_adapter_init_data(), and verified with prepared tests. Fixes: 1. Move size guard before ext_init dereference so spec->size is validated against sizeof(*ext_init) before any field is read. 2. Correct sizeof operand from sizeof(ext_init) to sizeof(*ext_init) (4 bytes → 12 bytes). 3. Guard the unconditional spec adjustment — compute consumed bytes and return -EINVAL if consumed > spec->size before subtracting. 4. Add upper-bound check in module_adapter_init_data() — reject cfgsz greater than MAILBOX_HOSTBOX_SIZE as a defense-in-depth measure. Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
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.
Three weaknesses compose into a single chain in module_ext_init_decode() that allows a crafted IPC4 ModuleInit payload to corrupt spec->size and spec->data before they are consumed by module_adapter_init_data().
The size guard used the wrong sizeof operand:
if (spec->size < sizeof(ext_init)) /* sizeof(pointer) = 4, not 12 */
This accepted any payload >= 4 bytes even though the struct header is 12 bytes. Additionally, ext_init->data_obj_array was dereferenced before the guard ran, allowing the object-walk loop to be skipped with no size validation. When the loop is skipped, the unconditional spec->size adjustment:
spec->size -= (unsigned char )obj - spec->data; / obj = data + 12 */
produces an unsigned underflow for spec->size in [4, 11], yielding values around 0xFFFFFFFC. The corrupted spec is then passed to module_adapter_init_data() where the inflated size bypasses the base_cfg guard and dst->base_cfg is populated from mailbox bytes beyond the declared payload boundary.
Found by semgrep static analysis, confirmed by manual review of the caller chain through module_adapter_init_data(), and verified with prepared tests.
Fixes:
Move size guard before ext_init dereference so spec->size is validated against sizeof(*ext_init) before any field is read.
Correct sizeof operand from sizeof(ext_init) to sizeof(*ext_init) (4 bytes → 12 bytes).
Guard the unconditional spec adjustment — compute consumed bytes and return -EINVAL if consumed > spec->size before subtracting.
Add upper-bound check in module_adapter_init_data() — reject cfgsz greater than MAILBOX_HOSTBOX_SIZE as a defense-in-depth measure.