From 78d838228375640231c78fa09ac0c9aa3ea732a8 Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Wed, 26 Apr 2023 19:43:22 -0700 Subject: [PATCH 1/2] ipc4: Handle typing errors in component retrieval In IPC4, the component ID is entirely under the control of the input protocol, and as components of multiple types exist in the list, it's possible for protocol messages to fetch an incorrectly typed object, leading to crashes at runtime. Force all usage to go through a get_comp() utility that does type checking. There was already a similar utility for the special case of pipelines (which have a separate ID to query). Signed-off-by: Andy Ross --- src/ipc/ipc4/helper.c | 23 ++++++++++++++--------- 1 file changed, 14 insertions(+), 9 deletions(-) diff --git a/src/ipc/ipc4/helper.c b/src/ipc/ipc4/helper.c index a58e0f252c9f..259d5b0407a4 100644 --- a/src/ipc/ipc4/helper.c +++ b/src/ipc/ipc4/helper.c @@ -135,6 +135,16 @@ struct comp_dev *comp_new_ipc4(struct ipc4_module_init_instance *module_init) return dev; } +static struct ipc_comp_dev *get_comp(struct ipc *ipc, uint16_t type, uint32_t id) +{ + struct ipc_comp_dev *c = ipc_get_comp_by_id(ipc, id); + + if (c && c->type == type) + return c; + + return NULL; +} + struct ipc_comp_dev *ipc_get_comp_by_ppl_id(struct ipc *ipc, uint16_t type, uint32_t ppl_id) { struct ipc_comp_dev *icd; @@ -283,7 +293,7 @@ int ipc_pipeline_free(struct ipc *ipc, uint32_t comp_id) int ret; /* check whether pipeline exists */ - ipc_pipe = ipc_get_comp_by_id(ipc, comp_id); + ipc_pipe = get_comp(ipc, COMP_TYPE_PIPELINE, comp_id); if (!ipc_pipe) return IPC4_INVALID_RESOURCE_ID; @@ -608,7 +618,7 @@ int ipc4_pipeline_complete(struct ipc *ipc, uint32_t comp_id) struct ipc_comp_dev *ipc_pipe; int ret; - ipc_pipe = ipc_get_comp_by_id(ipc, comp_id); + ipc_pipe = get_comp(ipc, COMP_TYPE_PIPELINE, comp_id); /* Pass IPC to target core */ if (!cpu_is_me(ipc_pipe->core)) @@ -728,14 +738,9 @@ const struct comp_driver *ipc4_get_comp_drv(int module_id) struct comp_dev *ipc4_get_comp_dev(uint32_t comp_id) { - struct ipc *ipc = ipc_get(); - struct ipc_comp_dev *icd; - - icd = ipc_get_comp_by_id(ipc, comp_id); - if (!icd) - return NULL; + struct ipc_comp_dev *icd = get_comp(ipc_get(), COMP_TYPE_COMPONENT, comp_id); - return icd->cd; + return icd ? icd->cd : NULL; } int ipc4_add_comp_dev(struct comp_dev *dev) From a94fa88b2f09a0eb9884338413cb59bc206da268 Mon Sep 17 00:00:00 2001 From: Andy Ross Date: Tue, 2 May 2023 10:09:48 -0700 Subject: [PATCH 2/2] IPC4: Handle duplicate component IDs In the SOF scheme, component IDs are allocated by the host driver. So they need to be validated before use when modifying the global component list to prevent duplicate entries from being inserted. IPC3 was doing this already. IPC4 had copied the form of the code in one spot, but had missed that the ID is global across types and was only checking for duplicate pipelines. Signed-off-by: Andy Ross --- src/ipc/ipc4/helper.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/ipc/ipc4/helper.c b/src/ipc/ipc4/helper.c index 259d5b0407a4..fcff2e54278e 100644 --- a/src/ipc/ipc4/helper.c +++ b/src/ipc/ipc4/helper.c @@ -178,10 +178,9 @@ static int ipc4_create_pipeline(struct ipc4_pipeline_create *pipe_desc) struct ipc *ipc = ipc_get(); /* check whether pipeline id is already taken or in use */ - ipc_pipe = ipc_get_comp_by_ppl_id(ipc, COMP_TYPE_PIPELINE, - pipe_desc->primary.r.instance_id); + ipc_pipe = ipc_get_comp_by_id(ipc, pipe_desc->primary.r.instance_id); if (ipc_pipe) { - tr_err(&ipc_tr, "ipc: pipeline id is already taken, pipe_desc->instance_id = %u", + tr_err(&ipc_tr, "ipc: comp id is already taken, pipe_desc->instance_id = %u", (uint32_t)pipe_desc->primary.r.instance_id); return IPC4_INVALID_RESOURCE_ID; } @@ -748,6 +747,13 @@ int ipc4_add_comp_dev(struct comp_dev *dev) struct ipc *ipc = ipc_get(); struct ipc_comp_dev *icd; + /* check id for duplicates */ + icd = ipc_get_comp_by_id(ipc, dev->ipc_config.id); + if (icd) { + tr_err(&ipc_tr, "ipc: duplicate component ID"); + return IPC4_INVALID_RESOURCE_ID; + } + /* allocate the IPC component container */ icd = rzalloc(SOF_MEM_ZONE_RUNTIME_SHARED, 0, SOF_MEM_CAPS_RAM, sizeof(struct ipc_comp_dev));