From ed086aa8cf4015342c2ba3073ae5084a323bf36f Mon Sep 17 00:00:00 2001 From: Charles Giessen Date: Mon, 23 Dec 2024 16:09:24 -0600 Subject: [PATCH 1/5] Rename loader_platform_* to test_platform_* This prevents conflicts with the loaders and tests platform abstractions. --- tests/framework/test_util.h | 42 ++++++++++++++++++------------------- 1 file changed, 20 insertions(+), 22 deletions(-) diff --git a/tests/framework/test_util.h b/tests/framework/test_util.h index cf8a43532..38495ba92 100644 --- a/tests/framework/test_util.h +++ b/tests/framework/test_util.h @@ -242,26 +242,26 @@ class FolderManager { inline void copy_string_to_char_array(std::string const& src, char* dst, size_t size_dst) { dst[src.copy(dst, size_dst - 1)] = 0; } #if defined(WIN32) -typedef HMODULE loader_platform_dl_handle; -inline loader_platform_dl_handle loader_platform_open_library(const wchar_t* lib_path) { +typedef HMODULE test_platform_dl_handle; +inline test_platform_dl_handle test_platform_open_library(const wchar_t* lib_path) { // Try loading the library the original way first. - loader_platform_dl_handle lib_handle = LoadLibraryW(lib_path); + test_platform_dl_handle lib_handle = LoadLibraryW(lib_path); if (lib_handle == nullptr && GetLastError() == ERROR_MOD_NOT_FOUND) { // If that failed, then try loading it with broader search folders. lib_handle = LoadLibraryExW(lib_path, nullptr, LOAD_LIBRARY_SEARCH_DEFAULT_DIRS | LOAD_LIBRARY_SEARCH_DLL_LOAD_DIR); } return lib_handle; } -inline void loader_platform_open_library_print_error(std::filesystem::path const& libPath) { +inline void test_platform_open_library_print_error(std::filesystem::path const& libPath) { std::wcerr << L"Unable to open library: " << libPath << L" due to: " << std::to_wstring(GetLastError()) << L"\n"; } -inline void loader_platform_close_library(loader_platform_dl_handle library) { FreeLibrary(library); } -inline void* loader_platform_get_proc_address(loader_platform_dl_handle library, const char* name) { +inline void test_platform_close_library(test_platform_dl_handle library) { FreeLibrary(library); } +inline void* test_platform_get_proc_address(test_platform_dl_handle library, const char* name) { assert(library); assert(name); return reinterpret_cast(GetProcAddress(library, name)); } -inline char* loader_platform_get_proc_address_error(const char* name) { +inline char* test_platform_get_proc_address_error(const char* name) { static char errorMsg[120]; snprintf(errorMsg, 119, "Failed to find function \"%s\" in dynamic library", name); return errorMsg; @@ -269,26 +269,24 @@ inline char* loader_platform_get_proc_address_error(const char* name) { #elif COMMON_UNIX_PLATFORMS -typedef void* loader_platform_dl_handle; -inline loader_platform_dl_handle loader_platform_open_library(const char* libPath) { - return dlopen(libPath, RTLD_LAZY | RTLD_LOCAL); -} -inline void loader_platform_open_library_print_error(std::filesystem::path const& libPath) { +typedef void* test_platform_dl_handle; +inline test_platform_dl_handle test_platform_open_library(const char* libPath) { return dlopen(libPath, RTLD_LAZY | RTLD_LOCAL); } +inline void test_platform_open_library_print_error(std::filesystem::path const& libPath) { std::wcerr << "Unable to open library: " << libPath << " due to: " << dlerror() << "\n"; } -inline void loader_platform_close_library(loader_platform_dl_handle library) { +inline void test_platform_close_library(test_platform_dl_handle library) { char* loader_disable_dynamic_library_unloading_env_var = getenv("VK_LOADER_DISABLE_DYNAMIC_LIBRARY_UNLOADING"); if (NULL == loader_disable_dynamic_library_unloading_env_var || 0 != strncmp(loader_disable_dynamic_library_unloading_env_var, "1", 2)) { dlclose(library); } } -inline void* loader_platform_get_proc_address(loader_platform_dl_handle library, const char* name) { +inline void* test_platform_get_proc_address(test_platform_dl_handle library, const char* name) { assert(library); assert(name); return dlsym(library, name); } -inline const char* loader_platform_get_proc_address_error([[maybe_unused]] const char* name) { return dlerror(); } +inline const char* test_platform_get_proc_address_error([[maybe_unused]] const char* name) { return dlerror(); } #endif class FromVoidStarFunc { @@ -308,15 +306,15 @@ class FromVoidStarFunc { struct LibraryWrapper { explicit LibraryWrapper() noexcept {} explicit LibraryWrapper(std::filesystem::path const& lib_path) noexcept : lib_path(lib_path) { - lib_handle = loader_platform_open_library(lib_path.native().c_str()); + lib_handle = test_platform_open_library(lib_path.native().c_str()); if (lib_handle == nullptr) { - loader_platform_open_library_print_error(lib_path); + test_platform_open_library_print_error(lib_path); assert(lib_handle != nullptr && "Must be able to open library"); } } ~LibraryWrapper() noexcept { if (lib_handle != nullptr) { - loader_platform_close_library(lib_handle); + test_platform_close_library(lib_handle); lib_handle = nullptr; } } @@ -328,7 +326,7 @@ struct LibraryWrapper { LibraryWrapper& operator=(LibraryWrapper&& wrapper) noexcept { if (this != &wrapper) { if (lib_handle != nullptr) { - loader_platform_close_library(lib_handle); + test_platform_close_library(lib_handle); } lib_handle = wrapper.lib_handle; lib_path = wrapper.lib_path; @@ -338,9 +336,9 @@ struct LibraryWrapper { } FromVoidStarFunc get_symbol(const char* symbol_name) const { assert(lib_handle != nullptr && "Cannot get symbol with null library handle"); - void* symbol = loader_platform_get_proc_address(lib_handle, symbol_name); + void* symbol = test_platform_get_proc_address(lib_handle, symbol_name); if (symbol == nullptr) { - fprintf(stderr, "Unable to open symbol %s: %s\n", symbol_name, loader_platform_get_proc_address_error(symbol_name)); + fprintf(stderr, "Unable to open symbol %s: %s\n", symbol_name, test_platform_get_proc_address_error(symbol_name)); assert(symbol != nullptr && "Must be able to get symbol"); } return FromVoidStarFunc(symbol); @@ -348,7 +346,7 @@ struct LibraryWrapper { explicit operator bool() const noexcept { return lib_handle != nullptr; } - loader_platform_dl_handle lib_handle = nullptr; + test_platform_dl_handle lib_handle = nullptr; std::filesystem::path lib_path; }; From fc74c1ccd8f384f40a83be579085c502f8921334 Mon Sep 17 00:00:00 2001 From: Charles Giessen Date: Mon, 6 Jan 2025 13:51:09 -0600 Subject: [PATCH 2/5] Use GoogleTest as static library --- tests/CMakeLists.txt | 10 ---------- tests/framework/CMakeLists.txt | 1 - 2 files changed, 11 deletions(-) diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 7020df5da..ba4445614 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -32,7 +32,6 @@ if (IS_DIRECTORY "${GOOGLETEST_INSTALL_DIR}/googletest") set(BUILD_GTEST ON) set(BUILD_GMOCK OFF) set(gtest_force_shared_crt ON) - set(BUILD_SHARED_LIBS ON) set(INSTALL_GTEST OFF) add_subdirectory("${GOOGLETEST_INSTALL_DIR}" ${CMAKE_CURRENT_BINARY_DIR}/gtest) else() @@ -111,20 +110,11 @@ if (ENABLE_LIVE_VERIFICATION_TESTS) endif() if(WIN32) - # Copy loader and googletest (gtest) libs to test dir so the test executable can find them. - add_custom_command(TARGET test_regression POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy $ $) # Copy the loader shared lib (if built) to the test application directory so the test app finds it. if(TARGET vulkan) add_custom_command(TARGET test_regression POST_BUILD COMMAND ${CMAKE_COMMAND} -E copy $ $) endif() - - # Copy the gtest shared lib (if built) to the live verification tests directory so the tests finds it. - if(ENABLE_LIVE_VERIFICATION_TESTS) - add_custom_command(TARGET test_regression POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy $ $) - endif() endif() # https://discourse.cmake.org/t/googletest-crash-when-using-cmake-xcode-arm64/5766 diff --git a/tests/framework/CMakeLists.txt b/tests/framework/CMakeLists.txt index 38c78137b..7e34c8ce2 100644 --- a/tests/framework/CMakeLists.txt +++ b/tests/framework/CMakeLists.txt @@ -86,7 +86,6 @@ add_library(testing_dependencies STATIC test_environment.cpp test_environment.h) target_link_libraries(testing_dependencies PUBLIC gtest Vulkan::Headers testing_framework_util shim-library) target_include_directories(testing_dependencies PUBLIC ${CMAKE_CURRENT_BINARY_DIR}) -target_compile_definitions(testing_dependencies PUBLIC "GTEST_LINKED_AS_SHARED_LIBRARY=1") if (APPLE_STATIC_LOADER) target_compile_definitions(testing_dependencies PUBLIC "APPLE_STATIC_LOADER=1") target_link_libraries(testing_dependencies PUBLIC vulkan) From d55576110b3532baa9aaf2062e0d7f2db330dab7 Mon Sep 17 00:00:00 2001 From: Charles Giessen Date: Mon, 23 Dec 2024 16:11:37 -0600 Subject: [PATCH 3/5] Allow tests to directly call loader internal functions This allows the fuzz tests to faithfully recreate the calling environment of OSS-Fuzz, meaning reproduction of failing fuzz tests can be done in the repo for regression testing. The implementation of this requires exporting several functions that the fuzz tests need which is done with a new macro that is only active when a specific compiler definition is set, which the build does when testing is active. --- loader/CMakeLists.txt | 4 ++++ loader/cJSON.c | 6 ++++-- loader/cJSON.h | 6 ++++-- loader/loader.c | 3 ++- loader/loader.h | 3 ++- loader/loader_json.c | 2 +- loader/loader_json.h | 2 +- loader/settings.c | 6 +++--- loader/settings.h | 7 ++++--- loader/vk_loader_platform.h | 11 +++++++++++ tests/CMakeLists.txt | 18 ++++++++++++++---- tests/loader_fuzz_tests.cpp | 35 ++++++++++++++++++++++++++++------- 12 files changed, 78 insertions(+), 25 deletions(-) diff --git a/loader/CMakeLists.txt b/loader/CMakeLists.txt index 1497018c3..624192dc3 100644 --- a/loader/CMakeLists.txt +++ b/loader/CMakeLists.txt @@ -517,6 +517,10 @@ if (UNKNOWN_FUNCTIONS_SUPPORTED) add_dependencies(vulkan loader_asm_gen_files) endif() +if (BUILD_TESTS) + target_compile_definitions(vulkan PRIVATE SHOULD_EXPORT_TEST_FUNCTIONS) +endif() + if (APPLE_STATIC_LOADER) # TLDR: This feature only exists at the request of Google for Chromium. No other project should use this! message(NOTICE "Apple STATIC lib: it will be built but not installed, and vulkan.pc and VulkanLoaderConfig.cmake won't be generated!") diff --git a/loader/cJSON.c b/loader/cJSON.c index da4ba8519..d4eb0c93d 100644 --- a/loader/cJSON.c +++ b/loader/cJSON.c @@ -150,7 +150,7 @@ static cJSON *cJSON_New_Item(const VkAllocationCallbacks *pAllocator) { } /* Delete a cJSON structure. */ -CJSON_PUBLIC(void) loader_cJSON_Delete(cJSON *item) { +TEST_FUNCTION_EXPORT CJSON_PUBLIC(void) loader_cJSON_Delete(cJSON *item) { cJSON *next = NULL; while (item != NULL) { next = item->next; @@ -949,7 +949,9 @@ static unsigned char *print(const cJSON *const item, cJSON_bool format, bool *ou } /* Render a cJSON item/entity/structure to text. */ -CJSON_PUBLIC(char *) loader_cJSON_Print(const cJSON *item, bool *out_of_memory) { return (char *)print(item, true, out_of_memory); } +TEST_FUNCTION_EXPORT CJSON_PUBLIC(char *) loader_cJSON_Print(const cJSON *item, bool *out_of_memory) { + return (char *)print(item, true, out_of_memory); +} CJSON_PUBLIC(char *) loader_cJSON_PrintUnformatted(const cJSON *item, bool *out_of_memory) { return (char *)print(item, false, out_of_memory); diff --git a/loader/cJSON.h b/loader/cJSON.h index 85f6f07d7..457d669a5 100644 --- a/loader/cJSON.h +++ b/loader/cJSON.h @@ -88,6 +88,8 @@ then using the CJSON_API_VISIBILITY flag to "export" the same symbols the way CJ #include #include +#include "vk_loader_platform.h" + /* cJSON Types: */ #define cJSON_Invalid (0) #define cJSON_False (1 << 0) @@ -163,7 +165,7 @@ loader_cJSON_ParseWithLengthOpts(const VkAllocationCallbacks *pAllocator, const const char **return_parse_end, cJSON_bool require_null_terminated, bool *out_of_memory); /* Render a cJSON entity to text for transfer/storage. */ -CJSON_PUBLIC(char *) loader_cJSON_Print(const cJSON *item, bool *out_of_memory); +TEST_FUNCTION_EXPORT CJSON_PUBLIC(char *) loader_cJSON_Print(const cJSON *item, bool *out_of_memory); /* Render a cJSON entity to text for transfer/storage without any formatting. */ CJSON_PUBLIC(char *) loader_cJSON_PrintUnformatted(const cJSON *item, bool *out_of_memory); /* Render a cJSON entity to text using a buffered strategy. prebuffer is a guess at the final size. guessing well reduces @@ -177,7 +179,7 @@ loader_cJSON_PrintBuffered(const cJSON *item, int prebuffer, cJSON_bool fmt, boo CJSON_PUBLIC(cJSON_bool) loader_cJSON_PrintPreallocated(cJSON *item, char *buffer, const int length, const cJSON_bool format); /* Delete a cJSON entity and all subentities. */ -CJSON_PUBLIC(void) loader_cJSON_Delete(cJSON *item); +TEST_FUNCTION_EXPORT CJSON_PUBLIC(void) loader_cJSON_Delete(cJSON *item); /* Returns the number of items in an array (or object). */ CJSON_PUBLIC(int) loader_cJSON_GetArraySize(const cJSON *array); diff --git a/loader/loader.c b/loader/loader.c index c35ecf2d4..e776594ab 100644 --- a/loader/loader.c +++ b/loader/loader.c @@ -508,7 +508,8 @@ bool loader_find_layer_name_in_blacklist(const char *layer_name, struct loader_l } // Remove all layer properties entries from the list -void loader_delete_layer_list_and_properties(const struct loader_instance *inst, struct loader_layer_list *layer_list) { +TEST_FUNCTION_EXPORT void loader_delete_layer_list_and_properties(const struct loader_instance *inst, + struct loader_layer_list *layer_list) { uint32_t i; if (!layer_list) return; diff --git a/loader/loader.h b/loader/loader.h index 0f1f5ed49..a5527b964 100644 --- a/loader/loader.h +++ b/loader/loader.h @@ -154,7 +154,8 @@ VkResult loader_add_device_extensions(const struct loader_instance *inst, VkResult loader_init_generic_list(const struct loader_instance *inst, struct loader_generic_list *list_info, size_t element_size); void loader_destroy_generic_list(const struct loader_instance *inst, struct loader_generic_list *list); void loader_destroy_pointer_layer_list(const struct loader_instance *inst, struct loader_pointer_layer_list *layer_list); -void loader_delete_layer_list_and_properties(const struct loader_instance *inst, struct loader_layer_list *layer_list); +TEST_FUNCTION_EXPORT void loader_delete_layer_list_and_properties(const struct loader_instance *inst, + struct loader_layer_list *layer_list); void loader_remove_layer_in_list(const struct loader_instance *inst, struct loader_layer_list *layer_list, uint32_t layer_to_remove); VkResult loader_init_scanned_icd_list(const struct loader_instance *inst, struct loader_icd_tramp_list *icd_tramp_list); diff --git a/loader/loader_json.c b/loader/loader_json.c index 6b63666c2..805d278c0 100644 --- a/loader/loader_json.c +++ b/loader/loader_json.c @@ -136,7 +136,7 @@ VkResult loader_read_entire_file(const struct loader_instance *inst, const char } #endif -VkResult loader_get_json(const struct loader_instance *inst, const char *filename, cJSON **json) { +TEST_FUNCTION_EXPORT VkResult loader_get_json(const struct loader_instance *inst, const char *filename, cJSON **json) { char *json_buf = NULL; VkResult res = VK_SUCCESS; diff --git a/loader/loader_json.h b/loader/loader_json.h index 824019b90..b78ffc95d 100644 --- a/loader/loader_json.h +++ b/loader/loader_json.h @@ -38,7 +38,7 @@ struct loader_string_list; // // @return - A pointer to a cJSON object representing the JSON parse tree. // This returned buffer should be freed by caller. -VkResult loader_get_json(const struct loader_instance *inst, const char *filename, cJSON **json); +TEST_FUNCTION_EXPORT VkResult loader_get_json(const struct loader_instance *inst, const char *filename, cJSON **json); // Given a cJSON object, find the string associated with the key and puts an pre-allocated string into out_string. // Length is given by out_str_len, and this function truncates the string with a null terminator if it the provided space isn't diff --git a/loader/settings.c b/loader/settings.c index 24f408f57..a1b2fa323 100644 --- a/loader/settings.c +++ b/loader/settings.c @@ -484,7 +484,7 @@ VkResult get_loader_settings(const struct loader_instance* inst, loader_settings return res; } -VkResult update_global_loader_settings(void) { +TEST_FUNCTION_EXPORT VkResult update_global_loader_settings(void) { loader_settings settings = {0}; VkResult res = get_loader_settings(NULL, &settings); loader_platform_thread_lock_mutex(&global_loader_settings_lock); @@ -538,8 +538,8 @@ void release_current_settings_lock(const struct loader_instance* inst) { } } -VkResult get_settings_layers(const struct loader_instance* inst, struct loader_layer_list* settings_layers, - bool* should_search_for_other_layers) { +TEST_FUNCTION_EXPORT VkResult get_settings_layers(const struct loader_instance* inst, struct loader_layer_list* settings_layers, + bool* should_search_for_other_layers) { VkResult res = VK_SUCCESS; *should_search_for_other_layers = true; // default to true diff --git a/loader/settings.h b/loader/settings.h index 0383bb5ba..4e776ff0a 100644 --- a/loader/settings.h +++ b/loader/settings.h @@ -30,6 +30,7 @@ #include "vulkan/vulkan_core.h" #include "log.h" +#include "vk_loader_platform.h" struct loader_instance; struct loader_layer_list; @@ -83,7 +84,7 @@ void free_loader_settings(const struct loader_instance* inst, loader_settings* l void log_settings(const struct loader_instance* inst, loader_settings* settings); // Every global function needs to call this at startup to insure that -VkResult update_global_loader_settings(void); +TEST_FUNCTION_EXPORT VkResult update_global_loader_settings(void); // Needs to be called during startup - void init_global_loader_settings(void); @@ -94,8 +95,8 @@ bool should_skip_logging_global_messages(VkFlags msg_type); // Query the current settings (either global or per-instance) and return the list of layers contained within. // should_search_for_other_layers tells the caller if the settings file should be used exclusively for layer searching or not -VkResult get_settings_layers(const struct loader_instance* inst, struct loader_layer_list* settings_layers, - bool* should_search_for_other_layers); +TEST_FUNCTION_EXPORT VkResult get_settings_layers(const struct loader_instance* inst, struct loader_layer_list* settings_layers, + bool* should_search_for_other_layers); // Take the provided list of settings_layers and add in the layers from regular search paths // Only adds layers that aren't already present in the settings_layers and in the location of the diff --git a/loader/vk_loader_platform.h b/loader/vk_loader_platform.h index 23908ae58..9a3b9c1c9 100644 --- a/loader/vk_loader_platform.h +++ b/loader/vk_loader_platform.h @@ -89,6 +89,17 @@ #define LOADER_EXPORT #endif +// For testing purposes, we want to expose some functions not normally callable on the library +#if defined(SHOULD_EXPORT_TEST_FUNCTIONS) +#if defined(_WIN32) +#define TEST_FUNCTION_EXPORT __declspec(dllexport) +#else +#define TEST_FUNCTION_EXPORT LOADER_EXPORT +#endif +#else +#define TEST_FUNCTION_EXPORT +#endif + #define MAX_STRING_SIZE 1024 // This is defined in vk_layer.h, but if there's problems we need to create the define diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index ba4445614..72cf32465 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -83,7 +83,6 @@ add_executable( loader_envvar_tests.cpp loader_get_proc_addr_tests.cpp loader_debug_ext_tests.cpp - loader_fuzz_tests.cpp loader_handle_validation_tests.cpp loader_layer_tests.cpp loader_regression_tests.cpp @@ -95,6 +94,14 @@ add_executable( target_link_libraries(test_regression PUBLIC testing_dependencies) target_compile_definitions(test_regression PUBLIC VK_NO_PROTOTYPES) +add_executable( + test_fuzzing + loader_testing_main.cpp + loader_fuzz_tests.cpp +) +target_link_libraries(test_fuzzing PUBLIC testing_dependencies vulkan) +target_include_directories(test_fuzzing PUBLIC ${CMAKE_SOURCE_DIR}/loader ${CMAKE_SOURCE_DIR}/loader/generated) + # Threading tests live in separate executabe just for threading tests as it'll need support # in the test harness to enable in CI, as thread sanitizer doesn't work with address sanitizer enabled. add_executable( @@ -110,10 +117,10 @@ if (ENABLE_LIVE_VERIFICATION_TESTS) endif() if(WIN32) - # Copy the loader shared lib (if built) to the test application directory so the test app finds it. + # Copy vulkan-1.dll to the test_fuzzing build directory so that the test_fuzzing exe can find it. if(TARGET vulkan) - add_custom_command(TARGET test_regression POST_BUILD - COMMAND ${CMAKE_COMMAND} -E copy $ $) + add_custom_command(TARGET test_fuzzing POST_BUILD + COMMAND ${CMAKE_COMMAND} -E copy $ $) endif() endif() @@ -128,8 +135,10 @@ endif() # must happen after the dll's get copied over if(NOT CMAKE_CROSSCOMPILING) gtest_discover_tests(test_regression PROPERTIES DISCOVERY_TIMEOUT 100) + gtest_discover_tests(test_fuzzing PROPERTIES DISCOVERY_TIMEOUT 100) else() gtest_add_tests(TARGET test_regression) + gtest_add_tests(TARGET test_fuzzing) endif() # When APPLE_STATIC_LOADER is ON installation is disabled @@ -163,4 +172,5 @@ set_tests_properties(integration.find_package PROPERTIES DEPENDS integration.ins if (CODE_COVERAGE) target_code_coverage(test_regression AUTO ALL ) + target_code_coverage(test_fuzzing AUTO ALL ) endif() diff --git a/tests/loader_fuzz_tests.cpp b/tests/loader_fuzz_tests.cpp index 897e69a35..b312adbf2 100644 --- a/tests/loader_fuzz_tests.cpp +++ b/tests/loader_fuzz_tests.cpp @@ -27,6 +27,12 @@ #include "test_environment.h" +extern "C" { +#include "loader.h" +#include "loader_json.h" +#include "settings.h" +} + void execute_instance_enumerate_fuzzer(std::filesystem::path const& filename) { FrameworkEnvironment env{}; env.write_file_from_source((std::filesystem::path(CLUSTERFUZZ_TESTCASE_DIRECTORY) / filename).string().c_str(), @@ -75,13 +81,25 @@ void execute_instance_create_fuzzer(std::filesystem::path const& filename) { env.vulkan_functions.vkDestroyInstance(inst, NULL); } -void execute_json_load_fuzzer(std::filesystem::path const& filename) { +void execute_json_load_fuzzer(std::string const& filename) { FrameworkEnvironment env{}; - env.write_file_from_source((std::filesystem::path(CLUSTERFUZZ_TESTCASE_DIRECTORY) / filename).string().c_str(), - ManifestCategory::explicit_layer, ManifestLocation::explicit_layer, "complex_layer.json"); - uint32_t count = 0; - env.vulkan_functions.vkEnumerateInstanceLayerProperties(&count, nullptr); + cJSON* json = NULL; + loader_get_json(NULL, filename.c_str(), &json); + + if (json == NULL) { + return; + } + bool out_of_mem = false; + char* json_data = loader_cJSON_Print(json, &out_of_mem); + + if (json_data != NULL) { + free(json_data); + } + + if (json != NULL) { + loader_cJSON_Delete(json); + } } void execute_setting_fuzzer(std::filesystem::path const& filename) { FrameworkEnvironment env{}; @@ -89,8 +107,11 @@ void execute_setting_fuzzer(std::filesystem::path const& filename) { env.write_file_from_source((std::filesystem::path(CLUSTERFUZZ_TESTCASE_DIRECTORY) / filename).string().c_str(), ManifestCategory::settings, ManifestLocation::settings_location, "vk_loader_settings.json"); - uint32_t version = 0; - env.vulkan_functions.vkEnumerateInstanceVersion(&version); + update_global_loader_settings(); + struct loader_layer_list settings_layers = {0}; + bool should_search_for_other_layers = true; + get_settings_layers(NULL, &settings_layers, &should_search_for_other_layers); + loader_delete_layer_list_and_properties(NULL, &settings_layers); } TEST(BadJsonInput, ClusterFuzzTestCase_5599244505186304) { From fe57f6c89bc800040d0dd58a603e57007caa5e71 Mon Sep 17 00:00:00 2001 From: Charles Giessen Date: Mon, 23 Dec 2024 17:23:17 -0600 Subject: [PATCH 4/5] Always memset newly reallocated memory Fuzz testing found a case where memory was left uninitialized after calling loader_realloc, causing a crash due to reading of that memory. The fix is to *always* memset newly reallocated memory, since a value of zero is a good default value, especially if that memory is for a list. This commit removes the redundant memsets, since realloc now has the responsibility to initialize memory. --- loader/allocation.c | 4 ++++ loader/loader.c | 3 --- ...tcase-instance_create_fuzzer-5817896795701248 | Bin 0 -> 7283 bytes tests/loader_fuzz_tests.cpp | 3 +++ 4 files changed, 7 insertions(+), 3 deletions(-) create mode 100644 tests/framework/data/fuzz_test_minimized_test_cases/clusterfuzz-testcase-instance_create_fuzzer-5817896795701248 diff --git a/loader/allocation.c b/loader/allocation.c index 3397ef992..4ad68027e 100644 --- a/loader/allocation.c +++ b/loader/allocation.c @@ -98,6 +98,10 @@ void *loader_realloc(const VkAllocationCallbacks *pAllocator, void *pMemory, siz #endif } else { pNewMem = realloc(pMemory, size); + // Clear out the newly allocated memory + if (size > orig_size) { + memset((uint8_t *)pNewMem + orig_size, 0, size - orig_size); + } } return pNewMem; } diff --git a/loader/loader.c b/loader/loader.c index e776594ab..9deff6881 100644 --- a/loader/loader.c +++ b/loader/loader.c @@ -323,8 +323,6 @@ VkResult append_str_to_string_list(const struct loader_instance *inst, struct lo loader_instance_heap_free(inst, str); // Must clean up in case of failure return VK_ERROR_OUT_OF_HOST_MEMORY; } - // Null out the new space - memset(string_list->list + string_list->allocated_count, 0, string_list->allocated_count); string_list->allocated_count *= 2; } string_list->list[string_list->count++] = str; @@ -439,7 +437,6 @@ VkResult loader_append_layer_property(const struct loader_instance *inst, struct goto out; } layer_list->list = new_ptr; - memset((uint8_t *)layer_list->list + layer_list->capacity, 0, layer_list->capacity); layer_list->capacity *= 2; } memcpy(&layer_list->list[layer_list->count], layer_property, sizeof(struct loader_layer_properties)); diff --git a/tests/framework/data/fuzz_test_minimized_test_cases/clusterfuzz-testcase-instance_create_fuzzer-5817896795701248 b/tests/framework/data/fuzz_test_minimized_test_cases/clusterfuzz-testcase-instance_create_fuzzer-5817896795701248 new file mode 100644 index 0000000000000000000000000000000000000000..7098959979bddf1618a094c1cd415d20393a417c GIT binary patch literal 7283 zcmeHM&1%~~5LSzO(V^EKBbGuha)=AP^d?$UN{kO~5=e_#RBHoEtRH0A#5E4|mN&?? z=N@wDla)R|IwQ-9C0Q%kl5l=B2OF)v-TiiEc1Fz1WCetr@gbvUwrlDhoiKOAZHv5D zNVBznuzx_*1!$;G;T4R3^Ph3z3@2N<$q;>VNKe~6>Wx`20}&|1n>s$?bN9=U*8R|q zz9+iFS9gU?iMp7`O=rkVW_h{?lvY7YRz42*FxZ3-7-bh8vyi`l=q($;G4)}RsPXUD z&nsNWq33LRfoJV2b5+K3E`_9mLbZe{S?+Y1P)YV8xL=OFqOgT6&=eibHwKaC>MY<> z`S9Cyd#)P`=}908{^XYUfr5#*#m_5eOUOH9@d_L}Q?Jwu@!9ZP&}Wj=NRkN_&uw_x zIXO%*Pw$krwM}Tl#m~h&m346g?#Ao2jzHhJ-MTd3)>bHuLEeROT%4^B^Dx)GUMHAb zGb2n~QC+R~iQ16LU2Z_`!Bm1Nnm{s3W`oYL04{HcuNGbfn}!tXri$h7gX(`&;0P~z zROmQ6b%d`4y!F$h`qpFSG<6oeKaSTJ(7>*)Og}YY-G4$8Q}?wrcqx{o^IojjHtVlrbtSGJvJO~IEB5W zXn)6ttv06Qxw5Ay{h&o>u+hNf4mQiM+;;?xF2!%#Db068eTi!=g$!)du`TGW(YwZ8 zf6;dfFvQ>^{X299nKj|AZDAu0mYo4N;$aVso=z>% literal 0 HcmV?d00001 diff --git a/tests/loader_fuzz_tests.cpp b/tests/loader_fuzz_tests.cpp index b312adbf2..b5b052e31 100644 --- a/tests/loader_fuzz_tests.cpp +++ b/tests/loader_fuzz_tests.cpp @@ -200,6 +200,9 @@ TEST(BadJsonInput, ClusterFuzzTestCase_6353004288081920) { // Stack overflow due to recursive meta layers execute_instance_create_fuzzer("clusterfuzz-testcase-minimized-instance_create_fuzzer-6353004288081920"); } +TEST(BadJsonInput, ClusterFuzzTestCase_5817896795701248) { + execute_instance_create_fuzzer("clusterfuzz-testcase-instance_create_fuzzer-5817896795701248"); +} TEST(BadJsonInput, ClusterFuzzTestCase_6465902356791296) { // Does crash with UBSAN // Doesn't crash with ASAN From e71025a343ca0b465a50e5b3772c64e73e0cb9a2 Mon Sep 17 00:00:00 2001 From: Charles Giessen Date: Mon, 6 Jan 2025 08:23:18 -0600 Subject: [PATCH 5/5] Include fuzz test which needs realloc memsetting --- ...tcase-instance_create_fuzzer-6541440380895232 | Bin 0 -> 1608 bytes tests/loader_fuzz_tests.cpp | 3 +++ 2 files changed, 3 insertions(+) create mode 100644 tests/framework/data/fuzz_test_minimized_test_cases/clusterfuzz-testcase-instance_create_fuzzer-6541440380895232 diff --git a/tests/framework/data/fuzz_test_minimized_test_cases/clusterfuzz-testcase-instance_create_fuzzer-6541440380895232 b/tests/framework/data/fuzz_test_minimized_test_cases/clusterfuzz-testcase-instance_create_fuzzer-6541440380895232 new file mode 100644 index 0000000000000000000000000000000000000000..e748a3c9c24c9fdc879e1d4fe4ef65abd0122044 GIT binary patch literal 1608 zcmcIk&yUhT6fQ>_uU_TlUG}nc>8ck`qSh=xm_ugMzv;Mn5@A`!0ltw30LE&9Mvr=S-uLo~$dNvVql~5^o z3b^F;|DAuX->Yx#aPaD)4pn(JO?0E`116bDMb-+1G~7^!JDt|fX)@MaubATI-q!7X zr=pnXL|Hf2jVMf;rlR58JHlg{$}}nXm?kOYGSRurC=*$)Lp0*s7wcz4v0zKSKq!6R zTm`MUSh?Z;ZO?#w8J5uA9fz%XduV`}L>SSO`93JljzZza3e>_elEl$dKc+$$lgPUl zjBCwT)bY@v3o(Fu80{mI#ou1kzGCxM5Cru+&KihDc3=(n_VrN_>$2(Qqz1-D-jAxw zdK9(7FapOTN7CTwV%A_+xz~07OU;{Vw~8f}I4uBP_OD*{`rDseGOLRXyFYijQKw;{ zuL+eFOnx_9PDs}aIU(YvmSfm?Lh&BdrQ5sL#XePZ;;8e3e(`P%G_qYe+>ji=EeU8! zX;(RL;(+(m-jsbrl=sd0@o5jh_iGime}-QQ+;U}