From ebf696247e75d0fb6d71761ce92a1edabeff514e Mon Sep 17 00:00:00 2001 From: Charles Giessen Date: Thu, 28 Sep 2023 13:08:46 -0600 Subject: [PATCH] Fix problems detected by CodeQL static analysis 3 types of issues were found: * Incorrect format specifiers * Not providing required format specifies * Using alloca in a loop There are multiple instances of alloca in loops, but to fix them would require significant refactoring. This commit includes 1 move of alloca inside a for loop to the outside, but this is because the logic was redoing work in a for loop that could have been done once at the start of the function. --- loader/cJSON.c | 2 +- loader/loader.c | 5 +++-- loader/settings.c | 52 ++++++++++++++++++++++++++++------------------- loader/settings.h | 2 +- 4 files changed, 36 insertions(+), 25 deletions(-) diff --git a/loader/cJSON.c b/loader/cJSON.c index 4fb9585c8e..47faa8357a 100644 --- a/loader/cJSON.c +++ b/loader/cJSON.c @@ -1271,7 +1271,7 @@ VkResult loader_get_json(const struct loader_instance *inst, const char *filenam json_buf = (char *)loader_instance_heap_calloc(inst, len + 1, VK_SYSTEM_ALLOCATION_SCOPE_COMMAND); if (json_buf == NULL) { loader_log(inst, VULKAN_LOADER_ERROR_BIT, 0, - "loader_get_json: Failed to allocate space for JSON file %s buffer of length %d", filename, len); + "loader_get_json: Failed to allocate space for JSON file %s buffer of length %lu", filename, len); res = VK_ERROR_OUT_OF_HOST_MEMORY; goto out; } diff --git a/loader/loader.c b/loader/loader.c index 313dbcdb65..109697149b 100644 --- a/loader/loader.c +++ b/loader/loader.c @@ -1495,7 +1495,8 @@ VkResult loader_add_direct_driver(const struct loader_instance *inst, uint32_t i void *new_ptr = loader_instance_heap_realloc(inst, icd_tramp_list->scanned_list, icd_tramp_list->capacity, icd_tramp_list->capacity * 2, VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE); if (NULL == new_ptr) { - loader_log(inst, VULKAN_LOADER_ERROR_BIT, 0, "loader_add_direct_driver: Realloc failed on icd library list for ICD %s"); + loader_log(inst, VULKAN_LOADER_ERROR_BIT, 0, + "loader_add_direct_driver: Realloc failed on icd library list for ICD index %u", index); return VK_ERROR_OUT_OF_HOST_MEMORY; } icd_tramp_list->scanned_list = new_ptr; @@ -5210,7 +5211,7 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_CreateInstance(const VkInstanceCreateI "#LLP_LAYER_21)"); } else if (LOADER_MAGIC_NUMBER != ptr_instance->magic) { loader_log(ptr_instance, VULKAN_LOADER_WARN_BIT, 0, - "terminator_CreateInstance: Instance pointer (%p) has invalid MAGIC value 0x%08x. Instance value possibly " + "terminator_CreateInstance: Instance pointer (%p) has invalid MAGIC value 0x%08lx. Instance value possibly " "corrupted by active layer (Policy #LLP_LAYER_21). ", ptr_instance, ptr_instance->magic); } diff --git a/loader/settings.c b/loader/settings.c index 8613766754..3d83c9050f 100644 --- a/loader/settings.c +++ b/loader/settings.c @@ -711,15 +711,35 @@ VkResult combine_settings_layers_with_regular_layers(const struct loader_instanc } VkResult enable_correct_layers_from_settings(const struct loader_instance* inst, const struct loader_envvar_all_filters* filters, - uint32_t name_count, const char* const* names, + uint32_t app_enabled_name_count, const char* const* app_enabled_names, const struct loader_layer_list* instance_layers, struct loader_pointer_layer_list* target_layer_list, struct loader_pointer_layer_list* activated_layer_list) { VkResult res = VK_SUCCESS; char* vk_instance_layers_env = loader_getenv(ENABLED_LAYERS_ENV, inst); + struct loader_string_list vk_instance_layers_env_list = {0}; if (vk_instance_layers_env != NULL) { - loader_log(inst, VULKAN_LOADER_WARN_BIT | VULKAN_LOADER_LAYER_BIT, 0, "env var \'%s\' defined and adding layers \"%s\"", - ENABLED_LAYERS_ENV, names); + size_t vk_instance_layers_env_len = strlen(vk_instance_layers_env) + 1; + char* layers_env_copy = loader_stack_alloc(vk_instance_layers_env_len); + if (layers_env_copy) { + loader_strncpy(layers_env_copy, vk_instance_layers_env_len, vk_instance_layers_env, vk_instance_layers_env_len); + + while (layers_env_copy && *layers_env_copy) { + char* next = loader_get_next_path(layers_env_copy); + if (strlen(layers_env_copy)) { + append_str_to_string_list(inst, &vk_instance_layers_env_list, layers_env_copy); + } + layers_env_copy = next; + } + } + loader_log(inst, VULKAN_LOADER_WARN_BIT | VULKAN_LOADER_LAYER_BIT, 0, + "env var \'%s\' defined and adding layers:", ENABLED_LAYERS_ENV); + for (uint32_t i = 0; i < vk_instance_layers_env_list.count; i++) { + if (vk_instance_layers_env_list.list && vk_instance_layers_env_list.list[i]) { + loader_log(inst, VULKAN_LOADER_WARN_BIT | VULKAN_LOADER_LAYER_BIT, 0, " \"%s\"", + vk_instance_layers_env_list.list[i]); + } + } } for (uint32_t i = 0; i < instance_layers->count; i++) { bool enable_layer = false; @@ -745,30 +765,20 @@ VkResult enable_correct_layers_from_settings(const struct loader_instance* inst, enable_layer = true; } - if (!enable_layer && vk_instance_layers_env) { - size_t vk_instance_layers_env_len = strlen(vk_instance_layers_env) + 1; - char* name = loader_stack_alloc(vk_instance_layers_env_len); - if (name != NULL) { - loader_strncpy(name, vk_instance_layers_env_len, vk_instance_layers_env, vk_instance_layers_env_len); - // First look for the old-fashion layers forced on with VK_INSTANCE_LAYERS - while (name && *name) { - char* next = loader_get_next_path(name); - - if (strlen(name) > 0) { - if (0 == strcmp(name, props->info.layerName)) { - enable_layer = true; - break; - } - name = next; - } + if (!enable_layer && vk_instance_layers_env_list.count > 0) { + // First look for the old-fashion layers forced on with VK_INSTANCE_LAYERS + for (uint32_t j = 0; j < vk_instance_layers_env_list.count; j++) { + if (0 == strcmp(vk_instance_layers_env_list.list[j], props->info.layerName)) { + enable_layer = true; + break; } } } // Check if it should be enabled by the application if (!enable_layer) { - for (uint32_t j = 0; j < name_count; j++) { - if (strcmp(props->info.layerName, names[j]) == 0) { + for (uint32_t j = 0; j < app_enabled_name_count; j++) { + if (strcmp(props->info.layerName, app_enabled_names[j]) == 0) { enable_layer = true; break; } diff --git a/loader/settings.h b/loader/settings.h index 1e9dbf0a9a..d3cd8b7cc9 100644 --- a/loader/settings.h +++ b/loader/settings.h @@ -108,7 +108,7 @@ VkResult combine_settings_layers_with_regular_layers(const struct loader_instanc // Fill out activated_layer_list with the layers that should be activated, based on environment variables, VkInstanceCreateInfo, and // the settings VkResult enable_correct_layers_from_settings(const struct loader_instance* inst, const struct loader_envvar_all_filters* filters, - uint32_t name_count, const char* const* names, + uint32_t app_enabled_name_count, const char* const* app_enabled_names, const struct loader_layer_list* instance_layers, struct loader_pointer_layer_list* target_layer_list, struct loader_pointer_layer_list* activated_layer_list);