fix: always check buffer clear offset for OOB

CHANGELOG.md

@@ -119,6 +119,7 @@ Bottom level categories:
 - Fix timeout when presenting a surface where no work has been done. By @waywardmonkeys in [#5200](https://github.com/gfx-rs/wgpu/pull/5200)
 - Simplify and speed up the allocation of internal IDs. By @nical in [#5229](https://github.com/gfx-rs/wgpu/pull/5229)
 - Fix an issue where command encoders weren't properly freed if an error occurred during command encoding. By @ErichDonGubler in [#5251](https://github.com/gfx-rs/wgpu/pull/5251).
+- Fix missing validation for `Device::clear_buffer` where `offset + size buffer.size` was not checked when `size` was omitted. By @ErichDonGubler in [#5282](https://github.com/gfx-rs/wgpu/pull/5282).
 
 #### WGL
 

tests/tests/buffer.rs

@@ -326,3 +326,62 @@ static MINIMUM_BUFFER_BINDING_SIZE_DISPATCH: GpuTestConfiguration = GpuTestConfi
             let _ = encoder.finish();
         });
     });
+
+#[gpu_test]
+static CLEAR_OFFSET_OUTSIDE_RESOURCE_BOUNDS: GpuTestConfiguration = GpuTestConfiguration::new()
+    .parameters(TestParameters::default())
+    .run_sync(|ctx| {
+        let size = 16;
+
+        let buffer = ctx.device.create_buffer(&wgpu::BufferDescriptor {
+            label: None,
+            size,
+            usage: wgpu::BufferUsages::COPY_DST,
+            mapped_at_creation: false,
+        });
+
+        let out_of_bounds = size.checked_add(wgpu::COPY_BUFFER_ALIGNMENT).unwrap();
+
+        ctx.device.push_error_scope(wgpu::ErrorFilter::Validation);
+        ctx.device
+            .create_command_encoder(&Default::default())
+            .clear_buffer(&buffer, out_of_bounds, None);
+        let err_msg = pollster::block_on(ctx.device.pop_error_scope())
+            .unwrap()
+            .to_string();
+        assert!(err_msg.contains(
+            "Clear of 20..20 would end up overrunning the bounds of the buffer of size 16"
+        ));
+    });
+
+#[gpu_test]
+static CLEAR_OFFSET_PLUS_SIZE_OUTSIDE_U64_BOUNDS: GpuTestConfiguration =
+    GpuTestConfiguration::new()
+        .parameters(TestParameters::default())
+        .run_sync(|ctx| {
+            let buffer = ctx.device.create_buffer(&wgpu::BufferDescriptor {
+                label: None,
+                size: 16, // unimportant for this test
+                usage: wgpu::BufferUsages::COPY_DST,
+                mapped_at_creation: false,
+            });
+
+            let max_valid_offset = u64::MAX - (u64::MAX % wgpu::COPY_BUFFER_ALIGNMENT);
+            let smallest_aligned_invalid_size = wgpu::COPY_BUFFER_ALIGNMENT;
+
+            ctx.device.push_error_scope(wgpu::ErrorFilter::Validation);
+            ctx.device
+                .create_command_encoder(&Default::default())
+                .clear_buffer(
+                    &buffer,
+                    max_valid_offset,
+                    Some(smallest_aligned_invalid_size),
+                );
+            let err_msg = pollster::block_on(ctx.device.pop_error_scope())
+                .unwrap()
+                .to_string();
+            assert!(err_msg.contains(concat!(
+                "Clear starts at offset 18446744073709551612 with size of 4, ",
+                "but these added together exceed `u64::MAX`"
+            )));
+        });

wgpu-core/src/command/clear.rs

@@ -39,6 +39,11 @@ pub enum ClearError {
     UnalignedFillSize(BufferAddress),
     #[error("Buffer offset {0:?} is not a multiple of `COPY_BUFFER_ALIGNMENT`")]
     UnalignedBufferOffset(BufferAddress),
+    #[error("Clear starts at offset {start_offset} with size of {requested_size}, but these added together exceed `u64::MAX`")]
+    OffsetPlusSizeExceeds64BitBounds {
+        start_offset: BufferAddress,
+        requested_size: BufferAddress,
+    },
     #[error("Clear of {start_offset}..{end_offset} would end up overrunning the bounds of the buffer of size {buffer_size}")]
     BufferOverrun {
         start_offset: BufferAddress,
@@ -117,25 +122,27 @@ impl Global {
         if offset % wgt::COPY_BUFFER_ALIGNMENT != 0 {
             return Err(ClearError::UnalignedBufferOffset(offset));
         }
-        if let Some(size) = size {
-            if size % wgt::COPY_BUFFER_ALIGNMENT != 0 {
-                return Err(ClearError::UnalignedFillSize(size));
-            }
-            let destination_end_offset = offset + size;
-            if destination_end_offset > dst_buffer.size {
-                return Err(ClearError::BufferOverrun {
+
+        let size = size.unwrap_or(dst_buffer.size.saturating_sub(offset));
+        if size % wgt::COPY_BUFFER_ALIGNMENT != 0 {
+            return Err(ClearError::UnalignedFillSize(size));
+        }
+        let end_offset =
+            offset
+                .checked_add(size)
+                .ok_or(ClearError::OffsetPlusSizeExceeds64BitBounds {
                     start_offset: offset,
-                    end_offset: destination_end_offset,
-                    buffer_size: dst_buffer.size,
-                });
-            }
+                    requested_size: size,
+                })?;
+        if end_offset > dst_buffer.size {
+            return Err(ClearError::BufferOverrun {
+                start_offset: offset,
+                end_offset,
+                buffer_size: dst_buffer.size,
+            });
         }
 
-        let end = match size {
-            Some(size) => offset + size,
-            None => dst_buffer.size,
-        };
-        if offset == end {
+        if offset == end_offset {
             log::trace!("Ignoring fill_buffer of size 0");
             return Ok(());
         }
@@ -144,7 +151,7 @@ impl Global {
         cmd_buf_data.buffer_memory_init_actions.extend(
             dst_buffer.initialization_status.read().create_action(
                 &dst_buffer,
-                offset..end,
+                offset..end_offset,
                 MemoryInitKind::ImplicitlyInitialized,
             ),
         );
@@ -154,7 +161,7 @@ impl Global {
         let cmd_buf_raw = cmd_buf_data.encoder.open()?;
         unsafe {
             cmd_buf_raw.transition_buffers(dst_barrier.into_iter());
-            cmd_buf_raw.clear_buffer(dst_raw, offset..end);
+            cmd_buf_raw.clear_buffer(dst_raw, offset..end_offset);
         }
         Ok(())
     }
@@ -366,7 +373,7 @@ fn clear_texture_via_buffer_copies<A: HalApi>(
         assert!(
             max_rows_per_copy > 0,
             "Zero buffer size is too small to fill a single row \
-                 of a texture with format {:?} and desc {:?}",
+            of a texture with format {:?} and desc {:?}",
             texture_desc.format,
             texture_desc.size
         );