Skip to content

Commit

Permalink
Call Vec::set_len() after checking for Vulkan errors
Browse files Browse the repository at this point in the history
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()`.
  • Loading branch information
MarijnS95 committed Nov 22, 2022
1 parent 29b935b commit 989f684
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 23 deletions.
6 changes: 3 additions & 3 deletions ash-window/src/lib.rs
Expand Up @@ -51,7 +51,7 @@ pub unsafe fn create_surface(

(RawDisplayHandle::Xlib(display), RawWindowHandle::Xlib(window)) => {
let surface_desc = vk::XlibSurfaceCreateInfoKHR::default()
.dpy(display.display as *mut _)
.dpy(display.display.cast())
.window(window.window);
let surface_fn = khr::XlibSurface::new(entry, instance);
surface_fn.create_xlib_surface(&surface_desc, allocation_callbacks)
Expand All @@ -77,7 +77,7 @@ pub unsafe fn create_surface(
use raw_window_metal::{appkit, Layer};

let layer = match appkit::metal_layer_from_handle(window) {
Layer::Existing(layer) | Layer::Allocated(layer) => layer as *mut _,
Layer::Existing(layer) | Layer::Allocated(layer) => layer.cast(),
Layer::None => return Err(vk::Result::ERROR_INITIALIZATION_FAILED),
};

Expand All @@ -91,7 +91,7 @@ pub unsafe fn create_surface(
use raw_window_metal::{uikit, Layer};

let layer = match uikit::metal_layer_from_handle(window) {
Layer::Existing(layer) | Layer::Allocated(layer) => layer as *mut _,
Layer::Existing(layer) | Layer::Allocated(layer) => layer.cast(),
Layer::None => return Err(vk::Result::ERROR_INITIALIZATION_FAILED),
};

Expand Down
16 changes: 9 additions & 7 deletions ash/src/device.rs
Expand Up @@ -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>
Expand Down Expand Up @@ -2026,7 +2027,7 @@ impl Device {
first_query,
data.len() as u32,
data_size,
data.as_mut_ptr() as *mut _,
data.as_mut_ptr().cast(),
mem::size_of::<T>() as _,
flags,
)
Expand Down Expand Up @@ -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>
Expand Down
7 changes: 4 additions & 3 deletions ash/src/extensions/khr/display_swapchain.rs
Expand Up @@ -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]
Expand Down
20 changes: 11 additions & 9 deletions ash/src/extensions/khr/ray_tracing_pipeline.rs
Expand Up @@ -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>
Expand All @@ -111,19 +112,20 @@ 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,
pipeline,
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>
Expand Down
3 changes: 2 additions & 1 deletion ash/src/prelude.rs
Expand Up @@ -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);
}
}
}
Expand Down

0 comments on commit 989f684

Please sign in to comment.