From 0bdd7a2a789c71dfa40995ff82f4d5f0001f1938 Mon Sep 17 00:00:00 2001 From: Stanislav Denisov Date: Wed, 22 Oct 2025 13:15:21 +0200 Subject: [PATCH] Fix out of bounds crash in SRV and UAV subresource vectors (#1250) * Fix out of bounds crash in SRV and UAV subresource vectors * Remove redundant type casting --- WickedEngine/wiGraphicsDevice_DX12.cpp | 16 +++++--- WickedEngine/wiGraphicsDevice_Vulkan.cpp | 48 ++++++++++++++---------- 2 files changed, 38 insertions(+), 26 deletions(-) diff --git a/WickedEngine/wiGraphicsDevice_DX12.cpp b/WickedEngine/wiGraphicsDevice_DX12.cpp index afe729846..192ed9bc4 100644 --- a/WickedEngine/wiGraphicsDevice_DX12.cpp +++ b/WickedEngine/wiGraphicsDevice_DX12.cpp @@ -1009,7 +1009,7 @@ namespace dx12_internal constexpr TextureDesc _ConvertTextureDesc_Inv(const D3D12_RESOURCE_DESC& desc) { TextureDesc retVal; - + switch (desc.Dimension) { case D3D12_RESOURCE_DIMENSION_TEXTURE1D: @@ -4311,7 +4311,7 @@ std::mutex queue_locker; { internal_state->desc.Flags |= D3D12_RAYTRACING_ACCELERATION_STRUCTURE_BUILD_FLAG_MINIMIZE_MEMORY; } - + switch (desc->type) { @@ -4345,7 +4345,7 @@ std::mutex queue_locker; } else if (x.type == RaytracingAccelerationStructureDesc::BottomLevel::Geometry::Type::PROCEDURAL_AABBS) { - geometry.Type = D3D12_RAYTRACING_GEOMETRY_TYPE_PROCEDURAL_PRIMITIVE_AABBS; + geometry.Type = D3D12_RAYTRACING_GEOMETRY_TYPE_PROCEDURAL_PRIMITIVE_AABBS; geometry.AABBs.AABBs.StartAddress = to_internal(&x.aabbs.aabb_buffer)->gpu_address + (D3D12_GPU_VIRTUAL_ADDRESS)x.aabbs.offset; geometry.AABBs.AABBs.StrideInBytes = (UINT64)x.aabbs.stride; @@ -5134,7 +5134,7 @@ std::mutex queue_locker; if (resource == nullptr || !resource->IsValid()) return -1; - auto internal_state = to_internal(resource); + const auto internal_state = to_internal(resource); switch (type) { @@ -5146,6 +5146,8 @@ std::mutex queue_locker; } else { + if (subresource >= (int)internal_state->subresources_srv.size()) + return -1; return internal_state->subresources_srv[subresource].index; } break; @@ -5156,6 +5158,8 @@ std::mutex queue_locker; } else { + if (subresource >= (int)internal_state->subresources_uav.size()) + return -1; return internal_state->subresources_uav[subresource].index; } break; @@ -5200,7 +5204,7 @@ std::mutex queue_locker; const void* identifier = internal_state->stateObjectProperties->GetShaderIdentifier(internal_state->group_strings[group_index].c_str()); std::memcpy(dest, identifier, D3D12_SHADER_IDENTIFIER_SIZE_IN_BYTES); } - + void GraphicsDevice_DX12::SetName(GPUResource* pResource, const char* name) const { wchar_t text[256]; @@ -7241,7 +7245,7 @@ std::mutex queue_locker; if (x.type == RaytracingAccelerationStructureDesc::BottomLevel::Geometry::Type::TRIANGLES) { - geometry.Triangles.VertexBuffer.StartAddress = to_internal(&x.triangles.vertex_buffer)->gpu_address + + geometry.Triangles.VertexBuffer.StartAddress = to_internal(&x.triangles.vertex_buffer)->gpu_address + (D3D12_GPU_VIRTUAL_ADDRESS)x.triangles.vertex_byte_offset; geometry.Triangles.IndexBuffer = to_internal(&x.triangles.index_buffer)->gpu_address + (D3D12_GPU_VIRTUAL_ADDRESS)x.triangles.index_offset * (x.triangles.index_format == IndexBufferFormat::UINT16 ? sizeof(uint16_t) : sizeof(uint32_t)); diff --git a/WickedEngine/wiGraphicsDevice_Vulkan.cpp b/WickedEngine/wiGraphicsDevice_Vulkan.cpp index bddaca96b..a68fca30d 100644 --- a/WickedEngine/wiGraphicsDevice_Vulkan.cpp +++ b/WickedEngine/wiGraphicsDevice_Vulkan.cpp @@ -617,7 +617,7 @@ namespace vulkan_internal } VKAPI_ATTR VkBool32 VKAPI_CALL debugUtilsMessengerCallback( - VkDebugUtilsMessageSeverityFlagBitsEXT message_severity, + VkDebugUtilsMessageSeverityFlagBitsEXT message_severity, VkDebugUtilsMessageTypeFlagsEXT message_type, const VkDebugUtilsMessengerCallbackDataEXT* callback_data, void* user_data) @@ -2412,7 +2412,7 @@ using namespace vulkan_internal; } } #endif // _WIN32 - + if (validationMode != ValidationMode::Disabled) { // Determine the optimal validation layers to enable that are necessary for useful debugging @@ -2954,7 +2954,7 @@ using namespace vulkan_internal; queueFamiliesVideo[i].sType = VK_STRUCTURE_TYPE_QUEUE_FAMILY_VIDEO_PROPERTIES_KHR; } vkGetPhysicalDeviceQueueFamilyProperties2(physicalDevice, &queueFamilyCount, queueFamilies.data()); - + // Query base queue families: for (uint32_t i = 0; i < queueFamilyCount; ++i) { @@ -3286,7 +3286,7 @@ using namespace vulkan_internal; allocInfo.preferredFlags = VK_MEMORY_PROPERTY_DEVICE_LOCAL_BIT; vulkan_check(vmaCreateBuffer(allocationhandler->allocator, &bufferInfo, &allocInfo, &nullBuffer, &nullBufferAllocation, nullptr)); - + VkBufferViewCreateInfo viewInfo = {}; viewInfo.sType = VK_STRUCTURE_TYPE_BUFFER_VIEW_CREATE_INFO; viewInfo.format = VK_FORMAT_R32G32B32A32_SFLOAT; @@ -3739,14 +3739,14 @@ using namespace vulkan_internal; size_t size{}; vulkan_check(vkGetPipelineCacheData(device, pipelineCache, &size, nullptr)); - // Get data of pipeline cache + // Get data of pipeline cache wi::vector data(size); vulkan_check(vkGetPipelineCacheData(device, pipelineCache, &size, data.data())); // Write pipeline cache data to a file in binary format wi::helper::FileWrite(get_shader_cache_path(), data.data(), size); - // Destroy Vulkan pipeline cache + // Destroy Vulkan pipeline cache vkDestroyPipelineCache(device, pipelineCache, nullptr); pipelineCache = VK_NULL_HANDLE; } @@ -4131,7 +4131,7 @@ using namespace vulkan_internal; 1, ©Region ); - + copyAllocator.submit(cmd); } } @@ -5511,7 +5511,7 @@ using namespace vulkan_internal; default: break; } - + } VkPipelineLayoutCreateInfo pipelineLayoutInfo = {}; @@ -6978,56 +6978,64 @@ using namespace vulkan_internal; case SubresourceType::SRV: if (resource->IsBuffer()) { - auto internal_state = to_internal((const GPUBuffer*)resource); + const auto internal_state = to_internal((const GPUBuffer*)resource); if (subresource < 0) { return internal_state->srv.index; } else { + if (subresource >= (int)internal_state->subresources_srv.size()) + return -1; return internal_state->subresources_srv[subresource].index; } } - else if(resource->IsTexture()) + else if (resource->IsTexture()) { - auto internal_state = to_internal((const Texture*)resource); + const auto internal_state = to_internal((const Texture*)resource); if (subresource < 0) { return internal_state->srv.index; } else { + if (subresource >= (int)internal_state->subresources_srv.size()) + return -1; return internal_state->subresources_srv[subresource].index; } } else if (resource->IsAccelerationStructure()) { - auto internal_state = to_internal((const RaytracingAccelerationStructure*)resource); + const auto internal_state = to_internal((const RaytracingAccelerationStructure*)resource); return internal_state->index; } break; case SubresourceType::UAV: if (resource->IsBuffer()) { - auto internal_state = to_internal((const GPUBuffer*)resource); + const auto internal_state = to_internal((const GPUBuffer*)resource); if (subresource < 0) { return internal_state->uav.index; } else { + if (subresource >= (int)internal_state->subresources_uav.size()) + return -1; return internal_state->subresources_uav[subresource].index; } } else if (resource->IsTexture()) { - auto internal_state = to_internal((const Texture*)resource); + const auto internal_state = to_internal((const Texture*)resource); if (subresource < 0) { return internal_state->uav.index; } else { + if (subresource >= (int)internal_state->subresources_uav.size()) + return -1; return internal_state->subresources_uav[subresource].index; } } @@ -7096,7 +7104,7 @@ using namespace vulkan_internal; { vulkan_check(vkGetRayTracingShaderGroupHandlesKHR(device, to_internal(rtpso)->pipeline, group_index, 1, SHADER_IDENTIFIER_SIZE, dest)); } - + void GraphicsDevice_Vulkan::SetName(GPUResource* pResource, const char* name) const { if (!debugUtils || pResource == nullptr || !pResource->IsValid()) @@ -7839,7 +7847,7 @@ using namespace vulkan_internal; assert(0); } commandlist.prev_swapchains.push_back(*swapchain); - + VkRenderingInfo info = {}; info.sType = VK_STRUCTURE_TYPE_RENDERING_INFO; info.renderArea.offset.x = 0; @@ -8078,7 +8086,7 @@ using namespace vulkan_internal; barrier.newLayout = _ConvertImageLayout(image.layout); assert(barrier.newLayout != VK_IMAGE_LAYOUT_UNDEFINED); - + barrier.srcStageMask = _ConvertPipelineStage(image.layout_before); barrier.dstStageMask = _ConvertPipelineStage(image.layout); barrier.srcAccessMask = _ParseResourceState(image.layout_before); @@ -9232,7 +9240,7 @@ using namespace vulkan_internal; raygen.deviceAddress += desc->ray_generation.offset; raygen.size = desc->ray_generation.size; raygen.stride = raygen.size; // raygen specifically must be size == stride - + VkStridedDeviceAddressRegionKHR miss = {}; miss.deviceAddress = desc->miss.buffer ? to_internal(desc->miss.buffer)->address : 0; miss.deviceAddress += desc->miss.offset; @@ -9257,8 +9265,8 @@ using namespace vulkan_internal; &miss, &hitgroup, &callable, - desc->width, - desc->height, + desc->width, + desc->height, desc->depth ); }