From 8f223898b98da836b19074a0b37f570850e6696d Mon Sep 17 00:00:00 2001
From: Marijn Suijten <marijn@traverseresearch.nl>
Date: Tue, 22 Nov 2022 01:07:49 +0100
Subject: [PATCH] Call `Vec::set_len()` after checking for Vulkan errors

The entire reason for calling `unsafe` `set_len()` after the Vulkan
driver function call is to ensure the `Vec` never gives safe access to
uninitialized values (as allocted via `Vec::with_capacity()`).  This
contract is broken within the implementation of these functions by
temporarily setting a nonzero length when the Vulkan driver may not have
initialized the underlying data at all, and communicated this by
returning an error code.

Simply check the error code first, before jumping to a now-infallible
codepath that calls `.set_len()` and always returns `Ok()`.
---
 ash/src/device.rs                             | 14 +++++++------
 ash/src/extensions/khr/display_swapchain.rs   |  7 ++++---
 .../extensions/khr/ray_tracing_pipeline.rs    | 20 ++++++++++---------
 ash/src/prelude.rs                            |  3 ++-
 4 files changed, 25 insertions(+), 19 deletions(-)

diff --git a/ash/src/device.rs b/ash/src/device.rs
index aecd13c0d..08f103186 100644
--- a/ash/src/device.rs
+++ b/ash/src/device.rs
@@ -1502,14 +1502,15 @@ impl Device {
         create_info: &vk::DescriptorSetAllocateInfo,
     ) -> VkResult<Vec<vk::DescriptorSet>> {
         let mut desc_set = Vec::with_capacity(create_info.descriptor_set_count as usize);
-        let err_code = (self.device_fn_1_0.allocate_descriptor_sets)(
+        (self.device_fn_1_0.allocate_descriptor_sets)(
             self.handle(),
             create_info,
             desc_set.as_mut_ptr(),
-        );
+        )
+        .result()?;
 
         desc_set.set_len(create_info.descriptor_set_count as usize);
-        err_code.result_with_success(desc_set)
+        Ok(desc_set)
     }
 
     /// <https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/vkCreateDescriptorSetLayout.html>
@@ -2484,13 +2485,14 @@ impl Device {
         create_info: &vk::CommandBufferAllocateInfo,
     ) -> VkResult<Vec<vk::CommandBuffer>> {
         let mut buffers = Vec::with_capacity(create_info.command_buffer_count as usize);
-        let err_code = (self.device_fn_1_0.allocate_command_buffers)(
+        (self.device_fn_1_0.allocate_command_buffers)(
             self.handle(),
             create_info,
             buffers.as_mut_ptr(),
-        );
+        )
+        .result()?;
         buffers.set_len(create_info.command_buffer_count as usize);
-        err_code.result_with_success(buffers)
+        Ok(buffers)
     }
 
     /// <https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/vkCreateCommandPool.html>
diff --git a/ash/src/extensions/khr/display_swapchain.rs b/ash/src/extensions/khr/display_swapchain.rs
index 649851ba1..e4a33b2bd 100755
--- a/ash/src/extensions/khr/display_swapchain.rs
+++ b/ash/src/extensions/khr/display_swapchain.rs
@@ -28,15 +28,16 @@ impl DisplaySwapchain {
         allocation_callbacks: Option<&vk::AllocationCallbacks>,
     ) -> VkResult<Vec<vk::SwapchainKHR>> {
         let mut swapchains = Vec::with_capacity(create_infos.len());
-        let err_code = (self.fp.create_shared_swapchains_khr)(
+        (self.fp.create_shared_swapchains_khr)(
             self.handle,
             create_infos.len() as u32,
             create_infos.as_ptr(),
             allocation_callbacks.as_raw_ptr(),
             swapchains.as_mut_ptr(),
-        );
+        )
+        .result()?;
         swapchains.set_len(create_infos.len());
-        err_code.result_with_success(swapchains)
+        Ok(swapchains)
     }
 
     #[inline]
diff --git a/ash/src/extensions/khr/ray_tracing_pipeline.rs b/ash/src/extensions/khr/ray_tracing_pipeline.rs
index 9e409e336..aba28cf74 100644
--- a/ash/src/extensions/khr/ray_tracing_pipeline.rs
+++ b/ash/src/extensions/khr/ray_tracing_pipeline.rs
@@ -90,16 +90,17 @@ impl RayTracingPipeline {
         data_size: usize,
     ) -> VkResult<Vec<u8>> {
         let mut data = Vec::<u8>::with_capacity(data_size);
-        let err_code = (self.fp.get_ray_tracing_shader_group_handles_khr)(
+        (self.fp.get_ray_tracing_shader_group_handles_khr)(
             self.handle,
             pipeline,
             first_group,
             group_count,
             data_size,
-            data.as_mut_ptr() as *mut std::ffi::c_void,
-        );
+            data.as_mut_ptr().cast(),
+        )
+        .result()?;
         data.set_len(data_size);
-        err_code.result_with_success(data)
+        Ok(data)
     }
 
     /// <https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/vkGetRayTracingCaptureReplayShaderGroupHandlesKHR.html>
@@ -111,8 +112,8 @@ impl RayTracingPipeline {
         group_count: u32,
         data_size: usize,
     ) -> VkResult<Vec<u8>> {
-        let mut data: Vec<u8> = Vec::with_capacity(data_size);
-        let err_code = (self
+        let mut data = Vec::<u8>::with_capacity(data_size);
+        (self
             .fp
             .get_ray_tracing_capture_replay_shader_group_handles_khr)(
             self.handle,
@@ -120,10 +121,11 @@ impl RayTracingPipeline {
             first_group,
             group_count,
             data_size,
-            data.as_mut_ptr() as *mut _,
-        );
+            data.as_mut_ptr().cast(),
+        )
+        .result()?;
         data.set_len(data_size);
-        err_code.result_with_success(data)
+        Ok(data)
     }
 
     /// <https://www.khronos.org/registry/vulkan/specs/1.3-extensions/man/html/vkCmdTraceRaysIndirectKHR.html>
diff --git a/ash/src/prelude.rs b/ash/src/prelude.rs
index c0b548c9e..321c3aadd 100644
--- a/ash/src/prelude.rs
+++ b/ash/src/prelude.rs
@@ -49,8 +49,9 @@ where
 
         let err_code = f(&mut count, data.as_mut_ptr());
         if err_code != vk::Result::INCOMPLETE {
+            err_code.result()?;
             data.set_len(count.try_into().expect("`N` failed to convert to `usize`"));
-            break err_code.result_with_success(data);
+            break Ok(data);
         }
     }
 }