Eliminate overflow in query set bounds checks (#6933)

This commit is contained in:
Erich Gubler 2025-01-17 11:36:22 -05:00 committed by GitHub
parent 3d13cc1bbe
commit 3b3e1939f9
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 22 additions and 14 deletions

View File

@ -60,6 +60,10 @@ Use `hashbrown` in `wgpu-core`, `wgpu-hal` & `wgpu-info` to simplify no-std supp
By @brodycj in [#6925](https://github.com/gfx-rs/wgpu/pull/6925). By @brodycj in [#6925](https://github.com/gfx-rs/wgpu/pull/6925).
### Bug Fixes
* Avoid overflow in query set bounds check validation. By @ErichDonGubler in [#????].
## v24.0.0 (2025-01-15) ## v24.0.0 (2025-01-15)
### Major changes ### Major changes

View File

@ -145,17 +145,17 @@ pub enum ResolveError {
#[error("Resolving queries {start_query}..{end_query} would overrun the query set of size {query_set_size}")] #[error("Resolving queries {start_query}..{end_query} would overrun the query set of size {query_set_size}")]
QueryOverrun { QueryOverrun {
start_query: u32, start_query: u32,
end_query: u32, end_query: u64,
query_set_size: u32, query_set_size: u32,
}, },
#[error("Resolving queries {start_query}..{end_query} ({stride} byte queries) will end up overrunning the bounds of the destination buffer of size {buffer_size} using offsets {buffer_start_offset}..{buffer_end_offset}")] #[error("Resolving queries {start_query}..{end_query} ({stride} byte queries) will end up overrunning the bounds of the destination buffer of size {buffer_size} using offsets {buffer_start_offset}..(<start> + {bytes_used})")]
BufferOverrun { BufferOverrun {
start_query: u32, start_query: u32,
end_query: u32, end_query: u32,
stride: u32, stride: u32,
buffer_size: BufferAddress, buffer_size: BufferAddress,
buffer_start_offset: BufferAddress, buffer_start_offset: BufferAddress,
buffer_end_offset: BufferAddress, bytes_used: BufferAddress,
}, },
} }
@ -404,8 +404,10 @@ impl Global {
.check_usage(wgt::BufferUsages::QUERY_RESOLVE) .check_usage(wgt::BufferUsages::QUERY_RESOLVE)
.map_err(ResolveError::MissingBufferUsage)?; .map_err(ResolveError::MissingBufferUsage)?;
let end_query = start_query + query_count; let end_query = u64::from(start_query)
if end_query > query_set.desc.count { .checked_add(u64::from(query_count))
.expect("`u64` overflow from adding two `u32`s, should be unreachable");
if end_query > u64::from(query_set.desc.count) {
return Err(ResolveError::QueryOverrun { return Err(ResolveError::QueryOverrun {
start_query, start_query,
end_query, end_query,
@ -413,6 +415,8 @@ impl Global {
} }
.into()); .into());
} }
let end_query = u32::try_from(end_query)
.expect("`u32` overflow for `end_query`, which should be `u32`");
let elements_per_query = match query_set.desc.ty { let elements_per_query = match query_set.desc.ty {
wgt::QueryType::Occlusion => 1, wgt::QueryType::Occlusion => 1,
@ -420,22 +424,22 @@ impl Global {
wgt::QueryType::Timestamp => 1, wgt::QueryType::Timestamp => 1,
}; };
let stride = elements_per_query * wgt::QUERY_SIZE; let stride = elements_per_query * wgt::QUERY_SIZE;
let bytes_used = (stride * query_count) as BufferAddress; let bytes_used: BufferAddress = u64::from(stride)
.checked_mul(u64::from(query_count))
.expect("`stride` * `query_count` overflowed `u32`, should be unreachable");
let buffer_start_offset = destination_offset; let buffer_start_offset = destination_offset;
let buffer_end_offset = buffer_start_offset + bytes_used; let buffer_end_offset = buffer_start_offset
.checked_add(bytes_used)
if buffer_end_offset > dst_buffer.size { .filter(|buffer_end_offset| *buffer_end_offset <= dst_buffer.size)
return Err(ResolveError::BufferOverrun { .ok_or(ResolveError::BufferOverrun {
start_query, start_query,
end_query, end_query,
stride, stride,
buffer_size: dst_buffer.size, buffer_size: dst_buffer.size,
buffer_start_offset, buffer_start_offset,
buffer_end_offset, bytes_used,
} })?;
.into());
}
// TODO(https://github.com/gfx-rs/wgpu/issues/3993): Need to track initialization state. // TODO(https://github.com/gfx-rs/wgpu/issues/3993): Need to track initialization state.
cmd_buf_data.buffer_memory_init_actions.extend( cmd_buf_data.buffer_memory_init_actions.extend(