Skip to content

Commit 82ff1e4

Browse files
tmlemankv2019i
authored andcommitted
audio: module_adapter: bound large_config fragment reassembly to buffer size
md->new_cfg_size, used to bound every SET_LARGE_CONFIG reassembly copy in module_set_configuration, is written in two places: - module_set_large_config, sets it unconditionally on a FIRST fragment - module_set_configuration, sets it while allocating md->runtime_params to that size Nothing keeps the two in sync when a second FIRST fragment arrives mid-reassembly. module_set_large_config allows to overwrite new_cfg_size with a new (larger) value while runtime_params still holds the previous, smaller allocation. A following MIDDLE/LAST fragment then passes the memcpy_s bound check and writes host-controlled mailbox data out of bounds of the heap buffer. Fix it in the reassembly path: - module_set_large_config: reject a FIRST fragment with -EBUSY when a reassembly is already in progress, before overwriting new_cfg_size. - module_set_configuration: commit new_cfg_size only after runtime_params is allocated, so the two always match, and reject an intermediate/last fragment whose host-supplied offset or size does not fit in the buffer, which otherwise underflows the memcpy_s destination bound. Closes #11153 Signed-off-by: Tomasz Leman <tomasz.m.leman@intel.com>
1 parent f7dc508 commit 82ff1e4

2 files changed

Lines changed: 18 additions & 7 deletions

File tree

src/audio/module_adapter/module/generic.c

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -839,32 +839,33 @@ int module_set_configuration(struct processing_module *mod,
839839
* verify input params & allocate memory for the config blob when the first
840840
* fragment arrives
841841
*/
842-
md->new_cfg_size = data_offset_size;
843842

844843
/* Check that there is no previous request in progress */
845844
if (md->runtime_params) {
846-
comp_err(dev, "error: busy with previous request");
845+
comp_err(dev, "busy with previous request");
847846
return -EBUSY;
848847
}
849848

850-
if (!md->new_cfg_size)
849+
if (!data_offset_size)
851850
return 0;
852851

853-
if (md->new_cfg_size > CONFIG_MODULE_MAX_BLOB_SIZE) {
854-
comp_err(dev, "error: blob size is too big cfg size %zu, allowed %d",
855-
md->new_cfg_size, CONFIG_MODULE_MAX_BLOB_SIZE);
852+
if (data_offset_size > CONFIG_MODULE_MAX_BLOB_SIZE) {
853+
comp_err(dev, "blob size is too big cfg size %zu, allowed %d",
854+
data_offset_size, CONFIG_MODULE_MAX_BLOB_SIZE);
856855
return -EINVAL;
857856
}
858857

859858
/* Allocate buffer for new params */
860859
md->runtime_params = sof_heap_alloc(sof_sys_user_heap_get(),
861860
SOF_MEM_FLAG_USER | SOF_MEM_FLAG_LARGE_BUFFER,
862-
md->new_cfg_size, 0);
861+
data_offset_size, 0);
863862
if (!md->runtime_params) {
864863
comp_err(dev, "space allocation for new params failed");
865864
return -ENOMEM;
866865
}
867866

867+
md->new_cfg_size = data_offset_size;
868+
868869
memset(md->runtime_params, 0, md->new_cfg_size);
869870
break;
870871
default:
@@ -875,6 +876,12 @@ int module_set_configuration(struct processing_module *mod,
875876

876877
/* set offset for intermediate and last fragments */
877878
offset = data_offset_size;
879+
if (offset > md->new_cfg_size ||
880+
fragment_size > md->new_cfg_size - offset) {
881+
comp_err(dev, "fragment (offset %zu, size %zu) exceeds config buffer %zu",
882+
offset, fragment_size, md->new_cfg_size);
883+
return -EINVAL;
884+
}
878885
break;
879886
}
880887

src/audio/module_adapter/module_adapter_ipc4.c

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -243,6 +243,10 @@ int module_set_large_config(struct comp_dev *dev, uint32_t param_id, bool first_
243243
fragment_size = MAILBOX_DSPBOX_SIZE;
244244
break;
245245
case MODULE_CFG_FRAGMENT_FIRST:
246+
if (md->runtime_params) {
247+
comp_err(dev, "FIRST fragment while a request is in progress");
248+
return -EBUSY;
249+
}
246250
md->new_cfg_size = data_offset_size;
247251
fragment_size = MAILBOX_DSPBOX_SIZE;
248252
break;

0 commit comments

Comments
 (0)