From 8f74b4df9953778aa09b4d4f264048fca5ba350b Mon Sep 17 00:00:00 2001 From: Tomasz Leman Date: Fri, 25 Sep 2026 18:06:17 +0200 Subject: [PATCH 1/2] ipc3: handler: pass control payload capacity to comp_cmd() ipc_comp_value() handed SOF_IPC_MSG_MAX_SIZE, the size of the whole comp_data buffer, to comp_cmd() as max_data_size. Every IPC3 binary control "get" handler uses that value as the memcpy_s() destination size for cdata->data->data, which sits behind struct sof_ipc_ctrl_data and struct sof_abi_hdr (~120 bytes into comp_data). The claimed destination therefore extended past the end of the allocation. Two consequences: - memcpy_s() checks the source against [dest, dest + dest_size). On native_sim comp_data and component private data share one heap, so a copy source located right after comp_data landed inside the phantom window and memcpy_s() returned -EINVAL for a copy that never overlapped. The handlers assert(!ret), turning it into a panic during IPC3 fuzzing. - A getter copying close to max_data_size bytes (comp_data_blob_get_cmd() with a large blob) could write up to the header size past comp_data. Fix it at the single IPC3 entry point: pass the capacity remaining after the two headers. All comp_cmd() consumers already treat max_data_size as payload capacity, so no component changes are needed. module_adapter's IPC3 get path passed cdata->num_elems as fragment_size instead of the buffer size, which made the getters' num_elems bound check a comparison of num_elems with itself and let the host request more than comp_data can hold. Forward max_data_size instead. Signed-off-by: Tomasz Leman --- src/audio/module_adapter/module_adapter_ipc3.c | 17 +++++++++++------ src/ipc/ipc3/handler.c | 10 ++++++++-- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/src/audio/module_adapter/module_adapter_ipc3.c b/src/audio/module_adapter/module_adapter_ipc3.c index e6098dec0799..ca41368ce970 100644 --- a/src/audio/module_adapter/module_adapter_ipc3.c +++ b/src/audio/module_adapter/module_adapter_ipc3.c @@ -178,7 +178,7 @@ int module_adapter_set_state(struct processing_module *mod, struct comp_dev *dev } static int module_adapter_get_set_params(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata, - bool set) + bool set, int max_data_size) { struct processing_module *mod = comp_mod(dev); const struct module_interface *const interface = mod->dev->drv->adapter_ops; @@ -225,9 +225,14 @@ static int module_adapter_get_set_params(struct comp_dev *dev, struct sof_ipc_ct return 0; } + /* + * For a get, the fragment is the reply buffer starting at cdata, so pass + * its full size rather than the host-controlled num_elems: getters check + * num_elems against fragment_size, which is meaningless if they are equal. + */ if (interface->get_configuration) return interface->get_configuration(mod, pos, &data_offset_size, - (uint8_t *)cdata, cdata->num_elems); + (uint8_t *)cdata, max_data_size); comp_err(dev, "no configuration op get for %d", dev_comp_id(dev)); @@ -235,7 +240,7 @@ static int module_adapter_get_set_params(struct comp_dev *dev, struct sof_ipc_ct } static int module_adapter_ctrl_get_set_data(struct comp_dev *dev, struct sof_ipc_ctrl_data *cdata, - bool set) + bool set, int max_data_size) { int ret; struct processing_module __maybe_unused *mod = comp_mod(dev); @@ -255,7 +260,7 @@ static int module_adapter_ctrl_get_set_data(struct comp_dev *dev, struct sof_ipc ret = -EIO; break; case SOF_CTRL_CMD_BINARY: - ret = module_adapter_get_set_params(dev, cdata, set); + ret = module_adapter_get_set_params(dev, cdata, set, max_data_size); break; default: comp_err(dev, "module_adapter_ctrl_set_data error: unknown set data command"); @@ -278,10 +283,10 @@ int module_adapter_cmd(struct comp_dev *dev, int cmd, void *data, int max_data_s switch (cmd) { case COMP_CMD_SET_DATA: - ret = module_adapter_ctrl_get_set_data(dev, cdata, true); + ret = module_adapter_ctrl_get_set_data(dev, cdata, true, max_data_size); break; case COMP_CMD_GET_DATA: - ret = module_adapter_ctrl_get_set_data(dev, cdata, false); + ret = module_adapter_ctrl_get_set_data(dev, cdata, false, max_data_size); break; case COMP_CMD_SET_VALUE: /* diff --git a/src/ipc/ipc3/handler.c b/src/ipc/ipc3/handler.c index 3c7c38f30f15..25831cf1c772 100644 --- a/src/ipc/ipc3/handler.c +++ b/src/ipc/ipc3/handler.c @@ -1208,8 +1208,14 @@ static int ipc_comp_value(uint32_t header, uint32_t cmd) tr_dbg(&ipc_tr, "ipc: comp %d -> cmd %d", data->comp_id, data->cmd); - /* get component values */ - ret = comp_cmd(comp_dev->cd, cmd, data, SOF_IPC_MSG_MAX_SIZE); + /* + * Components use max_data_size as the memcpy_s() destination size for + * data->data->data, so pass the payload capacity left after the + * control and ABI headers rather than the whole comp_data buffer. + */ + ret = comp_cmd(comp_dev->cd, cmd, data, + SOF_IPC_MSG_MAX_SIZE - offsetof(struct sof_ipc_ctrl_data, data) - + sizeof(struct sof_abi_hdr)); if (ret < 0) { ipc_cmd_err(&ipc_tr, "ipc: comp %d cmd %u failed %d", data->comp_id, data->cmd, ret); From 94dff237360236cd928ae2eb6f8d9fbe838e3e11 Mon Sep 17 00:00:00 2001 From: Tomasz Leman Date: Fri, 25 Sep 2026 18:07:39 +0200 Subject: [PATCH 2/2] samples: detect_test: bound IPC3 config get by struct size test_keyword_get_config() copies cd->config.size bytes out of the fixed-size cd->config. The size field is taken verbatim from the blob: the IPC3 set path checks it against sizeof(struct sof_detect_test_config) but the topology init path in test_keyword_new() only requires the blob to be at least that large, so the embedded size field can be arbitrary. A subsequent SOF_IPC_COMP_GET_DATA then reads past cd->config, up to the reply buffer capacity, and returns adjacent heap contents to the host. Bound bs by sizeof(cd->config) as smart_amp_get_config() already does. Signed-off-by: Tomasz Leman --- src/samples/audio/detect_test.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/samples/audio/detect_test.c b/src/samples/audio/detect_test.c index 939623b0d10d..0caa014bf060 100644 --- a/src/samples/audio/detect_test.c +++ b/src/samples/audio/detect_test.c @@ -580,7 +580,10 @@ static int test_keyword_get_config(struct comp_dev *dev, bs = cd->config.size; comp_info(dev, "value of block size: %zu", bs); - if (bs == 0 || bs > size) + /* bs comes from the host/topology blob and is the memcpy source length + * from the fixed-size cd->config, so bound it by the struct size too. + */ + if (bs == 0 || bs > sizeof(cd->config) || bs > size) return -EINVAL; ret = memcpy_s(cdata->data->data, size, &cd->config, bs);