Skip to content

ipc4: handler: reject get_large_config vendor payload larger than hostbox#10851

Open
tmleman wants to merge 1 commit into
thesofproject:mainfrom
tmleman:topic/upstream/pr/fix/ipc4/get_large_config/larger_payload
Open

ipc4: handler: reject get_large_config vendor payload larger than hostbox#10851
tmleman wants to merge 1 commit into
thesofproject:mainfrom
tmleman:topic/upstream/pr/fix/ipc4/get_large_config/larger_payload

Conversation

@tmleman
Copy link
Copy Markdown
Contributor

@tmleman tmleman commented Jun 8, 2026

ipc4_get_large_config_module_instance() reads config.extension.r.data_off_size straight from the host message and, for the VENDOR_CONFIG_PARAM case, uses it without an upper bound:

  • dcache_invalidate_region() is asked to invalidate that many bytes starting at MAILBOX_HOSTBOX_BASE, and
  • ipc4_get_vendor_config_module_instance() computes tl_count = data_off_size / sizeof(struct sof_tl) and walks that many TL records straight out of the hostbox.

data_off_size is a 20-bit field, so it can claim up to ~1 MB while the hostbox is only MAILBOX_HOSTBOX_SIZE (SOF_IPC_MSG_MAX_SIZE) bytes. The symmetric set path already rejects this in ipc4_set_vendor_config_module_instance() ("data_off_size ... > MAILBOX_HOSTBOX_SIZE"), the get path was missing the same guard, leaving a cache-maintenance over-range on real hardware and an out-of-bounds hostbox read for any vendor param that returns success with a zero-length value (which keeps produced_data from advancing the DSPBOX cap that otherwise terminates the loop early).

Found by audit while reviewing the IPC4 hostbox readers surfaced by the fuzz harness, not by a live crash: on native_sim CONFIG_CACHE is unset so the invalidate is a no-op, and the common TL walk is bounded in practice by the DSPBOX size check. Convert the missing bound into a hard rejection before the region is touched, mirroring the set path.

Copilot AI review requested due to automatic review settings June 8, 2026 13:18
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

This PR hardens the IPC4 “get large config” vendor-param path by rejecting host-provided data_off_size values that exceed the physical hostbox size, preventing cache maintenance over-range and out-of-bounds hostbox reads.

Changes:

  • Add a MAILBOX_HOSTBOX_SIZE upper-bound check for data_off_size in ipc4_get_large_config_module_instance() when handling VENDOR_CONFIG_PARAM.
  • Return IPC4_INVALID_CONFIG_DATA_STRUCT and log an error before any cache invalidation or hostbox parsing occurs.

…tbox

ipc4_get_large_config_module_instance() reads config.extension.r.data_off_size
straight from the host message and, for the VENDOR_CONFIG_PARAM case, uses it
without an upper bound:

  * dcache_invalidate_region() is asked to invalidate that many bytes starting
    at MAILBOX_HOSTBOX_BASE, and
  * ipc4_get_vendor_config_module_instance() computes
    tl_count = data_off_size / sizeof(struct sof_tl) and walks that many TL
    records straight out of the hostbox.

data_off_size is a 20-bit field, so it can claim up to ~1 MB while the hostbox
is only MAILBOX_HOSTBOX_SIZE (SOF_IPC_MSG_MAX_SIZE) bytes. The symmetric set
path already rejects this in ipc4_set_vendor_config_module_instance()
("data_off_size ... > MAILBOX_HOSTBOX_SIZE"), the get path was missing the
same guard, leaving a cache-maintenance over-range on real hardware and an
out-of-bounds hostbox read for any vendor param that returns success with a
zero-length value (which keeps produced_data from advancing the DSPBOX cap
that otherwise terminates the loop early).

Found by audit while reviewing the IPC4 hostbox readers surfaced by the fuzz
harness, not by a live crash: on native_sim CONFIG_CACHE is unset so the
invalidate is a no-op, and the common TL walk is bounded in practice by the
DSPBOX size check. Convert the missing bound into a hard rejection before the
region is touched, mirroring the set path.

Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
/* data_off_size is a 20-bit host-controlled field, so it can
* claim far more than the hostbox can physically hold.
*/
if (data_offset > MAILBOX_HOSTBOX_SIZE) {
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.

Do we need to include the container size or current offset in the calculation ? e.g. if object size + data_offset > MAILBOX_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 data is set to start of the mailbox, so this would be correct.

/* data_off_size is a 20-bit host-controlled field, so it can
* claim far more than the hostbox can physically hold.
*/
if (data_offset > MAILBOX_HOSTBOX_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 data is set to start of the mailbox, so this would be correct.

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