From f25e07b984ab391628d9568296d5970981d79d8b Mon Sep 17 00:00:00 2001 From: Nicolas Silva Date: Mon, 1 Jul 2024 18:12:50 +0200 Subject: [PATCH] Fix soundness issue with Snatchable 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! --- wgpu-core/src/device/queue.rs | 32 +++++++++++++++++--------------- wgpu-core/src/resource.rs | 10 ++++------ wgpu-core/src/snatch.rs | 4 ++-- 3 files changed, 23 insertions(+), 23 deletions(-) diff --git a/wgpu-core/src/device/queue.rs b/wgpu-core/src/device/queue.rs index 59490de92..7ded88ea0 100644 --- a/wgpu-core/src/device/queue.rs +++ b/wgpu-core/src/device/queue.rs @@ -1472,23 +1472,25 @@ impl Global { ) .collect::>(); - let mut submit_surface_textures = - SmallVec::<[_; 2]>::with_capacity(submit_surface_textures_owned.len()); + { + 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, - }); - } + 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"); diff --git a/wgpu-core/src/resource.rs b/wgpu-core/src/resource.rs index b420234d9..829762eb2 100644 --- a/wgpu-core/src/resource.rs +++ b/wgpu-core/src/resource.rs @@ -467,7 +467,7 @@ impl Drop for Buffer { } impl Buffer { - 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) } @@ -1054,7 +1054,7 @@ impl Texture { pub(crate) fn inner_mut<'a>( &'a self, - guard: &mut ExclusiveSnatchGuard, + guard: &'a mut ExclusiveSnatchGuard, ) -> Option<&'a mut TextureInner> { self.inner.get_mut(guard) } @@ -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) } diff --git a/wgpu-core/src/snatch.rs b/wgpu-core/src/snatch.rs index 08a1eba11..6f60f45d8 100644 --- a/wgpu-core/src/snatch.rs +++ b/wgpu-core/src/snatch.rs @@ -33,12 +33,12 @@ impl Snatchable { } /// 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() } }