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>
diff --git a/src/dawn_native/CommandValidation.cpp b/src/dawn_native/CommandValidation.cpp
index 034585a..7b2d8fc 100644
--- a/src/dawn_native/CommandValidation.cpp
+++ b/src/dawn_native/CommandValidation.cpp
@@ -370,15 +370,20 @@
static_cast<uint64_t>(maxStart);
}
- uint32_t ComputeRequiredBytesInCopy(const TexelBlockInfo& blockInfo,
- const Extent3D& copySize,
- uint32_t bytesPerRow,
- uint32_t rowsPerImage) {
+ ResultOrError<uint64_t> ComputeRequiredBytesInCopy(const TexelBlockInfo& blockInfo,
+ const Extent3D& copySize,
+ uint32_t bytesPerRow,
+ uint32_t rowsPerImage) {
// Default value for rowsPerImage
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 @@
ASSERT(copySize.height >= 1);
ASSERT(copySize.depth >= 1);
- uint64_t texelBlockRowsPerImage = rowsPerImage / blockInfo.blockHeight;
- uint64_t bytesPerImage = bytesPerRow * texelBlockRowsPerImage;
+ uint32_t texelBlockRowsPerImage = rowsPerImage / blockInfo.blockHeight;
+ // 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) +
- (copySize.width / blockInfo.blockWidth * blockInfo.blockByteSize);
+ uint64_t(bytesPerRow) * (copySize.height / blockInfo.blockHeight - 1) +
+ (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 @@
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 @@
// 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 {};
}
diff --git a/src/dawn_native/CommandValidation.h b/src/dawn_native/CommandValidation.h
index 719ce17..0aaa398 100644
--- a/src/dawn_native/CommandValidation.h
+++ b/src/dawn_native/CommandValidation.h
@@ -41,10 +41,10 @@
MaybeError ValidateTimestampQuery(QuerySetBase* querySet, uint32_t queryIndex);
- uint32_t ComputeRequiredBytesInCopy(const TexelBlockInfo& blockInfo,
- const Extent3D& copySize,
- uint32_t bytesPerRow,
- uint32_t rowsPerImage);
+ ResultOrError<uint64_t> ComputeRequiredBytesInCopy(const TexelBlockInfo& blockInfo,
+ const Extent3D& copySize,
+ uint32_t bytesPerRow,
+ uint32_t rowsPerImage);
MaybeError ValidateLinearTextureData(const TextureDataLayout& layout,
uint64_t byteSize,
diff --git a/src/dawn_native/Queue.cpp b/src/dawn_native/Queue.cpp
index 403e022..435d265 100644
--- a/src/dawn_native/Queue.cpp
+++ b/src/dawn_native/Queue.cpp
@@ -314,7 +314,7 @@
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);
diff --git a/src/dawn_native/d3d12/QueueD3D12.cpp b/src/dawn_native/d3d12/QueueD3D12.cpp
index c880892..c7c0978 100644
--- a/src/dawn_native/d3d12/QueueD3D12.cpp
+++ b/src/dawn_native/d3d12/QueueD3D12.cpp
@@ -37,8 +37,11 @@
const TextureDataLayout& dataLayout,
const Format& textureFormat,
const Extent3D& writeSizePixel) {
- uint32_t newDataSizeBytes = ComputeRequiredBytesInCopy(
- textureFormat, writeSizePixel, optimallyAlignedBytesPerRow, alignedRowsPerImage);
+ uint64_t newDataSizeBytes;
+ DAWN_TRY_ASSIGN(
+ newDataSizeBytes,
+ ComputeRequiredBytesInCopy(textureFormat, writeSizePixel,
+ optimallyAlignedBytesPerRow, alignedRowsPerImage));
UploadHandle uploadHandle;
DAWN_TRY_ASSIGN(uploadHandle, device->GetDynamicUploader()->Allocate(
diff --git a/src/dawn_native/metal/QueueMTL.mm b/src/dawn_native/metal/QueueMTL.mm
index 44cbb0f..07b1f79 100644
--- a/src/dawn_native/metal/QueueMTL.mm
+++ b/src/dawn_native/metal/QueueMTL.mm
@@ -34,8 +34,10 @@
const TextureDataLayout& dataLayout,
const TexelBlockInfo& blockInfo,
const Extent3D& writeSizePixel) {
- uint32_t newDataSizeBytes = ComputeRequiredBytesInCopy(
- blockInfo, writeSizePixel, alignedBytesPerRow, alignedRowsPerImage);
+ uint64_t newDataSizeBytes;
+ DAWN_TRY_ASSIGN(newDataSizeBytes,
+ ComputeRequiredBytesInCopy(blockInfo, writeSizePixel,
+ alignedBytesPerRow, alignedRowsPerImage));
UploadHandle uploadHandle;
DAWN_TRY_ASSIGN(uploadHandle, device->GetDynamicUploader()->Allocate(
diff --git a/src/dawn_native/vulkan/QueueVk.cpp b/src/dawn_native/vulkan/QueueVk.cpp
index 3842652..8d70fd5 100644
--- a/src/dawn_native/vulkan/QueueVk.cpp
+++ b/src/dawn_native/vulkan/QueueVk.cpp
@@ -37,8 +37,11 @@
const TextureDataLayout& dataLayout,
const TexelBlockInfo& blockInfo,
const Extent3D& writeSizePixel) {
- uint32_t newDataSizeBytes = ComputeRequiredBytesInCopy(
- blockInfo, writeSizePixel, optimallyAlignedBytesPerRow, alignedRowsPerImage);
+ uint64_t newDataSizeBytes;
+ DAWN_TRY_ASSIGN(
+ newDataSizeBytes,
+ ComputeRequiredBytesInCopy(blockInfo, writeSizePixel, optimallyAlignedBytesPerRow,
+ alignedRowsPerImage));
uint64_t optimalOffsetAlignment =
ToBackend(device)
diff --git a/src/tests/unittests/validation/CopyCommandsValidationTests.cpp b/src/tests/unittests/validation/CopyCommandsValidationTests.cpp
index 3d4f65c..52c242d 100644
--- a/src/tests/unittests/validation/CopyCommandsValidationTests.cpp
+++ b/src/tests/unittests/validation/CopyCommandsValidationTests.cpp
@@ -793,6 +793,20 @@
}
}
+// 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 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) {
diff --git a/src/tests/unittests/validation/QueueWriteTextureValidationTests.cpp b/src/tests/unittests/validation/QueueWriteTextureValidationTests.cpp
index d0aad61..ec65bf6 100644
--- a/src/tests/unittests/validation/QueueWriteTextureValidationTests.cpp
+++ b/src/tests/unittests/validation/QueueWriteTextureValidationTests.cpp
@@ -334,6 +334,18 @@
}
}
+ // 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;
