Skip to content

Commit 23440e2

Browse files
fix(core): validate start + size for buffer ops in more places
1 parent 7c74c1d commit 23440e2

3 files changed

Lines changed: 48 additions & 8 deletions

File tree

wgpu-core/src/command/transfer.rs

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -982,7 +982,14 @@ pub(super) fn copy_buffer_to_buffer(
982982
let dst_barrier = dst_pending.map(|pending| pending.into_hal(dst_buffer, state.snatch_guard));
983983

984984
let (size, source_end_offset) = match size {
985-
Some(size) => (size, source_offset + size),
985+
Some(size) => {
986+
if let Some(end) = source_offset.checked_add(size) {
987+
(size, end)
988+
} else {
989+
// TODO: Is this the right error to report?
990+
return Err(TransferError::SizeOverflow.into());
991+
}
992+
}
986993
None => (src_buffer.size - source_offset, src_buffer.size),
987994
};
988995

wgpu-core/src/device/queue.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -660,10 +660,13 @@ impl Queue {
660660
if !buffer_offset.is_multiple_of(wgt::COPY_BUFFER_ALIGNMENT) {
661661
return Err(TransferError::UnalignedBufferOffset(buffer_offset));
662662
}
663-
if buffer_offset + buffer_size.get() > buffer.size {
663+
let end_offset = buffer_offset
664+
.checked_add(buffer_size.get())
665+
.ok_or(TransferError::SizeOverflow)?;
666+
if end_offset > buffer.size {
664667
return Err(TransferError::BufferOverrun {
665668
start_offset: buffer_offset,
666-
end_offset: buffer_offset + buffer_size.get(),
669+
end_offset,
667670
buffer_size: buffer.size,
668671
side: CopySide::Destination,
669672
});
@@ -1609,10 +1612,14 @@ impl Global {
16091612
let queue = self.hub.queues.get(queue_id);
16101613
let buffer = self.hub.buffers.get(buffer_id).get()?;
16111614

1615+
let end_offset = buffer_offset
1616+
.checked_add(data.len() as u64)
1617+
.ok_or(TransferError::SizeOverflow)?;
1618+
16121619
#[cfg(feature = "trace")]
16131620
if let Some(ref mut trace) = *queue.device.trace.lock() {
16141621
use crate::device::trace::DataKind;
1615-
let range = buffer_offset..buffer_offset + data.len() as u64;
1622+
let range = buffer_offset..end_offset;
16161623
let data = trace.make_binary(DataKind::Bin, data);
16171624
trace.add(Action::WriteBuffer {
16181625
id: buffer.to_trace(),

wgpu-core/src/resource.rs

Lines changed: 30 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -301,6 +301,13 @@ pub enum BufferAccessError {
301301
MapAborted,
302302
#[error(transparent)]
303303
InvalidResource(#[from] InvalidResourceError),
304+
#[error(
305+
"Map end offset (start at {offset} + size of {size}) overflows unsigned 64-bit integer"
306+
)]
307+
MapEndOffsetOverflows {
308+
offset: wgt::BufferAddress,
309+
size: wgt::BufferAddress,
310+
},
304311
}
305312

306313
impl WebGpuError for BufferAccessError {
@@ -321,7 +328,8 @@ impl WebGpuError for BufferAccessError {
321328
| Self::OutOfBoundsUnderrun { .. }
322329
| Self::OutOfBoundsOverrun { .. }
323330
| Self::NegativeRange { .. }
324-
| Self::MapAborted => return ErrorType::Validation,
331+
| Self::MapAborted
332+
| Self::MapEndOffsetOverflows { .. } => return ErrorType::Validation,
325333
};
326334
e.webgpu_error_type()
327335
}
@@ -592,7 +600,19 @@ impl Buffer {
592600
return Err((op, BufferAccessError::UnalignedRangeSize { range_size }));
593601
}
594602

595-
let range = offset..(offset + range_size);
603+
let end_offset = match offset.checked_add(range_size) {
604+
Some(some) => some,
605+
None => {
606+
return Err((
607+
op,
608+
BufferAccessError::MapEndOffsetOverflows {
609+
offset,
610+
size: range_size,
611+
},
612+
))
613+
}
614+
};
615+
let range = offset..end_offset;
596616

597617
if range.start % wgt::MAP_ALIGNMENT != 0 || range.end % wgt::COPY_BUFFER_ALIGNMENT != 0 {
598618
return Err((op, BufferAccessError::UnalignedRange));
@@ -702,10 +722,16 @@ impl Buffer {
702722
let map_state = &*self.map_state.lock();
703723
match *map_state {
704724
BufferMapState::Init { ref staging_buffer } => {
725+
let mapping_end_offset = offset.checked_add(range_size).ok_or(
726+
BufferAccessError::MapEndOffsetOverflows {
727+
offset,
728+
size: range_size,
729+
},
730+
)?;
705731
// offset (u64) can not be < 0, so no need to validate the lower bound
706-
if offset + range_size > self.size {
732+
if mapping_end_offset > self.size {
707733
return Err(BufferAccessError::OutOfBoundsOverrun {
708-
index: offset + range_size - 1,
734+
index: mapping_end_offset.checked_sub(1).unwrap(),
709735
max: self.size,
710736
});
711737
}

0 commit comments

Comments
 (0)