From ed397cf4656586a5a8e600bee7ac72207bf469eb Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Mon, 19 Dec 2022 17:15:36 +0200 Subject: [PATCH 1/9] sof-logger: exit with error if malloc fails In user-space tools, memory allocations can reasonably be expected to always succeed. Make this assumption explicit by adding error handling after malloc. Signed-off-by: Kai Vehmanen (cherry picked from commit 4d64893b868297e259f578267a18e18275e538c1) --- tools/logger/logger.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/logger/logger.c b/tools/logger/logger.c index b5df7e185d32..11e3753638bf 100644 --- a/tools/logger/logger.c +++ b/tools/logger/logger.c @@ -223,6 +223,11 @@ static void *wait_open(const char *watched_dir, const char *expected_file) char * const fpath = malloc(strlen(watched_dir) + 1 + strlen(expected_file) + 1); + if (!fpath) { + fprintf(stderr, "error: can't allocate memory\n"); + exit(EXIT_FAILURE); + } + strcpy(fpath, watched_dir); strcat(fpath, "/"); strcat(fpath, expected_file); From 488d0ed21cfa3d04696240e19c494861c5c1fa42 Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Mon, 19 Dec 2022 16:41:48 +0200 Subject: [PATCH 2/9] logger: exit with error if calloc fails In user-space tools, memory allocations can reasonably be expected to always succeed. Make this assumption explicit by adding error handling after calloc. Signed-off-by: Kai Vehmanen (cherry picked from commit 4bec5b292c708a84a22a0a4d84e9747d61ecd70d) --- tools/logger/convert.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tools/logger/convert.c b/tools/logger/convert.c index 569f81579f52..fb0c8cdc2777 100644 --- a/tools/logger/convert.c +++ b/tools/logger/convert.c @@ -131,6 +131,10 @@ static const char *format_uid(uint32_t uid_ptr, int use_colors, bool be, bool up if (uid_ptr < uids_dict->base_address || uid_ptr >= uids_dict->base_address + uids_dict->data_length) { str = calloc(1, strlen(BAD_PTR_STR) + 1 + 6); + if (!str) { + log_err("can't allocate memory\n"); + exit(EXIT_FAILURE); + } sprintf(str, BAD_PTR_STR, uid_ptr); } else { uid_entry = get_uuid_entry(uid_ptr); From 4985fdfd764c1925f879cdb9e43b22ca74e7a99c Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 20 Dec 2022 08:02:48 +0100 Subject: [PATCH 3/9] copier: fix a potentially uninitialised warning The create_dai() function can return an uninitialised value. Fix it by reducing the scope of the variable. Signed-off-by: Guennadi Liakhovetski (cherry picked from commit aff99b209f05c55f5fdd21bf46f826a0d2a00bb3) Signed-off-by: Kai Vehmanen --- src/audio/copier/copier.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/src/audio/copier/copier.c b/src/audio/copier/copier.c index f1c816af9ee2..4a478a775b3e 100644 --- a/src/audio/copier/copier.c +++ b/src/audio/copier/copier.c @@ -312,7 +312,6 @@ static int create_dai(struct comp_dev *parent_dev, struct copier_data *cd, const struct comp_driver *drv; struct ipc_config_dai dai; int dai_count; - int ret; int i; drv = ipc4_get_drv((uint8_t *)&id); @@ -388,15 +387,17 @@ static int create_dai(struct comp_dev *parent_dev, struct copier_data *cd, } for (i = 0; i < dai_count; i++) { + int ret; + dai.dai_index = dai_index[i]; ret = init_dai(parent_dev, drv, config, copier, pipeline, &dai, type, i); if (ret) { comp_err(parent_dev, "failed to create dai"); - break; + return ret; } } - return ret; + return 0; } static int init_pipeline_reg(struct comp_dev *dev) From 3df3222239d6c312675fbc82706336106067f1d1 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 20 Dec 2022 08:06:51 +0100 Subject: [PATCH 4/9] data-blob: fix a NULL dereference If the main context variable is NULL, it cannot be used to print an error message. Since this actually should never happen, use an assertion similar to other similar cases in the file. Signed-off-by: Guennadi Liakhovetski (cherry picked from commit 10251c5037467babe835dcdb5305cd89c21b4e7e) Signed-off-by: Kai Vehmanen --- src/audio/data_blob.c | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/src/audio/data_blob.c b/src/audio/data_blob.c index 359dbe0bba5f..8179225350db 100644 --- a/src/audio/data_blob.c +++ b/src/audio/data_blob.c @@ -281,11 +281,7 @@ int ipc4_comp_data_blob_set(struct comp_data_blob_handler *blob_handler, int ret; int valid_data_size; - if (!blob_handler) { - comp_err(blob_handler->dev, - "ipc4_comp_data_blob_set(): blob_handler is NULL!"); - return -EINVAL; - } + assert(blob_handler); comp_dbg(blob_handler->dev, "ipc4_comp_data_blob_set(): data_offset = %d", From 9eb62769fdca6b40ad99c23995ded082cbe4b6e4 Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 20 Dec 2022 08:10:35 +0100 Subject: [PATCH 5/9] ipc: ipc4: fix an uninitialised variable This is a theoretical case of an invalid pipeline status, handle it correctly. Signed-off-by: Guennadi Liakhovetski (cherry picked from commit 40e6c1ddc061e04687a5d48e38dc02c37ace0703) Signed-off-by: Kai Vehmanen --- src/ipc/ipc4/handler.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/ipc/ipc4/handler.c b/src/ipc/ipc4/handler.c index cd4fd74fd20b..c52b7671f3c9 100644 --- a/src/ipc/ipc4/handler.c +++ b/src/ipc/ipc4/handler.c @@ -275,6 +275,10 @@ static int set_pipeline_state(struct ipc_comp_dev *ppl_icd, uint32_t cmd, } if (ret == PPL_STATUS_SCHEDULED) *delayed = true; + break; + default: + tr_err(&ipc_tr, "ipc: invalid status %d for RESET", status); + return IPC4_INVALID_REQUEST; } /* From eb0f26ddfdfbbf3bc001e3d55a877e5aecdbdb7e Mon Sep 17 00:00:00 2001 From: Guennadi Liakhovetski Date: Tue, 20 Dec 2022 08:13:36 +0100 Subject: [PATCH 6/9] probe: simplify ipc4_get_buffer() Use a local variable in ipc4_get_buffer() to simplify it and remove redundant code. Signed-off-by: Guennadi Liakhovetski (cherry picked from commit b516172a4147ca6198b3a7acb1b16beecb5bc977) Signed-off-by: Kai Vehmanen --- src/probe/probe.c | 30 +++++++++++------------------- 1 file changed, 11 insertions(+), 19 deletions(-) diff --git a/src/probe/probe.c b/src/probe/probe.c index 9f09e4374e37..5289936ab230 100644 --- a/src/probe/probe.c +++ b/src/probe/probe.c @@ -979,41 +979,33 @@ static struct comp_buffer *ipc4_get_buffer(struct ipc_comp_dev *dev, probe_point struct comp_buffer *buf; struct comp_buffer __sparse_cache *buf_c; struct list_item *sink_list, *source_list; + unsigned int queue_id; switch (probe_point.fields.type) { case PROBE_TYPE_INPUT: - if (list_is_empty(&dev->cd->bsource_list)) - return NULL; - list_for_item(source_list, &dev->cd->bsource_list) { buf = container_of(source_list, struct comp_buffer, sink_list); buf_c = buffer_acquire(buf); - - if (IPC4_SRC_QUEUE_ID(buf_c->id) == probe_point.fields.index) - break; - + queue_id = IPC4_SRC_QUEUE_ID(buf_c->id); buffer_release(buf_c); + + if (queue_id == probe_point.fields.index) + return buf; } break; case PROBE_TYPE_OUTPUT: - if (list_is_empty(&dev->cd->bsink_list)) - return NULL; - list_for_item(sink_list, &dev->cd->bsink_list) { buf = container_of(sink_list, struct comp_buffer, source_list); buf_c = buffer_acquire(buf); - - if (IPC4_SINK_QUEUE_ID(buf_c->id) == probe_point.fields.index) - break; - + queue_id = IPC4_SINK_QUEUE_ID(buf_c->id); buffer_release(buf_c); + + if (queue_id == probe_point.fields.index) + return buf; } - break; - default: - return NULL; } - buffer_release(buf_c); - return buf; + + return NULL; } #endif From bf39b096b47c358c2132ddcf39950f3698c2abcb Mon Sep 17 00:00:00 2001 From: Jaroslaw Stelter Date: Mon, 5 Dec 2022 17:43:13 +0100 Subject: [PATCH 7/9] library_manager: Fix dma_ext initialization issue In the lib_manager_dma_deinit() function proper checks for dma_ext structure fields must be added to avoid calling dma api with uninitialized parameters. Signed-off-by: Jaroslaw Stelter (cherry picked from commit 4cc849d34b1b674e27c83f7fe29ef8cd0602563f) Signed-off-by: Kai Vehmanen --- src/library_manager/lib_manager.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/library_manager/lib_manager.c b/src/library_manager/lib_manager.c index 5eeff4686986..55bb0782673c 100644 --- a/src/library_manager/lib_manager.c +++ b/src/library_manager/lib_manager.c @@ -416,6 +416,8 @@ static int lib_manager_dma_init(struct lib_manager_dma_ext *dma_ext, uint32_t dm int chan_index; int ret; + /* Initialize dma_ext with zeros */ + memset(dma_ext, 0, sizeof(struct lib_manager_dma_ext)); /* request DMA in the dir HMEM->LMEM */ dma_ext->dma = dma_get(DMA_DIR_HMEM_TO_LMEM, 0, DMA_DEV_HOST, DMA_ACCESS_EXCLUSIVE); @@ -451,8 +453,11 @@ static int lib_manager_dma_init(struct lib_manager_dma_ext *dma_ext, uint32_t dm static int lib_manager_dma_deinit(struct lib_manager_dma_ext *dma_ext, uint32_t dma_id) { - dma_release_channel(dma_ext->dma->z_dev, dma_id); - dma_put(dma_ext->dma); + if (dma_ext->dma) { + dma_put(dma_ext->dma); + if (dma_ext->dma->z_dev) + dma_release_channel(dma_ext->dma->z_dev, dma_id); + } return 0; } From f0216526be88ad89071f361259f7398bb17699d6 Mon Sep 17 00:00:00 2001 From: Jaroslaw Stelter Date: Mon, 5 Dec 2022 17:46:06 +0100 Subject: [PATCH 8/9] iadk modules: Add variable initialization This patch adds initialization of module_size_ variable in SystemAgent() creator. Signed-off-by: Jaroslaw Stelter (cherry picked from commit 9091635f10557a250221a6fe339153b548238d26) Signed-off-by: Kai Vehmanen --- src/audio/module_adapter/iadk/system_agent.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/audio/module_adapter/iadk/system_agent.cpp b/src/audio/module_adapter/iadk/system_agent.cpp index fc0e11bb6d1b..6e03a065d49f 100644 --- a/src/audio/module_adapter/iadk/system_agent.cpp +++ b/src/audio/module_adapter/iadk/system_agent.cpp @@ -81,10 +81,10 @@ SystemAgent::SystemAgent(uint32_t module_id, core_id_(core_id), module_id_(module_id), instance_id_(instance_id), - module_handle_(NULL) + module_handle_(NULL), + module_size_(0) {} - void SystemAgent::CheckIn(ProcessingModuleInterface& processing_module, ModuleHandle &module_handle, LogHandle *&log_handle) From f42250a65080365003aeb4b6bc76976d44bd5c72 Mon Sep 17 00:00:00 2001 From: Kai Vehmanen Date: Mon, 19 Dec 2022 16:55:24 +0200 Subject: [PATCH 9/9] sof-logger: print error if -u uart option is given with no infile sof-logger -u 115200 -d /lib/firmware/sof-foo.ldc Leads to silent failure as a NULL is passed to open(). Add explicit error handling for this case. Signed-off-by: Kai Vehmanen (cherry picked from commit 80adcdf36a1d0e0590012fe73c7fd4d593a9b3cd) --- tools/logger/logger.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/tools/logger/logger.c b/tools/logger/logger.c index 11e3753638bf..b61023a3f301 100644 --- a/tools/logger/logger.c +++ b/tools/logger/logger.c @@ -441,6 +441,11 @@ int main(int argc, char *argv[]) if (!config.in_file && !config.dump_ldc) config.in_file = "/sys/kernel/debug/sof/etrace"; + if (!config.in_file && baud) { + fprintf(stderr, "error: No UART device specified\n"); + usage(); + } + if (config.input_std) { config.in_fd = stdin; } else if (baud) {