Skip to content

Commit dcde013

Browse files
committed
audio: module_adapter: fix sizeof(pointer) and underflow in module_ext_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>
1 parent 8d75e2c commit dcde013

1 file changed

Lines changed: 23 additions & 7 deletions

File tree

src/audio/module_adapter/module_adapter_ipc4.c

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
#include <sof/audio/module_adapter/module/generic.h>
1717
#include <sof/audio/pipeline.h>
1818
#include <sof/common.h>
19+
#include <sof/lib/mailbox.h>
1920
#include <sof/platform.h>
2021
#include <sof/ut.h>
2122
#include <rtos/interrupt.h>
@@ -28,17 +29,22 @@ LOG_MODULE_DECLARE(module_adapter, CONFIG_SOF_LOG_LEVEL);
2829
int module_ext_init_decode(const struct comp_driver *drv, struct module_ext_init_data *ext_data,
2930
struct ipc_config_process *spec)
3031
{
31-
const struct ipc4_module_init_ext_init *ext_init =
32-
(const struct ipc4_module_init_ext_init *)spec->data;
33-
bool last_object = !ext_init->data_obj_array;
32+
const struct ipc4_module_init_ext_init *ext_init;
3433
const struct ipc4_module_init_ext_object *obj;
34+
bool last_object;
35+
size_t consumed;
3536

3637
assert(drv->type == SOF_COMP_MODULE_ADAPTER);
37-
if (spec->size < sizeof(ext_init)) {
38+
39+
/* Validate size before dereferencing ext_init pointer */
40+
if (spec->size < sizeof(*ext_init)) {
3841
comp_cl_err(drv, "Size too small for ext init %zu < %zu",
39-
spec->size, sizeof(ext_init));
42+
spec->size, sizeof(*ext_init));
4043
return -EINVAL;
4144
}
45+
46+
ext_init = (const struct ipc4_module_init_ext_init *)spec->data;
47+
last_object = !ext_init->data_obj_array;
4248
/* TODO: Handle ext_init->gna_used and ext_init->rtos_domain here */
4349
/* Get the first obj struct right after ext_init struct */
4450
obj = (const struct ipc4_module_init_ext_object *)(ext_init + 1);
@@ -103,7 +109,15 @@ int module_ext_init_decode(const struct comp_driver *drv, struct module_ext_init
103109
}
104110

105111
/* Remove decoded ext_init payload from spec */
106-
spec->size -= (unsigned char *)obj - spec->data;
112+
consumed = (unsigned char *)obj - spec->data;
113+
114+
if (consumed > spec->size) {
115+
comp_cl_err(drv, "ext_init consumed more than spec->size (%zu > %zu)",
116+
consumed, spec->size);
117+
return -EINVAL;
118+
}
119+
120+
spec->size -= consumed;
107121
spec->data = (const unsigned char *)obj;
108122

109123
return 0;
@@ -132,8 +146,10 @@ int module_adapter_init_data(struct comp_dev *dev,
132146

133147
if (cfg == NULL)
134148
return -EINVAL;
135-
if (cfgsz < sizeof(cfg->base_cfg))
149+
if (cfgsz > MAILBOX_HOSTBOX_SIZE || cfgsz < sizeof(cfg->base_cfg)) {
150+
comp_err(dev, "invalid config size %zu", cfgsz);
136151
return -EINVAL;
152+
}
137153

138154
dst->base_cfg = cfg->base_cfg;
139155
dst->size = cfgsz;

0 commit comments

Comments
 (0)