Skip to content

Commit

Permalink
Fix soundness issue with Snatchable
Browse files Browse the repository at this point in the history
The code was written with the incorrect assumption that if no lifetime is specified in a method definition, then all lifetimes are elided to the lifetime of self. In fact only lifetimes in the returned value are elided to the lifetime of self, and other parameters get their own lifetimes.

Kudos to @teoxoy for catching the issue!
  • Loading branch information
nical authored and teoxoy committed Jul 1, 2024
1 parent 0a76c0f commit f25e07b
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 24 deletions.
34 changes: 18 additions & 16 deletions wgpu-core/src/device/queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1472,23 +1472,25 @@ impl Global {
)
.collect::<Vec<_>>();

let mut submit_surface_textures =
SmallVec::<[_; 2]>::with_capacity(submit_surface_textures_owned.len());

for texture in submit_surface_textures_owned.values() {
submit_surface_textures.extend(match texture.inner.get(&snatch_guard) {
Some(TextureInner::Surface { raw, .. }) => raw.as_ref(),
_ => None,
});
}
{
let mut submit_surface_textures =
SmallVec::<[_; 2]>::with_capacity(submit_surface_textures_owned.len());

for texture in submit_surface_textures_owned.values() {
submit_surface_textures.extend(match texture.inner.get(&snatch_guard) {
Some(TextureInner::Surface { raw, .. }) => raw.as_ref(),
_ => None,
});
}

unsafe {
queue
.raw
.as_ref()
.unwrap()
.submit(&refs, &submit_surface_textures, (fence, submit_index))
.map_err(DeviceError::from)?;
unsafe {
queue
.raw
.as_ref()
.unwrap()
.submit(&refs, &submit_surface_textures, (fence, submit_index))
.map_err(DeviceError::from)?;
}
}

profiling::scope!("cleanup");
Expand Down
10 changes: 4 additions & 6 deletions wgpu-core/src/resource.rs
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ impl<A: HalApi> Drop for Buffer<A> {
}

impl<A: HalApi> Buffer<A> {
pub(crate) fn raw(&self, guard: &SnatchGuard) -> Option<&A::Buffer> {
pub(crate) fn raw<'a>(&'a self, guard: &'a SnatchGuard) -> Option<&'a A::Buffer> {
self.raw.get(guard)
}

Expand Down Expand Up @@ -1054,7 +1054,7 @@ impl<A: HalApi> Texture<A> {

pub(crate) fn inner_mut<'a>(
&'a self,
guard: &mut ExclusiveSnatchGuard,
guard: &'a mut ExclusiveSnatchGuard,
) -> Option<&'a mut TextureInner<A>> {
self.inner.get_mut(guard)
}
Expand Down Expand Up @@ -1153,10 +1153,8 @@ impl Global {
let buffer_opt = { hub.buffers.try_get(id).ok().flatten() };
let buffer = buffer_opt.as_ref().unwrap();

let hal_buffer = {
let snatch_guard = buffer.device.snatchable_lock.read();
buffer.raw(&snatch_guard)
};
let snatch_guard = buffer.device.snatchable_lock.read();
let hal_buffer = buffer.raw(&snatch_guard);

hal_buffer_callback(hal_buffer)
}
Expand Down
4 changes: 2 additions & 2 deletions wgpu-core/src/snatch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,12 +33,12 @@ impl<T> Snatchable<T> {
}

/// Get read access to the value. Requires a the snatchable lock's read guard.
pub fn get(&self, _guard: &SnatchGuard) -> Option<&T> {
pub fn get<'a>(&'a self, _guard: &'a SnatchGuard) -> Option<&'a T> {
unsafe { (*self.value.get()).as_ref() }
}

/// Get write access to the value. Requires a the snatchable lock's write guard.
pub fn get_mut(&self, _guard: &mut ExclusiveSnatchGuard) -> Option<&mut T> {
pub fn get_mut<'a>(&'a self, _guard: &'a mut ExclusiveSnatchGuard) -> Option<&'a mut T> {
unsafe { (*self.value.get()).as_mut() }
}

Expand Down

0 comments on commit f25e07b

Please sign in to comment.