D3D11: Fix out-of-range access with robust access.
When using a vertex buffer with DYNAMIC usage, with robust buffer
access enabled, we would sometimes read out-of-bounds when using very
large values for the index range. An unchecked signed addition would
overflow and lead to reading a negative offset.
Fix this problem by keeping the value size_t whenever possible. Also do
clamped casts when converting to a smaller values.
Also adds a regression test.
Bug: chromium:842028
Change-Id: Ie630ac857c6acfc0bace849a03eebfbaa2fbe89a
Reviewed-on: https://chromium-review.googlesource.com/1055928
Commit-Queue: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
diff --git a/src/libANGLE/renderer/d3d/BufferD3D.cpp b/src/libANGLE/renderer/d3d/BufferD3D.cpp
index 7769ab2..3df7e4f 100644
--- a/src/libANGLE/renderer/d3d/BufferD3D.cpp
+++ b/src/libANGLE/renderer/d3d/BufferD3D.cpp
@@ -160,10 +160,11 @@
}
// Creates static buffers if sufficient used data has been left unmodified
-void BufferD3D::promoteStaticUsage(const gl::Context *context, int dataSize)
+void BufferD3D::promoteStaticUsage(const gl::Context *context, size_t dataSize)
{
if (mUsage == D3DBufferUsage::DYNAMIC)
{
+ // Note: This is not a safe math operation. 'dataSize' can come from the app.
mUnmodifiedDataUse += dataSize;
if (mUnmodifiedDataUse > 3 * getSize())
diff --git a/src/libANGLE/renderer/d3d/BufferD3D.h b/src/libANGLE/renderer/d3d/BufferD3D.h
index 6015374..2f0ff48 100644
--- a/src/libANGLE/renderer/d3d/BufferD3D.h
+++ b/src/libANGLE/renderer/d3d/BufferD3D.h
@@ -55,7 +55,7 @@
virtual void initializeStaticData(const gl::Context *context);
virtual void invalidateStaticData(const gl::Context *context);
- void promoteStaticUsage(const gl::Context *context, int dataSize);
+ void promoteStaticUsage(const gl::Context *context, size_t dataSize);
gl::Error getIndexRange(const gl::Context *context,
GLenum type,
@@ -80,7 +80,7 @@
StaticIndexBufferInterface *mStaticIndexBuffer;
unsigned int mStaticBufferCacheTotalSize;
unsigned int mStaticVertexBufferOutOfDate;
- unsigned int mUnmodifiedDataUse;
+ size_t mUnmodifiedDataUse;
D3DBufferUsage mUsage;
};
diff --git a/src/libANGLE/renderer/d3d/RendererD3D.h b/src/libANGLE/renderer/d3d/RendererD3D.h
index 1ed48ec..86ea932 100644
--- a/src/libANGLE/renderer/d3d/RendererD3D.h
+++ b/src/libANGLE/renderer/d3d/RendererD3D.h
@@ -95,7 +95,7 @@
virtual gl::ErrorOrResult<unsigned int> getVertexSpaceRequired(
const gl::VertexAttribute &attrib,
const gl::VertexBinding &binding,
- GLsizei count,
+ size_t count,
GLsizei instances) const = 0;
};
diff --git a/src/libANGLE/renderer/d3d/VertexBuffer.cpp b/src/libANGLE/renderer/d3d/VertexBuffer.cpp
index 7c2d5ae..cb6d26f 100644
--- a/src/libANGLE/renderer/d3d/VertexBuffer.cpp
+++ b/src/libANGLE/renderer/d3d/VertexBuffer.cpp
@@ -155,7 +155,7 @@
const gl::VertexBinding &binding,
GLenum currentValueType,
GLint start,
- GLsizei count,
+ size_t count,
GLsizei instances,
unsigned int *outStreamOffset,
const uint8_t *sourceData)
@@ -190,7 +190,7 @@
gl::Error StreamingVertexBufferInterface::reserveVertexSpace(const gl::VertexAttribute &attrib,
const gl::VertexBinding &binding,
- GLsizei count,
+ size_t count,
GLsizei instances)
{
unsigned int requiredSpace = 0;
diff --git a/src/libANGLE/renderer/d3d/VertexBuffer.h b/src/libANGLE/renderer/d3d/VertexBuffer.h
index df8085d..9ee39ff 100644
--- a/src/libANGLE/renderer/d3d/VertexBuffer.h
+++ b/src/libANGLE/renderer/d3d/VertexBuffer.h
@@ -45,7 +45,7 @@
const gl::VertexBinding &binding,
GLenum currentValueType,
GLint start,
- GLsizei count,
+ size_t count,
GLsizei instances,
unsigned int offset,
const uint8_t *sourceData) = 0;
@@ -110,14 +110,14 @@
const gl::VertexBinding &binding,
GLenum currentValueType,
GLint start,
- GLsizei count,
+ size_t count,
GLsizei instances,
unsigned int *outStreamOffset,
const uint8_t *sourceData);
gl::Error reserveVertexSpace(const gl::VertexAttribute &attribute,
const gl::VertexBinding &binding,
- GLsizei count,
+ size_t count,
GLsizei instances);
private:
diff --git a/src/libANGLE/renderer/d3d/VertexDataManager.cpp b/src/libANGLE/renderer/d3d/VertexDataManager.cpp
index f20386b..183c895 100644
--- a/src/libANGLE/renderer/d3d/VertexDataManager.cpp
+++ b/src/libANGLE/renderer/d3d/VertexDataManager.cpp
@@ -392,7 +392,7 @@
std::vector<TranslatedAttribute> *translatedAttribs,
const gl::AttributesMask &dynamicAttribsMask,
GLint start,
- GLsizei count,
+ size_t count,
GLsizei instances)
{
// Instantiating this class will ensure the streaming buffer is never left mapped.
@@ -434,7 +434,7 @@
const gl::Context *context,
const std::vector<TranslatedAttribute> &translatedAttribs,
const gl::AttributesMask &dynamicAttribsMask,
- GLsizei count)
+ size_t count)
{
for (auto attribIndex : dynamicAttribsMask)
{
@@ -445,16 +445,17 @@
gl::Buffer *buffer = binding.getBuffer().get();
if (buffer)
{
+ // Note: this multiplication can overflow. It should not be a security problem.
BufferD3D *bufferD3D = GetImplAs<BufferD3D>(buffer);
size_t typeSize = ComputeVertexAttributeTypeSize(*dynamicAttrib.attribute);
- bufferD3D->promoteStaticUsage(context, count * static_cast<int>(typeSize));
+ bufferD3D->promoteStaticUsage(context, count * typeSize);
}
}
}
gl::Error VertexDataManager::reserveSpaceForAttrib(const TranslatedAttribute &translatedAttrib,
GLint start,
- GLsizei count,
+ size_t count,
GLsizei instances) const
{
ASSERT(translatedAttrib.attribute && translatedAttrib.binding);
@@ -467,8 +468,8 @@
BufferD3D *bufferD3D = buffer ? GetImplAs<BufferD3D>(buffer) : nullptr;
ASSERT(!bufferD3D || bufferD3D->getStaticVertexBuffer(attrib, binding) == nullptr);
- size_t totalCount = gl::ComputeVertexBindingElementCount(
- binding.getDivisor(), static_cast<size_t>(count), static_cast<size_t>(instances));
+ size_t totalCount = gl::ComputeVertexBindingElementCount(binding.getDivisor(), count,
+ static_cast<size_t>(instances));
// TODO(jiajia.qin@intel.com): force the index buffer to clamp any out of range indices instead
// of invalid operation here.
if (bufferD3D)
@@ -486,15 +487,14 @@
return gl::InvalidOperation() << "Vertex buffer is not big enough for the draw call.";
}
}
- return mStreamingBuffer->reserveVertexSpace(attrib, binding, static_cast<GLsizei>(totalCount),
- instances);
+ return mStreamingBuffer->reserveVertexSpace(attrib, binding, totalCount, instances);
}
gl::Error VertexDataManager::storeDynamicAttrib(const gl::Context *context,
TranslatedAttribute *translated,
GLint start,
- GLsizei count,
- GLsizei instances)
+ size_t count,
+ GLsizei instances) const
{
ASSERT(translated->attribute && translated->binding);
const auto &attrib = *translated->attribute;
@@ -529,8 +529,8 @@
translated->storage = nullptr;
ANGLE_TRY_RESULT(mFactory->getVertexSpaceRequired(attrib, binding, 1, 0), translated->stride);
- size_t totalCount = gl::ComputeVertexBindingElementCount(
- binding.getDivisor(), static_cast<size_t>(count), static_cast<size_t>(instances));
+ size_t totalCount = gl::ComputeVertexBindingElementCount(binding.getDivisor(), count,
+ static_cast<size_t>(instances));
ANGLE_TRY(mStreamingBuffer->storeDynamicAttribute(
attrib, binding, translated->currentValueType, firstVertexIndex,
diff --git a/src/libANGLE/renderer/d3d/VertexDataManager.h b/src/libANGLE/renderer/d3d/VertexDataManager.h
index 694366d..2fb4fe6 100644
--- a/src/libANGLE/renderer/d3d/VertexDataManager.h
+++ b/src/libANGLE/renderer/d3d/VertexDataManager.h
@@ -105,14 +105,14 @@
std::vector<TranslatedAttribute> *translatedAttribs,
const gl::AttributesMask &dynamicAttribsMask,
GLint start,
- GLsizei count,
+ size_t count,
GLsizei instances);
// Promote static usage of dynamic buffers.
static void PromoteDynamicAttribs(const gl::Context *context,
const std::vector<TranslatedAttribute> &translatedAttribs,
const gl::AttributesMask &dynamicAttribsMask,
- GLsizei count);
+ size_t count);
gl::Error storeCurrentValue(const gl::VertexAttribCurrentValueData ¤tValue,
TranslatedAttribute *translated,
@@ -130,15 +130,15 @@
};
gl::Error reserveSpaceForAttrib(const TranslatedAttribute &translatedAttrib,
- GLsizei count,
GLint start,
+ size_t count,
GLsizei instances) const;
gl::Error storeDynamicAttrib(const gl::Context *context,
TranslatedAttribute *translated,
GLint start,
- GLsizei count,
- GLsizei instances);
+ size_t count,
+ GLsizei instances) const;
BufferFactoryD3D *const mFactory;
diff --git a/src/libANGLE/renderer/d3d/d3d11/InputLayoutCache.cpp b/src/libANGLE/renderer/d3d/d3d11/InputLayoutCache.cpp
index 8dd5ad8..f8f8865 100644
--- a/src/libANGLE/renderer/d3d/d3d11/InputLayoutCache.cpp
+++ b/src/libANGLE/renderer/d3d/d3d11/InputLayoutCache.cpp
@@ -246,11 +246,12 @@
// As per the spec for ANGLE_instanced_arrays, not all attributes can be instanced
// simultaneously, so a non-instanced element must exist.
- GLsizei numIndicesPerInstance = 0;
+ UINT numIndicesPerInstance = 0;
if (drawCallParams.instances() > 0)
{
// This requires that the index range is resolved.
- numIndicesPerInstance = drawCallParams.vertexCount();
+ // Note: Vertex indexes can be arbitrarily large.
+ numIndicesPerInstance = drawCallParams.getClampedVertexCount<UINT>();
}
for (size_t elementIndex = 0; elementIndex < inputElementCount; ++elementIndex)
diff --git a/src/libANGLE/renderer/d3d/d3d11/Renderer11.cpp b/src/libANGLE/renderer/d3d/d3d11/Renderer11.cpp
index 2c48668..3d0ad0c 100644
--- a/src/libANGLE/renderer/d3d/d3d11/Renderer11.cpp
+++ b/src/libANGLE/renderer/d3d/d3d11/Renderer11.cpp
@@ -1403,7 +1403,7 @@
gl::Error Renderer11::drawArrays(const gl::Context *context, const gl::DrawCallParams ¶ms)
{
- if (params.vertexCount() < mStateManager.getCurrentMinimumDrawCount())
+ if (params.vertexCount() < static_cast<size_t>(mStateManager.getCurrentMinimumDrawCount()))
{
return gl::NoError();
}
@@ -1419,6 +1419,9 @@
GLsizei adjustedInstanceCount = GetAdjustedInstanceCount(program, params.instances());
ProgramD3D *programD3D = GetImplAs<ProgramD3D>(program);
+ // Note: vertex indexes can be arbitrarily large.
+ UINT clampedVertexCount = params.getClampedVertexCount<UINT>();
+
if (programD3D->usesGeometryShader(params.mode()) &&
glState.isTransformFeedbackActiveUnpaused())
{
@@ -1430,11 +1433,11 @@
if (adjustedInstanceCount > 0)
{
- mDeviceContext->DrawInstanced(params.vertexCount(), adjustedInstanceCount, 0, 0);
+ mDeviceContext->DrawInstanced(clampedVertexCount, adjustedInstanceCount, 0, 0);
}
else
{
- mDeviceContext->Draw(params.vertexCount(), 0);
+ mDeviceContext->Draw(clampedVertexCount, 0);
}
rx::ShaderExecutableD3D *pixelExe = nullptr;
@@ -1458,24 +1461,24 @@
if (adjustedInstanceCount > 0)
{
- mDeviceContext->DrawInstanced(params.vertexCount(), adjustedInstanceCount, 0, 0);
+ mDeviceContext->DrawInstanced(clampedVertexCount, adjustedInstanceCount, 0, 0);
}
else
{
- mDeviceContext->Draw(params.vertexCount(), 0);
+ mDeviceContext->Draw(clampedVertexCount, 0);
}
return gl::NoError();
}
if (params.mode() == GL_LINE_LOOP)
{
- return drawLineLoop(context, params.vertexCount(), GL_NONE, nullptr, 0,
+ return drawLineLoop(context, clampedVertexCount, GL_NONE, nullptr, 0,
adjustedInstanceCount);
}
if (params.mode() == GL_TRIANGLE_FAN)
{
- return drawTriangleFan(context, params.vertexCount(), GL_NONE, nullptr, 0,
+ return drawTriangleFan(context, clampedVertexCount, GL_NONE, nullptr, 0,
adjustedInstanceCount);
}
@@ -1486,11 +1489,11 @@
{
if (adjustedInstanceCount == 0)
{
- mDeviceContext->Draw(params.vertexCount(), 0);
+ mDeviceContext->Draw(clampedVertexCount, 0);
}
else
{
- mDeviceContext->DrawInstanced(params.vertexCount(), adjustedInstanceCount, 0, 0);
+ mDeviceContext->DrawInstanced(clampedVertexCount, adjustedInstanceCount, 0, 0);
}
return gl::NoError();
}
@@ -1503,7 +1506,7 @@
// D3D_PRIMITIVE_TOPOLOGY_TRIANGLELIST and DrawIndexedInstanced is called instead.
if (adjustedInstanceCount == 0)
{
- mDeviceContext->DrawIndexedInstanced(6, params.vertexCount(), 0, 0, 0);
+ mDeviceContext->DrawIndexedInstanced(6, clampedVertexCount, 0, 0, 0);
return gl::NoError();
}
@@ -1516,7 +1519,7 @@
{
ANGLE_TRY(
mStateManager.updateVertexOffsetsForPointSpritesEmulation(params.baseVertex(), i));
- mDeviceContext->DrawIndexedInstanced(6, params.vertexCount(), 0, 0, 0);
+ mDeviceContext->DrawIndexedInstanced(6, clampedVertexCount, 0, 0, 0);
}
// This required by updateVertexOffsets... above but is outside of the loop for speed.
@@ -1595,13 +1598,13 @@
// efficent code path. Instanced rendering of emulated pointsprites requires a loop to draw each
// batch of points. An offset into the instanced data buffer is calculated and applied on each
// iteration to ensure all instances are rendered correctly.
- GLsizei elementsToRender = params.vertexCount();
+ UINT clampedVertexCount = params.getClampedVertexCount<UINT>();
// Each instance being rendered requires the inputlayout cache to reapply buffers and offsets.
for (GLsizei i = 0; i < params.instances(); i++)
{
ANGLE_TRY(mStateManager.updateVertexOffsetsForPointSpritesEmulation(startVertex, i));
- mDeviceContext->DrawIndexedInstanced(6, elementsToRender, 0, 0, 0);
+ mDeviceContext->DrawIndexedInstanced(6, clampedVertexCount, 0, 0, 0);
}
mStateManager.invalidateVertexBuffer();
return gl::NoError();
@@ -1653,7 +1656,7 @@
}
gl::Error Renderer11::drawLineLoop(const gl::Context *context,
- GLsizei count,
+ GLuint count,
GLenum type,
const void *indexPointer,
int baseVertex,
@@ -1690,8 +1693,6 @@
}
// Checked by Renderer11::applyPrimitiveType
- ASSERT(count >= 0);
-
if (static_cast<unsigned int>(count) + 1 >
(std::numeric_limits<unsigned int>::max() / sizeof(unsigned int)))
{
@@ -1737,7 +1738,7 @@
}
gl::Error Renderer11::drawTriangleFan(const gl::Context *context,
- GLsizei count,
+ GLuint count,
GLenum type,
const void *indices,
int baseVertex,
@@ -3570,7 +3571,7 @@
gl::ErrorOrResult<unsigned int> Renderer11::getVertexSpaceRequired(
const gl::VertexAttribute &attrib,
const gl::VertexBinding &binding,
- GLsizei count,
+ size_t count,
GLsizei instances) const
{
if (!attrib.enabled)
diff --git a/src/libANGLE/renderer/d3d/d3d11/Renderer11.h b/src/libANGLE/renderer/d3d/d3d11/Renderer11.h
index 4f1a3f4..3e6c4e4 100644
--- a/src/libANGLE/renderer/d3d/d3d11/Renderer11.h
+++ b/src/libANGLE/renderer/d3d/d3d11/Renderer11.h
@@ -342,7 +342,7 @@
// function.
gl::ErrorOrResult<unsigned int> getVertexSpaceRequired(const gl::VertexAttribute &attrib,
const gl::VertexBinding &binding,
- GLsizei count,
+ size_t count,
GLsizei instances) const override;
gl::Error readFromAttachment(const gl::Context *context,
@@ -460,13 +460,13 @@
angle::WorkaroundsD3D generateWorkarounds() const override;
gl::Error drawLineLoop(const gl::Context *context,
- GLsizei count,
+ GLuint count,
GLenum type,
const void *indices,
int baseVertex,
int instances);
gl::Error drawTriangleFan(const gl::Context *context,
- GLsizei count,
+ GLuint count,
GLenum type,
const void *indices,
int baseVertex,
diff --git a/src/libANGLE/renderer/d3d/d3d11/VertexBuffer11.cpp b/src/libANGLE/renderer/d3d/d3d11/VertexBuffer11.cpp
index 69e5987..3649cbe 100644
--- a/src/libANGLE/renderer/d3d/d3d11/VertexBuffer11.cpp
+++ b/src/libANGLE/renderer/d3d/d3d11/VertexBuffer11.cpp
@@ -96,7 +96,7 @@
const gl::VertexBinding &binding,
GLenum currentValueType,
GLint start,
- GLsizei count,
+ size_t count,
GLsizei instances,
unsigned int offset,
const uint8_t *sourceData)
diff --git a/src/libANGLE/renderer/d3d/d3d11/VertexBuffer11.h b/src/libANGLE/renderer/d3d/d3d11/VertexBuffer11.h
index ab619ae..7778c31 100644
--- a/src/libANGLE/renderer/d3d/d3d11/VertexBuffer11.h
+++ b/src/libANGLE/renderer/d3d/d3d11/VertexBuffer11.h
@@ -31,7 +31,7 @@
const gl::VertexBinding &binding,
GLenum currentValueType,
GLint start,
- GLsizei count,
+ size_t count,
GLsizei instances,
unsigned int offset,
const uint8_t *sourceData) override;
diff --git a/src/libANGLE/renderer/d3d/d3d9/Renderer9.cpp b/src/libANGLE/renderer/d3d/d3d9/Renderer9.cpp
index 1abd244..316de46 100644
--- a/src/libANGLE/renderer/d3d/d3d9/Renderer9.cpp
+++ b/src/libANGLE/renderer/d3d/d3d9/Renderer9.cpp
@@ -3007,7 +3007,7 @@
gl::ErrorOrResult<unsigned int> Renderer9::getVertexSpaceRequired(const gl::VertexAttribute &attrib,
const gl::VertexBinding &binding,
- GLsizei count,
+ size_t count,
GLsizei instances) const
{
if (!attrib.enabled)
diff --git a/src/libANGLE/renderer/d3d/d3d9/Renderer9.h b/src/libANGLE/renderer/d3d/d3d9/Renderer9.h
index 239bfd0..0d5f450 100644
--- a/src/libANGLE/renderer/d3d/d3d9/Renderer9.h
+++ b/src/libANGLE/renderer/d3d/d3d9/Renderer9.h
@@ -346,7 +346,7 @@
// function.
gl::ErrorOrResult<unsigned int> getVertexSpaceRequired(const gl::VertexAttribute &attrib,
const gl::VertexBinding &binding,
- GLsizei count,
+ size_t count,
GLsizei instances) const override;
gl::Error copyToRenderTarget(IDirect3DSurface9 *dest,
diff --git a/src/libANGLE/renderer/d3d/d3d9/VertexBuffer9.cpp b/src/libANGLE/renderer/d3d/d3d9/VertexBuffer9.cpp
index c0b80a8..51af37a 100644
--- a/src/libANGLE/renderer/d3d/d3d9/VertexBuffer9.cpp
+++ b/src/libANGLE/renderer/d3d/d3d9/VertexBuffer9.cpp
@@ -61,7 +61,7 @@
const gl::VertexBinding &binding,
GLenum currentValueType,
GLint start,
- GLsizei count,
+ size_t count,
GLsizei instances,
unsigned int offset,
const uint8_t *sourceData)
@@ -71,8 +71,8 @@
return gl::OutOfMemory() << "Internal vertex buffer is not initialized.";
}
- int inputStride = static_cast<int>(gl::ComputeVertexAttributeStride(attrib, binding));
- int elementSize = static_cast<int>(gl::ComputeVertexAttributeTypeSize(attrib));
+ size_t inputStride = gl::ComputeVertexAttributeStride(attrib, binding);
+ size_t elementSize = gl::ComputeVertexAttributeTypeSize(attrib);
DWORD lockFlags = mDynamicUsage ? D3DLOCK_NOOVERWRITE : 0;
@@ -105,7 +105,7 @@
if (!needsConversion && inputStride == elementSize)
{
- size_t copySize = static_cast<size_t>(count) * static_cast<size_t>(inputStride);
+ size_t copySize = count * inputStride;
memcpy(mapPtr, input, copySize);
}
else
diff --git a/src/libANGLE/renderer/d3d/d3d9/VertexBuffer9.h b/src/libANGLE/renderer/d3d/d3d9/VertexBuffer9.h
index 983616f..90defb3 100644
--- a/src/libANGLE/renderer/d3d/d3d9/VertexBuffer9.h
+++ b/src/libANGLE/renderer/d3d/d3d9/VertexBuffer9.h
@@ -28,7 +28,7 @@
const gl::VertexBinding &binding,
GLenum currentValueType,
GLint start,
- GLsizei count,
+ size_t count,
GLsizei instances,
unsigned int offset,
const uint8_t *sourceData) override;