From 1798c53636a42305582ac8054d6530502e67563d Mon Sep 17 00:00:00 2001 From: Nathan Date: Sat, 27 Nov 2021 04:18:53 -0600 Subject: [PATCH] [trunk] -> [extra] 'vulkan-icd-loader-1.2.199-2' add --- ...ToolProps-on-drivers-without-support.patch | 126 ++++++++++++++++++ trunk/PKGBUILD | 15 ++- ...ToolProps-on-drivers-without-support.patch | 126 ++++++++++++++++++ x86_64/extra/PKGBUILD | 15 ++- 4 files changed, 276 insertions(+), 6 deletions(-) create mode 100644 trunk/0001-Dont-call-ToolProps-on-drivers-without-support.patch create mode 100644 x86_64/extra/0001-Dont-call-ToolProps-on-drivers-without-support.patch diff --git a/trunk/0001-Dont-call-ToolProps-on-drivers-without-support.patch b/trunk/0001-Dont-call-ToolProps-on-drivers-without-support.patch new file mode 100644 index 0000000..c91e0c8 --- /dev/null +++ b/trunk/0001-Dont-call-ToolProps-on-drivers-without-support.patch @@ -0,0 +1,126 @@ +From 22e9c7c538a2d2cfe2a81cb4fee6a710798b8f11 Mon Sep 17 00:00:00 2001 +From: Charles Giessen +Date: Wed, 17 Nov 2021 14:44:11 -0700 +Subject: [PATCH] Dont call ToolProps on drivers without support + +The loader recently added support for calling into drivers in +vkGetPhysicalDeviceToolPropertieesEXT. However, it only used the value of +the function pointer to determine if it was safe. It was found that Mesa +drivers will return a non-null function pointer, even though they do not +support the extension, and so the loader called this function pointer which +would then segfault. + +The loader prevents this by first checking if the extension is supported by +the physical device. This necessitates calling +vkEnumerateDeviceExtensionProperties and allocating some memory on each call +but since the number of times this function is called should be low, it is +not an undue performance burden. In the future, the loader should cache the +list of extensions when calling vkEnumeratePhysicalDevices so that checking if +a extension function is supported is fast and easy. + +Signed-off-by: Laurent Carlier +--- + loader/extension_manual.c | 46 ++++++++++++++++++++++++++++--- + tests/loader_regression_tests.cpp | 21 ++++++++++++-- + 2 files changed, 61 insertions(+), 6 deletions(-) + +diff --git a/loader/extension_manual.c b/loader/extension_manual.c +index 415b64b74..65c573d91 100644 +--- a/loader/extension_manual.c ++++ b/loader/extension_manual.c +@@ -305,11 +305,49 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_GetPhysicalDeviceToolPropertiesEXT(VkP + struct loader_physical_device_term *phys_dev_term = (struct loader_physical_device_term *)physicalDevice; + struct loader_icd_term *icd_term = phys_dev_term->this_icd_term; + +- if (icd_term->dispatch.GetPhysicalDeviceToolPropertiesEXT) { +- return icd_term->dispatch.GetPhysicalDeviceToolPropertiesEXT(phys_dev_term->phys_dev, pToolCount, pToolProperties); ++ bool tooling_info_supported = false; ++ uint32_t ext_count = 0; ++ VkExtensionProperties *ext_props = NULL; ++ VkResult res = VK_SUCCESS; ++ VkResult enumerate_res = VK_SUCCESS; ++ ++ enumerate_res = icd_term->dispatch.EnumerateDeviceExtensionProperties(phys_dev_term->phys_dev, NULL, &ext_count, NULL); ++ if (enumerate_res != VK_SUCCESS) { ++ goto out; + } + ++ ext_props = loader_instance_heap_alloc(icd_term->this_instance, sizeof(VkExtensionProperties) * ext_count, ++ VK_SYSTEM_ALLOCATION_SCOPE_COMMAND); ++ if (!ext_props) { ++ res = VK_ERROR_OUT_OF_HOST_MEMORY; ++ goto out; ++ } ++ ++ enumerate_res = icd_term->dispatch.EnumerateDeviceExtensionProperties(phys_dev_term->phys_dev, NULL, &ext_count, ext_props); ++ if (enumerate_res != VK_SUCCESS) { ++ goto out; ++ } ++ ++ for (uint32_t i = 0; i < ext_count; i++) { ++ if (strncmp(ext_props[i].extensionName, VK_EXT_TOOLING_INFO_EXTENSION_NAME, VK_MAX_EXTENSION_NAME_SIZE) == 0) { ++ tooling_info_supported = true; ++ break; ++ } ++ } ++ ++ if (tooling_info_supported && icd_term->dispatch.GetPhysicalDeviceToolPropertiesEXT) { ++ res = icd_term->dispatch.GetPhysicalDeviceToolPropertiesEXT(phys_dev_term->phys_dev, pToolCount, pToolProperties); ++ } ++ ++out: + // In the case the driver didn't support the extension, make sure that the first layer doesn't find the count uninitialized +- *pToolCount = 0; +- return VK_SUCCESS; ++ if (!tooling_info_supported || !icd_term->dispatch.GetPhysicalDeviceToolPropertiesEXT) { ++ *pToolCount = 0; ++ } ++ ++ if (ext_props) { ++ loader_instance_heap_free(icd_term->this_instance, ext_props); ++ } ++ ++ return res; + } +diff --git a/tests/loader_regression_tests.cpp b/tests/loader_regression_tests.cpp +index d1687a4f5..469aa8925 100644 +--- a/tests/loader_regression_tests.cpp ++++ b/tests/loader_regression_tests.cpp +@@ -849,7 +849,24 @@ TEST(ExtensionManual, ToolingProperties) { + VK_TOOL_PURPOSE_VALIDATION_BIT_EXT, + "This tool does not exist", + "No-Layer"}; +- { // extension ++ { // No support in driver ++ SingleICDShim env{TestICDDetails{TEST_ICD_PATH_VERSION_6}}; ++ env.get_test_icd().physical_devices.push_back({}); ++ ++ InstWrapper inst{env.vulkan_functions}; ++ inst.CheckCreate(); ++ ++ auto phys_dev = inst.GetPhysDev(); ++ ++ auto getToolProperties = reinterpret_cast( ++ inst.functions->vkGetInstanceProcAddr(inst, "vkGetPhysicalDeviceToolPropertiesEXT")); ++ handle_assert_has_value(getToolProperties); ++ ++ uint32_t tool_count = 0; ++ ASSERT_EQ(VK_SUCCESS, getToolProperties(phys_dev, &tool_count, nullptr)); ++ ASSERT_EQ(tool_count, 0); ++ } ++ { // extension is supported in driver + SingleICDShim env{TestICDDetails{TEST_ICD_PATH_VERSION_6}}; + env.get_test_icd().physical_devices.push_back({}); + env.get_test_icd().supports_tooling_info_ext = true; +@@ -864,7 +881,7 @@ TEST(ExtensionManual, ToolingProperties) { + auto getToolProperties = reinterpret_cast( + inst.functions->vkGetInstanceProcAddr(inst, "vkGetPhysicalDeviceToolPropertiesEXT")); + handle_assert_has_value(getToolProperties); +- uint32_t tool_count = 1; ++ uint32_t tool_count = 0; + ASSERT_EQ(VK_SUCCESS, getToolProperties(phys_dev, &tool_count, nullptr)); + ASSERT_EQ(tool_count, 1); + VkPhysicalDeviceToolPropertiesEXT props{}; +-- +2.34.1 + diff --git a/trunk/PKGBUILD b/trunk/PKGBUILD index 2cf8df5..a35447a 100644 --- a/trunk/PKGBUILD +++ b/trunk/PKGBUILD @@ -2,7 +2,7 @@ pkgname=vulkan-icd-loader pkgver=1.2.199 -pkgrel=1 +pkgrel=2 arch=(x86_64) pkgdesc="Vulkan Installable Client Driver (ICD) Loader" url="https://www.khronos.org/vulkan/" @@ -11,8 +11,17 @@ makedepends=('cmake' 'python-lxml' 'libx11' 'libxrandr' 'wayland' 'vulkan-header depends=(glibc) optdepends=('vulkan-driver: packaged vulkan driver') # vulkan-driver: vulkan-intel/vulkan-radeon/nvidia-utils/.... provides=('libvulkan.so') -source=("${pkgname}-${pkgver}.tar.gz::https://github.com/KhronosGroup/Vulkan-Loader/archive/v${pkgver}.tar.gz") -sha256sums=('cc496f6725c7e088510d1a5e7c6a97b61e356b147dcc3d697233ca775dd768ef') +source=("${pkgname}-${pkgver}.tar.gz::https://github.com/KhronosGroup/Vulkan-Loader/archive/v${pkgver}.tar.gz" + 0001-Dont-call-ToolProps-on-drivers-without-support.patch) +sha256sums=('cc496f6725c7e088510d1a5e7c6a97b61e356b147dcc3d697233ca775dd768ef' + 'a29f06969f43bbf315ab766a212d88eccbd9921edf270ec187dd2c1d436cce12') + +prepare() { + cd "${srcdir}"/Vulkan-Loader* + + # fix segfault with mesa drivers + patch -Np1 -i ../0001-Dont-call-ToolProps-on-drivers-without-support.patch +} build() { cd "${srcdir}"/Vulkan-Loader* diff --git a/x86_64/extra/0001-Dont-call-ToolProps-on-drivers-without-support.patch b/x86_64/extra/0001-Dont-call-ToolProps-on-drivers-without-support.patch new file mode 100644 index 0000000..c91e0c8 --- /dev/null +++ b/x86_64/extra/0001-Dont-call-ToolProps-on-drivers-without-support.patch @@ -0,0 +1,126 @@ +From 22e9c7c538a2d2cfe2a81cb4fee6a710798b8f11 Mon Sep 17 00:00:00 2001 +From: Charles Giessen +Date: Wed, 17 Nov 2021 14:44:11 -0700 +Subject: [PATCH] Dont call ToolProps on drivers without support + +The loader recently added support for calling into drivers in +vkGetPhysicalDeviceToolPropertieesEXT. However, it only used the value of +the function pointer to determine if it was safe. It was found that Mesa +drivers will return a non-null function pointer, even though they do not +support the extension, and so the loader called this function pointer which +would then segfault. + +The loader prevents this by first checking if the extension is supported by +the physical device. This necessitates calling +vkEnumerateDeviceExtensionProperties and allocating some memory on each call +but since the number of times this function is called should be low, it is +not an undue performance burden. In the future, the loader should cache the +list of extensions when calling vkEnumeratePhysicalDevices so that checking if +a extension function is supported is fast and easy. + +Signed-off-by: Laurent Carlier +--- + loader/extension_manual.c | 46 ++++++++++++++++++++++++++++--- + tests/loader_regression_tests.cpp | 21 ++++++++++++-- + 2 files changed, 61 insertions(+), 6 deletions(-) + +diff --git a/loader/extension_manual.c b/loader/extension_manual.c +index 415b64b74..65c573d91 100644 +--- a/loader/extension_manual.c ++++ b/loader/extension_manual.c +@@ -305,11 +305,49 @@ VKAPI_ATTR VkResult VKAPI_CALL terminator_GetPhysicalDeviceToolPropertiesEXT(VkP + struct loader_physical_device_term *phys_dev_term = (struct loader_physical_device_term *)physicalDevice; + struct loader_icd_term *icd_term = phys_dev_term->this_icd_term; + +- if (icd_term->dispatch.GetPhysicalDeviceToolPropertiesEXT) { +- return icd_term->dispatch.GetPhysicalDeviceToolPropertiesEXT(phys_dev_term->phys_dev, pToolCount, pToolProperties); ++ bool tooling_info_supported = false; ++ uint32_t ext_count = 0; ++ VkExtensionProperties *ext_props = NULL; ++ VkResult res = VK_SUCCESS; ++ VkResult enumerate_res = VK_SUCCESS; ++ ++ enumerate_res = icd_term->dispatch.EnumerateDeviceExtensionProperties(phys_dev_term->phys_dev, NULL, &ext_count, NULL); ++ if (enumerate_res != VK_SUCCESS) { ++ goto out; + } + ++ ext_props = loader_instance_heap_alloc(icd_term->this_instance, sizeof(VkExtensionProperties) * ext_count, ++ VK_SYSTEM_ALLOCATION_SCOPE_COMMAND); ++ if (!ext_props) { ++ res = VK_ERROR_OUT_OF_HOST_MEMORY; ++ goto out; ++ } ++ ++ enumerate_res = icd_term->dispatch.EnumerateDeviceExtensionProperties(phys_dev_term->phys_dev, NULL, &ext_count, ext_props); ++ if (enumerate_res != VK_SUCCESS) { ++ goto out; ++ } ++ ++ for (uint32_t i = 0; i < ext_count; i++) { ++ if (strncmp(ext_props[i].extensionName, VK_EXT_TOOLING_INFO_EXTENSION_NAME, VK_MAX_EXTENSION_NAME_SIZE) == 0) { ++ tooling_info_supported = true; ++ break; ++ } ++ } ++ ++ if (tooling_info_supported && icd_term->dispatch.GetPhysicalDeviceToolPropertiesEXT) { ++ res = icd_term->dispatch.GetPhysicalDeviceToolPropertiesEXT(phys_dev_term->phys_dev, pToolCount, pToolProperties); ++ } ++ ++out: + // In the case the driver didn't support the extension, make sure that the first layer doesn't find the count uninitialized +- *pToolCount = 0; +- return VK_SUCCESS; ++ if (!tooling_info_supported || !icd_term->dispatch.GetPhysicalDeviceToolPropertiesEXT) { ++ *pToolCount = 0; ++ } ++ ++ if (ext_props) { ++ loader_instance_heap_free(icd_term->this_instance, ext_props); ++ } ++ ++ return res; + } +diff --git a/tests/loader_regression_tests.cpp b/tests/loader_regression_tests.cpp +index d1687a4f5..469aa8925 100644 +--- a/tests/loader_regression_tests.cpp ++++ b/tests/loader_regression_tests.cpp +@@ -849,7 +849,24 @@ TEST(ExtensionManual, ToolingProperties) { + VK_TOOL_PURPOSE_VALIDATION_BIT_EXT, + "This tool does not exist", + "No-Layer"}; +- { // extension ++ { // No support in driver ++ SingleICDShim env{TestICDDetails{TEST_ICD_PATH_VERSION_6}}; ++ env.get_test_icd().physical_devices.push_back({}); ++ ++ InstWrapper inst{env.vulkan_functions}; ++ inst.CheckCreate(); ++ ++ auto phys_dev = inst.GetPhysDev(); ++ ++ auto getToolProperties = reinterpret_cast( ++ inst.functions->vkGetInstanceProcAddr(inst, "vkGetPhysicalDeviceToolPropertiesEXT")); ++ handle_assert_has_value(getToolProperties); ++ ++ uint32_t tool_count = 0; ++ ASSERT_EQ(VK_SUCCESS, getToolProperties(phys_dev, &tool_count, nullptr)); ++ ASSERT_EQ(tool_count, 0); ++ } ++ { // extension is supported in driver + SingleICDShim env{TestICDDetails{TEST_ICD_PATH_VERSION_6}}; + env.get_test_icd().physical_devices.push_back({}); + env.get_test_icd().supports_tooling_info_ext = true; +@@ -864,7 +881,7 @@ TEST(ExtensionManual, ToolingProperties) { + auto getToolProperties = reinterpret_cast( + inst.functions->vkGetInstanceProcAddr(inst, "vkGetPhysicalDeviceToolPropertiesEXT")); + handle_assert_has_value(getToolProperties); +- uint32_t tool_count = 1; ++ uint32_t tool_count = 0; + ASSERT_EQ(VK_SUCCESS, getToolProperties(phys_dev, &tool_count, nullptr)); + ASSERT_EQ(tool_count, 1); + VkPhysicalDeviceToolPropertiesEXT props{}; +-- +2.34.1 + diff --git a/x86_64/extra/PKGBUILD b/x86_64/extra/PKGBUILD index 2cf8df5..a35447a 100644 --- a/x86_64/extra/PKGBUILD +++ b/x86_64/extra/PKGBUILD @@ -2,7 +2,7 @@ pkgname=vulkan-icd-loader pkgver=1.2.199 -pkgrel=1 +pkgrel=2 arch=(x86_64) pkgdesc="Vulkan Installable Client Driver (ICD) Loader" url="https://www.khronos.org/vulkan/" @@ -11,8 +11,17 @@ makedepends=('cmake' 'python-lxml' 'libx11' 'libxrandr' 'wayland' 'vulkan-header depends=(glibc) optdepends=('vulkan-driver: packaged vulkan driver') # vulkan-driver: vulkan-intel/vulkan-radeon/nvidia-utils/.... provides=('libvulkan.so') -source=("${pkgname}-${pkgver}.tar.gz::https://github.com/KhronosGroup/Vulkan-Loader/archive/v${pkgver}.tar.gz") -sha256sums=('cc496f6725c7e088510d1a5e7c6a97b61e356b147dcc3d697233ca775dd768ef') +source=("${pkgname}-${pkgver}.tar.gz::https://github.com/KhronosGroup/Vulkan-Loader/archive/v${pkgver}.tar.gz" + 0001-Dont-call-ToolProps-on-drivers-without-support.patch) +sha256sums=('cc496f6725c7e088510d1a5e7c6a97b61e356b147dcc3d697233ca775dd768ef' + 'a29f06969f43bbf315ab766a212d88eccbd9921edf270ec187dd2c1d436cce12') + +prepare() { + cd "${srcdir}"/Vulkan-Loader* + + # fix segfault with mesa drivers + patch -Np1 -i ../0001-Dont-call-ToolProps-on-drivers-without-support.patch +} build() { cd "${srcdir}"/Vulkan-Loader*