diff --git a/loader/loader.c b/loader/loader.c index 58f680466..579f7e8f3 100644 --- a/loader/loader.c +++ b/loader/loader.c @@ -7455,6 +7455,11 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_EnumerateDeviceExtensionProperties(VkP if (res != VK_SUCCESS) { return res; } + // A driver returning VK_SUCCESS must not claim it wrote more entries than the storage it was given. Clamp so the + // de-duplication scan and the appends below stay inside the caller's pProperties buffer. + if (written_count > *pPropertyCount) { + written_count = *pPropertyCount; + } // Iterate over active layers, if they are an implicit layer, add their device extensions // After calling into the driver, written_count contains the amount of device extensions written. We can therefore write diff --git a/tests/framework/icd/test_icd.cpp b/tests/framework/icd/test_icd.cpp index 13abde7aa..e9add8d95 100644 --- a/tests/framework/icd/test_icd.cpp +++ b/tests/framework/icd/test_icd.cpp @@ -683,7 +683,12 @@ VKAPI_ATTR VkResult VKAPI_CALL test_vkEnumerateDeviceExtensionProperties(VkPhysi assert(false && "Drivers don't contain layers???"); return VK_SUCCESS; } else { // instance extensions - return FillCountPtr(phys_dev.extensions, pPropertyCount, pProperties); + VkResult res = FillCountPtr(phys_dev.extensions, pPropertyCount, pProperties); + if (pProperties != nullptr && phys_dev.overreported_device_extension_count != 0) { + *pPropertyCount = phys_dev.overreported_device_extension_count; + return VK_SUCCESS; + } + return res; } } diff --git a/tests/framework/icd/test_icd.h b/tests/framework/icd/test_icd.h index 3e3fc4936..32a6b10a6 100644 --- a/tests/framework/icd/test_icd.h +++ b/tests/framework/icd/test_icd.h @@ -112,6 +112,11 @@ struct PhysicalDevice { BUILDER_VECTOR(Extension, extensions, extension) + // When non-zero, vkEnumerateDeviceExtensionProperties reports this many device extensions on the fill + // call (pProperties != nullptr) and returns VK_SUCCESS, ignoring the caller's buffer size. Models a + // driver that writes back a count larger than the storage it was handed. + BUILDER_VALUE_WITH_DEFAULT(uint32_t, overreported_device_extension_count, 0) + BUILDER_VALUE(VkSurfaceCapabilitiesKHR, surface_capabilities) BUILDER_VALUE_WITH_DEFAULT(VkResult, surface_capabilities_result, VK_SUCCESS) BUILDER_VECTOR(VkSurfaceFormatKHR, surface_formats, surface_format) diff --git a/tests/loader_regression_tests.cpp b/tests/loader_regression_tests.cpp index 667aae427..1f93a6489 100644 --- a/tests/loader_regression_tests.cpp +++ b/tests/loader_regression_tests.cpp @@ -600,6 +600,35 @@ TEST(EnumerateDeviceExtensionProperties, NoDriverExtensionsImplicitLayerPresentW exercise_EnumerateDeviceExtensionProperties(inst, physical_device, exts); } +// A driver that returns VK_SUCCESS but reports more device extensions than it actually wrote must not +// make the loader read past the caller's pProperties buffer while de-duplicating an implicit layer's +// device extensions against the driver's list. +TEST(EnumerateDeviceExtensionProperties, ImplicitLayerDriverOverreportsWrittenCount) { + FrameworkEnvironment env{}; + auto& driver_phys_dev = env.add_icd(TEST_ICD_PATH_VERSION_2).add_and_get_physical_device({}); + driver_phys_dev.overreported_device_extension_count = 64; + + std::vector layer_exts{{"LayerDeviceExt", 1}}; + env.add_implicit_layer({}, ManifestLayer{}.add_layer(ManifestLayer::LayerDescription{} + .set_name("implicit_layer_name") + .set_lib_path(TEST_LAYER_PATH_EXPORT_VERSION_2) + .set_disable_environment("DISABLE_ME") + .add_device_extensions({layer_exts}))); + env.get_test_layer().device_extensions = {Extension{"LayerDeviceExt", 1}}; + + InstWrapper inst{env.vulkan_functions}; + inst.CheckCreate(); + + VkPhysicalDevice physical_device = inst.GetPhysDev(); + + // Hand the loader a single-element buffer. The driver claims 64 extensions were written, so the + // implicit-layer dedup loop would read 64 entries out of this 1-element allocation without a clamp. + std::array storage{}; + uint32_t extension_count = static_cast(storage.size()); + ASSERT_EQ(VK_INCOMPLETE, + inst->vkEnumerateDeviceExtensionProperties(physical_device, nullptr, &extension_count, storage.data())); +} + TEST(EnumerateDeviceExtensionProperties, NoDriverExtensionsImplicitLayerPresentWithLotsOfExtensions) { FrameworkEnvironment env{}; env.add_icd(TEST_ICD_PATH_VERSION_2).add_physical_device({});