From eb8c7b071a449be3d1331e0961c8fdd0a78efca9 Mon Sep 17 00:00:00 2001 From: Charles Giessen Date: Wed, 8 May 2024 14:31:15 -0500 Subject: [PATCH] Check for NULL in unloading of preloaded-icd's strcmp has undefined behavior if either parameter is NULL. Due to clearing out unloaded entries, this would cause unload_drivers_without_physical_devices to try to compare against already unloaded entries. This commit also condenses the preloaded_icds list so that it doesn't contain gaps. --- loader/loader.c | 15 ++++++++++++++- tests/loader_regression_tests.cpp | 29 +++++++++++++++++++++++++++++ 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/loader/loader.c b/loader/loader.c index 98ad3784..6bcefcd7 100644 --- a/loader/loader.c +++ b/loader/loader.c @@ -6527,8 +6527,21 @@ void unload_drivers_without_physical_devices(struct loader_instance *inst) { loader_platform_thread_lock_mutex(&loader_preload_icd_lock); if (NULL != preloaded_icds.scanned_list) { for (uint32_t i = 0; i < preloaded_icds.count; i++) { - if (strcmp(preloaded_icds.scanned_list[i].lib_name, scanned_icd_to_remove->lib_name) == 0) { + if (NULL != preloaded_icds.scanned_list[i].lib_name && NULL != scanned_icd_to_remove->lib_name && + strcmp(preloaded_icds.scanned_list[i].lib_name, scanned_icd_to_remove->lib_name) == 0) { loader_unload_scanned_icd(inst, &preloaded_icds.scanned_list[i]); + // condense the list so that it doesn't contain empty elements. + if (i < preloaded_icds.count - 1) { + memcpy((void *)&preloaded_icds.scanned_list[i], + (void *)&preloaded_icds.scanned_list[preloaded_icds.count - 1], + sizeof(struct loader_scanned_icd)); + memset((void *)&preloaded_icds.scanned_list[preloaded_icds.count - 1], 0, + sizeof(struct loader_scanned_icd)); + } + if (i > 0) { + preloaded_icds.count--; + } + break; } } diff --git a/tests/loader_regression_tests.cpp b/tests/loader_regression_tests.cpp index 523b5bf6..366936fa 100644 --- a/tests/loader_regression_tests.cpp +++ b/tests/loader_regression_tests.cpp @@ -4679,6 +4679,35 @@ TEST(DriverUnloadingFromZeroPhysDevs, AtFrontAndBack) { } } +TEST(DriverUnloadingFromZeroPhysDevs, MultipleEnumerateCalls) { + FrameworkEnvironment env{}; + add_empty_driver_for_unloading_testing(env); + add_empty_driver_for_unloading_testing(env); + add_driver_for_unloading_testing(env); + add_driver_for_unloading_testing(env); + add_empty_driver_for_unloading_testing(env); + add_empty_driver_for_unloading_testing(env); + + uint32_t extension_count = 0; + ASSERT_EQ(VK_SUCCESS, env.vulkan_functions.vkEnumerateInstanceExtensionProperties(nullptr, &extension_count, 0)); + ASSERT_EQ(extension_count, 6U); // default extensions + surface extensions + std::array extensions; + ASSERT_EQ(VK_SUCCESS, + env.vulkan_functions.vkEnumerateInstanceExtensionProperties(nullptr, &extension_count, extensions.data())); + + { + InstWrapper inst{env.vulkan_functions}; + inst.CheckCreate(); + auto phys_devs1 = inst.GetPhysDevs(); + auto phys_devs2 = inst.GetPhysDevs(); + } + { + InstWrapper inst{env.vulkan_functions}; + inst.CheckCreate(); + auto phys_devs1 = inst.GetPhysDevs(); + auto phys_devs2 = inst.GetPhysDevs(); + } +} TEST(DriverUnloadingFromZeroPhysDevs, NoPhysicalDevices) { FrameworkEnvironment env{}; add_empty_driver_for_unloading_testing(env); -- 2.34.1