Adding validation for requiredBytesInCopy overflow

Also some uint32_t computations are now done on uint64_t.

Bug: dawn:482
Change-Id: Ia0094e16999ec3a9fec193f27760e73cd14d289c
Reviewed-on: https://dawn-review.googlesource.com/c/dawn/+/26540
Reviewed-by: Corentin Wallez <cwallez@chromium.org>
Reviewed-by: Jiawei Shao <jiawei.shao@intel.com>
Commit-Queue: Tomek Ponitka <tommek@google.com>

src/dawn_native/CommandValidation.cpp

@@ -370,7 +370,7 @@ namespace dawn_native {
                static_cast<uint64_t>(maxStart);
     }
 
-    uint32_t ComputeRequiredBytesInCopy(const TexelBlockInfo& blockInfo,
+    ResultOrError<uint64_t> ComputeRequiredBytesInCopy(const TexelBlockInfo& blockInfo,
                                                        const Extent3D& copySize,
                                                        uint32_t bytesPerRow,
                                                        uint32_t rowsPerImage) {
@@ -378,7 +378,12 @@ namespace dawn_native {
         if (rowsPerImage == 0) {
             rowsPerImage = copySize.height;
         }
+
         ASSERT(rowsPerImage >= copySize.height);
+        if (copySize.height > 1 || copySize.depth > 1) {
+            ASSERT(bytesPerRow >= copySize.width / blockInfo.blockWidth * blockInfo.blockByteSize);
+        }
+
         if (copySize.width == 0 || copySize.height == 0 || copySize.depth == 0) {
             return 0;
         }
@@ -386,11 +391,23 @@ namespace dawn_native {
         ASSERT(copySize.height >= 1);
         ASSERT(copySize.depth >= 1);
 
-        uint64_t texelBlockRowsPerImage = rowsPerImage / blockInfo.blockHeight;
+        uint32_t texelBlockRowsPerImage = rowsPerImage / blockInfo.blockHeight;
-        uint64_t bytesPerImage = bytesPerRow * texelBlockRowsPerImage;
+        // bytesPerImage won't overflow since we're multiplying two uint32_t numbers
+        uint64_t bytesPerImage = uint64_t(texelBlockRowsPerImage) * bytesPerRow;
+        // Provided that copySize.height > 1: bytesInLastSlice won't overflow since it's at most
+        // bytesPerImage. Otherwise the result is a multiplication of two uint32_t numbers.
         uint64_t bytesInLastSlice =
-            bytesPerRow * (copySize.height / blockInfo.blockHeight - 1) +
+            uint64_t(bytesPerRow) * (copySize.height / blockInfo.blockHeight - 1) +
-            (copySize.width / blockInfo.blockWidth * blockInfo.blockByteSize);
+            (uint64_t(copySize.width) / blockInfo.blockWidth * blockInfo.blockByteSize);
+
+        // This error cannot be thrown for copySize.depth = 1.
+        // For copySize.depth > 1 we know that:
+        // requiredBytesInCopy >= (copySize.depth * bytesPerImage) / 2, so if
+        // copySize.depth * bytesPerImage overflows uint64_t, then requiredBytesInCopy is definitely
+        // too large to fit in the available data size.
+        if (std::numeric_limits<uint64_t>::max() / copySize.depth < bytesPerImage) {
+            return DAWN_VALIDATION_ERROR("requiredBytesInCopy is too large");
+        }
         return bytesPerImage * (copySize.depth - 1) + bytesInLastSlice;
     }
 
@@ -420,25 +437,6 @@ namespace dawn_native {
             return DAWN_VALIDATION_ERROR("Offset must be a multiple of the texel or block size");
         }
 
-        // Validation for the copy being in-bounds:
-        if (layout.rowsPerImage != 0 && layout.rowsPerImage < copyExtent.height) {
-            return DAWN_VALIDATION_ERROR("rowsPerImage must not be less than the copy height.");
-        }
-
-        // We compute required bytes in copy after validating texel block alignments
-        // because the divisibility conditions are necessary for the algorithm to be valid.
-        // TODO(tommek@google.com): to match the spec this should only be checked when
-        // copyExtent.depth > 1.
-        uint32_t requiredBytesInCopy = ComputeRequiredBytesInCopy(
-            blockInfo, copyExtent, layout.bytesPerRow, layout.rowsPerImage);
-
-        bool fitsInData =
-            layout.offset <= byteSize && (requiredBytesInCopy <= (byteSize - layout.offset));
-        if (!fitsInData) {
-            return DAWN_VALIDATION_ERROR(
-                "Required size for texture data layout exceeds the given size");
-        }
-
         // Validation for other members in layout:
         if ((copyExtent.height > 1 || copyExtent.depth > 1) &&
             layout.bytesPerRow <
@@ -450,6 +448,26 @@ namespace dawn_native {
         // TODO(tommek@google.com): to match the spec there should be another condition here
         // on rowsPerImage >= copyExtent.height if copyExtent.depth > 1.
 
+        // Validation for the copy being in-bounds:
+        if (layout.rowsPerImage != 0 && layout.rowsPerImage < copyExtent.height) {
+            return DAWN_VALIDATION_ERROR("rowsPerImage must not be less than the copy height.");
+        }
+
+        // We compute required bytes in copy after validating texel block alignments
+        // because the divisibility conditions are necessary for the algorithm to be valid,
+        // also the bytesPerRow bound is necessary to avoid overflows.
+        uint64_t requiredBytesInCopy;
+        DAWN_TRY_ASSIGN(requiredBytesInCopy,
+                        ComputeRequiredBytesInCopy(blockInfo, copyExtent, layout.bytesPerRow,
+                                                   layout.rowsPerImage));
+
+        bool fitsInData =
+            layout.offset <= byteSize && (requiredBytesInCopy <= (byteSize - layout.offset));
+        if (!fitsInData) {
+            return DAWN_VALIDATION_ERROR(
+                "Required size for texture data layout exceeds the given size");
+        }
+
         return {};
     }
 

src/dawn_native/CommandValidation.h

@@ -41,7 +41,7 @@ namespace dawn_native {
 
     MaybeError ValidateTimestampQuery(QuerySetBase* querySet, uint32_t queryIndex);
 
-    uint32_t ComputeRequiredBytesInCopy(const TexelBlockInfo& blockInfo,
+    ResultOrError<uint64_t> ComputeRequiredBytesInCopy(const TexelBlockInfo& blockInfo,
                                                        const Extent3D& copySize,
                                                        uint32_t bytesPerRow,
                                                        uint32_t rowsPerImage);

src/dawn_native/Queue.cpp

@@ -314,7 +314,7 @@ namespace dawn_native {
                 srcPointer += imageAdditionalStride;
             }
         } else {
-            uint64_t layerSize = rowsPerImageInBlock * actualBytesPerRow;
+            uint64_t layerSize = uint64_t(rowsPerImageInBlock) * actualBytesPerRow;
             if (!copyWholeData) {  // copy layer by layer
                 for (uint32_t d = 0; d < depth; ++d) {
                     memcpy(dstPointer, srcPointer, layerSize);

src/dawn_native/d3d12/QueueD3D12.cpp

@@ -37,8 +37,11 @@ namespace dawn_native { namespace d3d12 {
             const TextureDataLayout& dataLayout,
             const Format& textureFormat,
             const Extent3D& writeSizePixel) {
-            uint32_t newDataSizeBytes = ComputeRequiredBytesInCopy(
+            uint64_t newDataSizeBytes;
-                textureFormat, writeSizePixel, optimallyAlignedBytesPerRow, alignedRowsPerImage);
+            DAWN_TRY_ASSIGN(
+                newDataSizeBytes,
+                ComputeRequiredBytesInCopy(textureFormat, writeSizePixel,
+                                           optimallyAlignedBytesPerRow, alignedRowsPerImage));
 
             UploadHandle uploadHandle;
             DAWN_TRY_ASSIGN(uploadHandle, device->GetDynamicUploader()->Allocate(

src/dawn_native/metal/QueueMTL.mm

@@ -34,8 +34,10 @@ namespace dawn_native { namespace metal {
             const TextureDataLayout& dataLayout,
             const TexelBlockInfo& blockInfo,
             const Extent3D& writeSizePixel) {
-            uint32_t newDataSizeBytes = ComputeRequiredBytesInCopy(
+            uint64_t newDataSizeBytes;
-                blockInfo, writeSizePixel, alignedBytesPerRow, alignedRowsPerImage);
+            DAWN_TRY_ASSIGN(newDataSizeBytes,
+                            ComputeRequiredBytesInCopy(blockInfo, writeSizePixel,
+                                                       alignedBytesPerRow, alignedRowsPerImage));
 
             UploadHandle uploadHandle;
             DAWN_TRY_ASSIGN(uploadHandle, device->GetDynamicUploader()->Allocate(

src/dawn_native/vulkan/QueueVk.cpp

@@ -37,8 +37,11 @@ namespace dawn_native { namespace vulkan {
             const TextureDataLayout& dataLayout,
             const TexelBlockInfo& blockInfo,
             const Extent3D& writeSizePixel) {
-            uint32_t newDataSizeBytes = ComputeRequiredBytesInCopy(
+            uint64_t newDataSizeBytes;
-                blockInfo, writeSizePixel, optimallyAlignedBytesPerRow, alignedRowsPerImage);
+            DAWN_TRY_ASSIGN(
+                newDataSizeBytes,
+                ComputeRequiredBytesInCopy(blockInfo, writeSizePixel, optimallyAlignedBytesPerRow,
+                                           alignedRowsPerImage));
 
             uint64_t optimalOffsetAlignment =
                 ToBackend(device)

src/tests/unittests/validation/CopyCommandsValidationTests.cpp

@@ -793,6 +793,20 @@ TEST_F(CopyCommandTest_B2T, CopyToStencilAspect) {
     }
 }
 
+// Test that CopyB2T throws an error when requiredBytesInCopy overflows uint64_t
+TEST_F(CopyCommandTest_B2T, RequiredBytesInCopyOverflow) {
+    wgpu::Buffer source = CreateBuffer(10000, wgpu::BufferUsage::CopySrc);
+    wgpu::Texture destination =
+        Create2DTexture(1, 1, 1, 16, wgpu::TextureFormat::RGBA8Unorm, wgpu::TextureUsage::CopyDst);
+
+    // Success
+    TestB2TCopy(utils::Expectation::Success, source, 0, (1 << 31), (1 << 31), destination, 0,
+                {0, 0, 0}, {1, 1, 1});
+    // Failure because bytesPerImage * (depth - 1) overflows
+    TestB2TCopy(utils::Expectation::Failure, source, 0, (1 << 31), (1 << 31), destination, 0,
+                {0, 0, 0}, {1, 1, 16});
+}
+
 class CopyCommandTest_T2B : public CopyCommandTest {};
 
 // Test a successfull T2B copy
@@ -1227,6 +1241,20 @@ TEST_F(CopyCommandTest_T2B, CopyFromStencilAspect) {
     }
 }
 
+// Test that CopyT2B throws an error when requiredBytesInCopy overflows uint64_t
+TEST_F(CopyCommandTest_T2B, RequiredBytesInCopyOverflow) {
+    wgpu::Buffer destination = CreateBuffer(10000, wgpu::BufferUsage::CopyDst);
+    wgpu::Texture source =
+        Create2DTexture(1, 1, 1, 16, wgpu::TextureFormat::RGBA8Unorm, wgpu::TextureUsage::CopySrc);
+
+    // Success
+    TestT2BCopy(utils::Expectation::Success, source, 0, {0, 0, 0}, destination, 0, (1 << 31),
+                (1 << 31), {1, 1, 1});
+    // Failure because bytesPerImage * (depth - 1) overflows
+    TestT2BCopy(utils::Expectation::Failure, source, 0, {0, 0, 0}, destination, 0, (1 << 31),
+                (1 << 31), {1, 1, 16});
+}
+
 class CopyCommandTest_T2T : public CopyCommandTest {};
 
 TEST_F(CopyCommandTest_T2T, Success) {

src/tests/unittests/validation/QueueWriteTextureValidationTests.cpp

@@ -334,6 +334,18 @@ namespace {
         }
     }
 
+    // Test that WriteTexture throws an error when requiredBytesInCopy overflows uint64_t
+    TEST_F(QueueWriteTextureValidationTest, RequiredBytesInCopyOverflow) {
+        wgpu::Texture destination = Create2DTexture({1, 1, 16}, 1, wgpu::TextureFormat::RGBA8Unorm,
+                                                    wgpu::TextureUsage::CopyDst);
+
+        // success because depth = 1.
+        TestWriteTexture(10000, 0, (1 << 31), (1 << 31), destination, 0, {0, 0, 0}, {1, 1, 1});
+        // failure because bytesPerImage * (depth - 1) overflows.
+        ASSERT_DEVICE_ERROR(TestWriteTexture(10000, 0, (1 << 31), (1 << 31), destination, 0,
+                                             {0, 0, 0}, {1, 1, 16}));
+    }
+
     // Regression tests for a bug in the computation of texture data size in Dawn.
     TEST_F(QueueWriteTextureValidationTest, TextureWriteDataSizeLastRowComputation) {
         constexpr uint32_t kBytesPerRow = 256;