Avoid overflow in check texture copy bounds. (#2963)

Author: jimblandy

.github/workflows/ci.yml

@@ -225,6 +225,8 @@ jobs:
           for backend in ${{ matrix.backends }}; do
             echo "======= NATIVE TESTS $backend ======";
             WGPU_BACKEND=$backend cargo nextest run -p wgpu --no-fail-fast
+            # Test that we catch overflows in `--release` builds too.
+            WGPU_BACKEND=$backend cargo nextest run --release -p wgpu --no-fail-fast
           done
 
   fmt:

CHANGELOG.md

@@ -64,6 +64,7 @@ the same every time it is rendered, we now warn if it is missing.
 
 #### General
 - Free `StagingBuffers` even when an error occurs in the operation that consumes them. By @jimblandy in [#2961](https://github.com/gfx-rs/wgpu/pull/2961)
+- Avoid overflow when checking that texture copies fall within bounds. By @jimblandy in [#2963](https://github.com/gfx-rs/wgpu/pull/2963)
 - Improve the validation and error reporting of buffer mappings by @nical in [#2848](https://github.com/gfx-rs/wgpu/pull/2848)
 - Fix compilation errors when using wgpu-core in isolation while targetting `wasm32-unknown-unknown` by @Seamooo in [#2922](https://github.com/gfx-rs/wgpu/pull/2922)
 - Fixed opening of RenderDoc library by @abuffseagull in [#2930](https://github.com/gfx-rs/wgpu/pull/2930)

wgpu-core/src/command/transfer.rs

@@ -25,7 +25,7 @@ use std::iter;
 pub type ImageCopyBuffer = wgt::ImageCopyBuffer<BufferId>;
 pub type ImageCopyTexture = wgt::ImageCopyTexture<TextureId>;
 
-#[derive(Clone, Debug)]
+#[derive(Clone, Copy, Debug)]
 pub enum CopySide {
     Source,
     Destination,
@@ -326,37 +326,52 @@ pub(crate) fn validate_texture_copy_range(
         _ => {}
     }
 
-    let x_copy_max = texture_copy_view.origin.x + copy_size.width;
-    if x_copy_max > extent.width {
-        return Err(TransferError::TextureOverrun {
-            start_offset: texture_copy_view.origin.x,
-            end_offset: x_copy_max,
-            texture_size: extent.width,
-            dimension: TextureErrorDimension::X,
-            side: texture_side,
-        });
-    }
-    let y_copy_max = texture_copy_view.origin.y + copy_size.height;
-    if y_copy_max > extent.height {
-        return Err(TransferError::TextureOverrun {
-            start_offset: texture_copy_view.origin.y,
-            end_offset: y_copy_max,
-            texture_size: extent.height,
-            dimension: TextureErrorDimension::Y,
-            side: texture_side,
-        });
-    }
-    let z_copy_max = texture_copy_view.origin.z + copy_size.depth_or_array_layers;
-    if z_copy_max > extent.depth_or_array_layers {
-        return Err(TransferError::TextureOverrun {
-            start_offset: texture_copy_view.origin.z,
-            end_offset: z_copy_max,
-            texture_size: extent.depth_or_array_layers,
-            dimension: TextureErrorDimension::Z,
-            side: texture_side,
-        });
+    /// Return `Ok` if a run `size` texels long starting at `start_offset` falls
+    /// entirely within `texture_size`. Otherwise, return an appropriate a`Err`.
+    fn check_dimension(
+        dimension: TextureErrorDimension,
+        side: CopySide,
+        start_offset: u32,
+        size: u32,
+        texture_size: u32,
+    ) -> Result<(), TransferError> {
+        // Avoid underflow in the subtraction by checking start_offset against
+        // texture_size first.
+        if start_offset <= texture_size && size <= texture_size - start_offset {
+            Ok(())
+        } else {
+            Err(TransferError::TextureOverrun {
+                start_offset,
+                end_offset: start_offset.wrapping_add(size),
+                texture_size,
+                dimension,
+                side,
+            })
+        }
     }
 
+    check_dimension(
+        TextureErrorDimension::X,
+        texture_side,
+        texture_copy_view.origin.x,
+        copy_size.width,
+        extent.width,
+    )?;
+    check_dimension(
+        TextureErrorDimension::Y,
+        texture_side,
+        texture_copy_view.origin.y,
+        copy_size.height,
+        extent.height,
+    )?;
+    check_dimension(
+        TextureErrorDimension::Z,
+        texture_side,
+        texture_copy_view.origin.z,
+        copy_size.depth_or_array_layers,
+        extent.depth_or_array_layers,
+    )?;
+
     if texture_copy_view.origin.x % block_width != 0 {
         return Err(TransferError::UnalignedCopyOriginX);
     }

wgpu/tests/texture_bounds.rs

@@ -5,7 +5,7 @@ use std::num::NonZeroU32;
 
 #[test]
 fn bad_copy_origin() {
-    fn try_origin(origin: wgpu::Origin3d, should_panic: bool) {
+    fn try_origin(origin: wgpu::Origin3d, size: wgpu::Extent3d, should_panic: bool) {
         let mut parameters = TestParameters::default();
         if should_panic {
             parameters = parameters.failure();
@@ -23,15 +23,68 @@ fn bad_copy_origin() {
                 },
                 &data,
                 BUFFER_COPY_LAYOUT,
-                TEXTURE_SIZE,
+                size,
             );
         });
     }
 
-    try_origin(wgpu::Origin3d { x: 0, y: 0, z: 0 }, false);
-    try_origin(wgpu::Origin3d { x: 1, y: 0, z: 0 }, true);
-    try_origin(wgpu::Origin3d { x: 0, y: 1, z: 0 }, true);
-    try_origin(wgpu::Origin3d { x: 0, y: 0, z: 1 }, true);
+    try_origin(wgpu::Origin3d { x: 0, y: 0, z: 0 }, TEXTURE_SIZE, false);
+    try_origin(wgpu::Origin3d { x: 1, y: 0, z: 0 }, TEXTURE_SIZE, true);
+    try_origin(wgpu::Origin3d { x: 0, y: 1, z: 0 }, TEXTURE_SIZE, true);
+    try_origin(wgpu::Origin3d { x: 0, y: 0, z: 1 }, TEXTURE_SIZE, true);
+
+    try_origin(
+        wgpu::Origin3d {
+            x: TEXTURE_SIZE.width - 1,
+            y: TEXTURE_SIZE.height - 1,
+            z: TEXTURE_SIZE.depth_or_array_layers - 1,
+        },
+        wgpu::Extent3d {
+            width: 1,
+            height: 1,
+            depth_or_array_layers: 1,
+        },
+        false,
+    );
+    try_origin(
+        wgpu::Origin3d {
+            x: u32::MAX,
+            y: 0,
+            z: 0,
+        },
+        wgpu::Extent3d {
+            width: 1,
+            height: 1,
+            depth_or_array_layers: 1,
+        },
+        true,
+    );
+    try_origin(
+        wgpu::Origin3d {
+            x: u32::MAX,
+            y: 0,
+            z: 0,
+        },
+        wgpu::Extent3d {
+            width: 1,
+            height: 1,
+            depth_or_array_layers: 1,
+        },
+        true,
+    );
+    try_origin(
+        wgpu::Origin3d {
+            x: u32::MAX,
+            y: 0,
+            z: 0,
+        },
+        wgpu::Extent3d {
+            width: 1,
+            height: 1,
+            depth_or_array_layers: 1,
+        },
+        true,
+    );
 }
 
 const TEXTURE_SIZE: wgpu::Extent3d = wgpu::Extent3d {