Skip to content

audio: module_adapter_ipc4: add range check to module_get_large_config()#10875

Open
kv2019i wants to merge 1 commit into
thesofproject:mainfrom
kv2019i:202606-module-adapt-largeconfig-fix
Open

audio: module_adapter_ipc4: add range check to module_get_large_config()#10875
kv2019i wants to merge 1 commit into
thesofproject:mainfrom
kv2019i:202606-module-adapt-largeconfig-fix

Conversation

@kv2019i

@kv2019i kv2019i commented Jun 11, 2026

Copy link
Copy Markdown
Collaborator

In a multi-block get case, if the host sends data_off_size > md->cfg.size, the calculation of the last fragment size is incorrect if a sufficiently large value is passed.

Add validation to catch this case and return an error data_off_size is too large.

Copilot AI review requested due to automatic review settings June 11, 2026 12:28
@kv2019i kv2019i requested a review from ranj063 as a code owner June 11, 2026 12:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR hardens IPC4 large configuration reads by validating a host-supplied offset in module_get_large_config() to prevent an unsigned underflow when calculating the final fragment size.

Changes:

  • Add a range check rejecting *data_offset_size values that exceed md->cfg.size for the last fragment in multi-block GET.
  • Improve control flow clarity in the multi-block/last-block branch by adding braces and a defensive error return.

@lgirdwood

Copy link
Copy Markdown
Member

@kv2019i can you checking in with QB CI, Ive restarted the GH actions.

* field) and both operands are unsigned, so reject an offset
* past the config size to avoid the subtraction underflowing
* into a huge fragment_size passed to get_configuration().
*/

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Is such a long comment really needed for such a simple and self-explanatory check?

Save tokens wherever you are! 😎

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I personally removed those copilot added comments. If we explain every line of code with such detailed comments, the code becomes unreadable.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

I kept the verbose comment here as these get_set configs deserve more attention. But ack, maybe this is too much. Will remove in next revision.

* field) and both operands are unsigned, so reject an offset
* past the config size to avoid the subtraction underflowing
* into a huge fragment_size passed to get_configuration().
*/

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I personally removed those copilot added comments. If we explain every line of code with such detailed comments, the code becomes unreadable.

In a multi-block get case, if the host sends data_off_size > md->cfg.size,
the calculation of the last fragment size is incorrect if a sufficiently
large value is passed.

Add validation to catch this case and return an error data_off_size is too
large.

Signed-off-by: Kai Vehmanen <kai.vehmanen@linux.intel.com>
@kv2019i kv2019i force-pushed the 202606-module-adapt-largeconfig-fix branch from 9f39959 to d31bf48 Compare June 12, 2026 15:48
@kv2019i

kv2019i commented Jun 12, 2026

Copy link
Copy Markdown
Collaborator Author

V2:

  • remove the verbose comments

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.

6 participants