Skip to content

audio: module_adapter: fix sizeof(pointer) and underflow in module_ext_init_decode#10748

Open
tmleman wants to merge 1 commit intothesofproject:mainfrom
tmleman:topic/upstream/pr/ipc4/module_adapter/fix/sizeof_pointer
Open

audio: module_adapter: fix sizeof(pointer) and underflow in module_ext_init_decode#10748
tmleman wants to merge 1 commit intothesofproject:mainfrom
tmleman:topic/upstream/pr/ipc4/module_adapter/fix/sizeof_pointer

Conversation

@tmleman
Copy link
Copy Markdown
Contributor

@tmleman tmleman commented May 7, 2026

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.

@tmleman tmleman requested a review from ranj063 as a code owner May 7, 2026 08:51
Copilot AI review requested due to automatic review settings May 7, 2026 08:51
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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->size against the full ext_init header (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 than MAILBOX_HOSTBOX_SIZE.

Comment thread src/audio/module_adapter/module_adapter_ipc4.c
Comment thread src/audio/module_adapter/module_adapter_ipc4.c
Copy link
Copy Markdown
Contributor

@jsarha jsarha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good caught. The copilot comment is valid, but not that critical (and the error is probably copypasted from my earlier print, feel free fix that too if you get to it).

spec->size -= (unsigned char *)obj - spec->data;
consumed = (unsigned char *)obj - spec->data;

if (consumed > spec->size) {
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is already checked in the loop when processing that last object in lines 55-67 (with the patch applied), isn't it?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right, with the sizeof fix in place this check is technically redundant. The no-loop path can't underflow because we already validated spec->size >= sizeof(*ext_init), and when the loop runs the bounds checks on lines 55-67 cover it.

I kept it as a defense-in-depth catch-all right before the subtraction. It's one comparison and it makes the function self-contained. If someone refactors the loop bounds logic later, this still prevents the underflow. Happy to drop it if you feel it's unnecessary noise.

@tmleman tmleman force-pushed the topic/upstream/pr/ipc4/module_adapter/fix/sizeof_pointer branch from dcde013 to 8da6464 Compare May 8, 2026 10:34
…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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants