Skip to content

Commit e85e8e4

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 e85e8e4

1 file changed

Lines changed: 29 additions & 11 deletions

File tree

src/audio/module_adapter/module_adapter_ipc4.c

Lines changed: 29 additions & 11 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-
comp_cl_err(drv, "Size too small for ext init %zu < %zu",
39-
spec->size, sizeof(ext_init));
38+
39+
/* Validate size before dereferencing ext_init pointer */
40+
if (spec->size < sizeof(*ext_init)) {
41+
comp_cl_err(drv, "Size too small for ext init %u < %zu",
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);
@@ -47,15 +53,15 @@ int module_ext_init_decode(const struct comp_driver *drv, struct module_ext_init
4753

4854
/* Check if there is space for the object header */
4955
if ((unsigned char *)(obj + 1) - spec->data > spec->size) {
50-
comp_cl_err(drv, "ext init obj overflow, %u > %zu",
56+
comp_cl_err(drv, "ext init obj overflow, %u > %u",
5157
(unsigned char *)(obj + 1) - spec->data, spec->size);
5258
return -EINVAL;
5359
}
5460
/* Calculate would be next object position and check if current object fits */
5561
next_obj = (const struct ipc4_module_init_ext_object *)
5662
(((uint32_t *) (obj + 1)) + obj->object_words);
5763
if ((unsigned char *)next_obj - spec->data > spec->size) {
58-
comp_cl_err(drv, "ext init object array overflow, %u > %zu",
64+
comp_cl_err(drv, "ext init object array overflow, %u > %u",
5965
(unsigned char *)obj - spec->data, spec->size);
6066
return -EINVAL;
6167
}
@@ -102,8 +108,18 @@ int module_ext_init_decode(const struct comp_driver *drv, struct module_ext_init
102108
obj = next_obj;
103109
}
104110

105-
/* Remove decoded ext_init payload from spec */
106-
spec->size -= (unsigned char *)obj - spec->data;
111+
/*
112+
* Remove decoded ext_init payload from spec.
113+
* consumed <= spec->size is guaranteed here:
114+
* - no-loop path: consumed == sizeof(*ext_init) <= spec->size (validated above)
115+
* - loop path: in-loop bounds checks (obj/next_obj vs spec->size) ensure
116+
* obj never advances past spec->data + spec->size
117+
*/
118+
consumed = (unsigned char *)obj - spec->data;
119+
__ASSERT(consumed <= spec->size, "ext_init consumed %zu > spec->size %u",
120+
consumed, spec->size);
121+
122+
spec->size -= consumed;
107123
spec->data = (const unsigned char *)obj;
108124

109125
return 0;
@@ -132,8 +148,10 @@ int module_adapter_init_data(struct comp_dev *dev,
132148

133149
if (cfg == NULL)
134150
return -EINVAL;
135-
if (cfgsz < sizeof(cfg->base_cfg))
151+
if (cfgsz > MAILBOX_HOSTBOX_SIZE || cfgsz < sizeof(cfg->base_cfg)) {
152+
comp_err(dev, "invalid config size %zu", cfgsz);
136153
return -EINVAL;
154+
}
137155

138156
dst->base_cfg = cfg->base_cfg;
139157
dst->size = cfgsz;

0 commit comments

Comments
 (0)