From 8da64643b22f1a6328c27a63732604fd2fbe8d79 Mon Sep 17 00:00:00 2001 From: Tomasz Leman Date: Thu, 7 May 2026 11:58:28 +0200 Subject: [PATCH] audio: module_adapter: fix sizeof(pointer) and underflow in module_ext_init_decode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../module_adapter/module_adapter_ipc4.c | 36 +++++++++++++------ 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/src/audio/module_adapter/module_adapter_ipc4.c b/src/audio/module_adapter/module_adapter_ipc4.c index 092f93314bac..1b66bc766b06 100644 --- a/src/audio/module_adapter/module_adapter_ipc4.c +++ b/src/audio/module_adapter/module_adapter_ipc4.c @@ -16,6 +16,7 @@ #include #include #include +#include #include #include #include @@ -28,17 +29,22 @@ LOG_MODULE_DECLARE(module_adapter, CONFIG_SOF_LOG_LEVEL); int module_ext_init_decode(const struct comp_driver *drv, struct module_ext_init_data *ext_data, struct ipc_config_process *spec) { - const struct ipc4_module_init_ext_init *ext_init = - (const struct ipc4_module_init_ext_init *)spec->data; - bool last_object = !ext_init->data_obj_array; + const struct ipc4_module_init_ext_init *ext_init; const struct ipc4_module_init_ext_object *obj; + bool last_object; + size_t consumed; assert(drv->type == SOF_COMP_MODULE_ADAPTER); - if (spec->size < sizeof(ext_init)) { - comp_cl_err(drv, "Size too small for ext init %zu < %zu", - spec->size, sizeof(ext_init)); + + /* Validate size before dereferencing ext_init pointer */ + if (spec->size < sizeof(*ext_init)) { + comp_cl_err(drv, "Size too small for ext init %u < %zu", + spec->size, sizeof(*ext_init)); return -EINVAL; } + + ext_init = (const struct ipc4_module_init_ext_init *)spec->data; + last_object = !ext_init->data_obj_array; /* TODO: Handle ext_init->gna_used and ext_init->rtos_domain here */ /* Get the first obj struct right after ext_init struct */ obj = (const struct ipc4_module_init_ext_object *)(ext_init + 1); @@ -47,7 +53,7 @@ int module_ext_init_decode(const struct comp_driver *drv, struct module_ext_init /* Check if there is space for the object header */ if ((unsigned char *)(obj + 1) - spec->data > spec->size) { - comp_cl_err(drv, "ext init obj overflow, %u > %zu", + comp_cl_err(drv, "ext init obj overflow, %u > %u", (unsigned char *)(obj + 1) - spec->data, spec->size); return -EINVAL; } @@ -55,7 +61,7 @@ int module_ext_init_decode(const struct comp_driver *drv, struct module_ext_init next_obj = (const struct ipc4_module_init_ext_object *) (((uint32_t *) (obj + 1)) + obj->object_words); if ((unsigned char *)next_obj - spec->data > spec->size) { - comp_cl_err(drv, "ext init object array overflow, %u > %zu", + comp_cl_err(drv, "ext init object array overflow, %u > %u", (unsigned char *)obj - spec->data, spec->size); return -EINVAL; } @@ -103,7 +109,15 @@ int module_ext_init_decode(const struct comp_driver *drv, struct module_ext_init } /* Remove decoded ext_init payload from spec */ - spec->size -= (unsigned char *)obj - spec->data; + consumed = (unsigned char *)obj - spec->data; + + if (consumed > spec->size) { + comp_cl_err(drv, "ext_init consumed more than spec->size (%zu > %u)", + consumed, spec->size); + return -EINVAL; + } + + spec->size -= consumed; spec->data = (const unsigned char *)obj; return 0; @@ -132,8 +146,10 @@ int module_adapter_init_data(struct comp_dev *dev, if (cfg == NULL) return -EINVAL; - if (cfgsz < sizeof(cfg->base_cfg)) + if (cfgsz > MAILBOX_HOSTBOX_SIZE || cfgsz < sizeof(cfg->base_cfg)) { + comp_err(dev, "invalid config size %zu", cfgsz); return -EINVAL; + } dst->base_cfg = cfg->base_cfg; dst->size = cfgsz;