Skip to content

Commit

Permalink
Add bounds check to Buffer slice method
Browse files Browse the repository at this point in the history
  • Loading branch information
beholdnec committed Oct 21, 2024
1 parent e06f10e commit 0dc91ab
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 1 deletion.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -160,6 +160,7 @@ By @bradwerth [#6216](https://github.com/gfx-rs/wgpu/pull/6216).
- `Rg11b10Float` is renamed to `Rg11b10Ufloat`. By @sagudev in [#6108](https://github.com/gfx-rs/wgpu/pull/6108)
- Invalidate the device when we encounter driver-induced device loss or on unexpected errors. By @teoxoy in [#6229](https://github.com/gfx-rs/wgpu/pull/6229).
- Make Vulkan error handling more robust. By @teoxoy in [#6119](https://github.com/gfx-rs/wgpu/pull/6119).
- Add bounds checking to Buffer slice method. By @beholdnec in [#6432](https://github.com/gfx-rs/wgpu/pull/6432).

#### Internal

Expand Down
55 changes: 54 additions & 1 deletion wgpu/src/api/buffer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -242,6 +242,7 @@ impl Buffer {
/// end of the buffer.
pub fn slice<S: RangeBounds<BufferAddress>>(&self, bounds: S) -> BufferSlice<'_> {
let (offset, size) = range_to_offset_size(bounds);
check_buffer_bounds(self.size, offset, size);
BufferSlice {
buffer: self,
offset,
Expand Down Expand Up @@ -673,6 +674,32 @@ impl Drop for Buffer {
}
}

fn check_buffer_bounds(
buffer_size: BufferAddress,
offset: BufferAddress,
size: Option<BufferSize>,
) {
// A slice of length 0 is invalid, so the offset must not be equal to or greater than the buffer size.
if offset >= buffer_size {
panic!(
"slice offset {} is out of range for buffer of size {}",
offset, buffer_size
);
}

if let Some(size) = size {
// Calculate the end carefully to prevent integer overflow. Subtracting 1 is safe
// since BufferSize is a NonZeroU64.
let end = offset.checked_add(size.get() - 1);
if end.is_none() || end.unwrap() >= buffer_size {
panic!(
"slice offset {} size {} is out of range for buffer of size {}",
offset, size, buffer_size
);
}
}
}

fn range_to_offset_size<S: RangeBounds<BufferAddress>>(
bounds: S,
) -> (BufferAddress, Option<BufferSize>) {
Expand All @@ -690,9 +717,10 @@ fn range_to_offset_size<S: RangeBounds<BufferAddress>>(

(offset, size)
}

#[cfg(test)]
mod tests {
use super::{range_to_offset_size, BufferSize};
use super::{check_buffer_bounds, range_to_offset_size, BufferSize};

#[test]
fn range_to_offset_size_works() {
Expand All @@ -715,4 +743,29 @@ mod tests {
fn range_to_offset_size_panics_for_unbounded_empty_range() {
range_to_offset_size(..0);
}

#[test]
#[should_panic]
fn check_buffer_bounds_panics_for_offset_at_size() {
check_buffer_bounds(100, 100, None);
}

#[test]
fn check_buffer_bounds_works_for_end_in_range() {
check_buffer_bounds(200, 100, BufferSize::new(50));
check_buffer_bounds(200, 100, BufferSize::new(100));
check_buffer_bounds(u64::MAX, u64::MAX - 100, BufferSize::new(100));
}

#[test]
#[should_panic]
fn check_buffer_bounds_panics_for_end_over_size() {
check_buffer_bounds(200, 100, BufferSize::new(101));
}

#[test]
#[should_panic]
fn check_buffer_bounds_panics_for_end_wraparound() {
check_buffer_bounds(u64::MAX, u64::MAX - 100, BufferSize::new(101));
}
}

0 comments on commit 0dc91ab

Please sign in to comment.