github/electron
1
1
Fork 0
mirror of https://github.com/electron/electron.git synced 2026-04-10 03:01:51 -04:00
Code Issues Packages Projects Releases Wiki Activity

chore: cherry-pick ca32852e4f from angle (#33223)

Browse Source 
Co-authored-by: Electron Bot <electron@github.com>
This commit is contained in:
Pedro Pontes
2022-03-23 01:09:18 +01:00
committed by GitHub
parent 3ea5a0ba6b
commit d2abd19cff
3 changed files with 356 additions and 0 deletions
Download Patch File Download Diff File Expand all files Collapse all files

2
patches/angle/.patches
View File

@@ -7,4 +7,6 @@ cherry-pick-cc44ae61f37b.patch
m98_vulkan_fix_vkcmdresolveimage_extents.patch
m98_vulkan_fix_vkcmdresolveimage_offsets.patch
cherry-pick-49e8ff16f1fe.patch
m99_vulkan_prevent_out_of_bounds_read_in_divisor_emulation_path.patch
m99_vulkan_streamvertexdatawithdivisor_write_beyond_buffer_boundary.patch
m98_protect_against_deleting_a_current_xfb_buffer.patch

303
patches/angle/m99_vulkan_prevent_out_of_bounds_read_in_divisor_emulation_path.patch Normal file
View File

@@ -0,0 +1,303 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Roman Lavrov <romanl@google.com>
Date: Tue, 18 Jan 2022 20:05:55 +0000
Subject: M99: Vulkan: Prevent out of bounds read in divisor emulation path.
Split the replicated part of StreamVertexData out to
StreamVertexDataWithDivisor, there is only a partial argument
overlap between the two.
Bug: chromium:1285885
Change-Id: Ibf6ab3efc6b12b430b1d391c6ae61bd9668b4407
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3398816
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Shahbaz Youssefi <syoussefi@chromium.org>
Commit-Queue: Roman Lavrov <romanl@google.com>
(cherry picked from commit 5f0badf4541ba52659c937cfe9190d3735a76c10)
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3461414
diff --git a/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp b/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp
index ed416508cab06797a8c4d52e7ab982d538f57e3c..1ca486af3ff004e11aea82c391b4c504b569096c 100644
--- a/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp
+++ b/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp
@@ -81,34 +81,62 @@ angle::Result StreamVertexData(ContextVk *contextVk,
size_t destOffset,
size_t vertexCount,
size_t sourceStride,
- size_t destStride,
VertexCopyFunction vertexLoadFunction,
vk::BufferHelper **bufferOut,
- VkDeviceSize *bufferOffsetOut,
- uint32_t replicateCount)
+ VkDeviceSize *bufferOffsetOut)
{
uint8_t *dst = nullptr;
ANGLE_TRY(dynamicBuffer->allocate(contextVk, bytesToAllocate, &dst, nullptr, bufferOffsetOut,
nullptr));
*bufferOut = dynamicBuffer->getCurrentBuffer();
dst += destOffset;
- if (replicateCount == 1)
+ vertexLoadFunction(sourceData, sourceStride, vertexCount, dst);
+
+ ANGLE_TRY(dynamicBuffer->flush(contextVk));
+ return angle::Result::Continue;
+}
+
+angle::Result StreamVertexDataWithDivisor(ContextVk *contextVk,
+ vk::DynamicBuffer *dynamicBuffer,
+ const uint8_t *sourceData,
+ size_t bytesToAllocate,
+ size_t sourceStride,
+ size_t destStride,
+ VertexCopyFunction vertexLoadFunction,
+ vk::BufferHelper **bufferOut,
+ VkDeviceSize *bufferOffsetOut,
+ uint32_t divisor,
+ size_t numSrcVertices)
+{
+ uint8_t *dst = nullptr;
+ ANGLE_TRY(dynamicBuffer->allocate(contextVk, bytesToAllocate, &dst, nullptr, bufferOffsetOut,
+ nullptr));
+ *bufferOut = dynamicBuffer->getCurrentBuffer();
+
+ // Each source vertex is used `divisor` times before advancing. Clamp to avoid OOB reads.
+ size_t clampedSize = std::min(numSrcVertices * destStride * divisor, bytesToAllocate);
+
+ ASSERT(clampedSize % destStride == 0);
+ ASSERT(divisor > 0);
+
+ uint32_t sourceVertexUseCount = 0;
+ for (size_t dataCopied = 0; dataCopied < clampedSize; dataCopied += destStride, dst += destStride)
{
- vertexLoadFunction(sourceData, sourceStride, vertexCount, dst);
+ vertexLoadFunction(sourceData, sourceStride, 1, dst);
+ sourceVertexUseCount++;
+ if (sourceVertexUseCount == divisor)
+ {
+ sourceData += sourceStride;
+ sourceVertexUseCount = 0;
+ }
}
- else
+
+ // Satisfy robustness constraints (only if extension enabled)
+ if (contextVk->getExtensions().robustness)
{
- ASSERT(replicateCount > 1);
- uint32_t sourceRemainingCount = replicateCount - 1;
- for (size_t dataCopied = 0; dataCopied < bytesToAllocate;
- dataCopied += destStride, dst += destStride, sourceRemainingCount--)
+ if (clampedSize < bytesToAllocate)
{
- vertexLoadFunction(sourceData, sourceStride, 1, dst);
- if (sourceRemainingCount == 0)
- {
- sourceData += sourceStride;
- sourceRemainingCount = replicateCount;
- }
+ memset(dst + clampedSize, 0, bytesToAllocate - clampedSize);
}
}
@@ -124,6 +152,7 @@ size_t GetVertexCount(BufferVk *srcBuffer, const gl::VertexBinding &binding, uin
return 0;
// Count the last vertex. It may occupy less than a full stride.
+ // This is also correct if stride happens to be less than srcFormatSize.
size_t numVertices = 1;
bytes -= srcFormatSize;
@@ -452,8 +481,8 @@ angle::Result VertexArrayVk::convertVertexBufferCPU(ContextVk *contextVk,
ASSERT(GetVertexInputAlignment(vertexFormat, compressed) <= vk::kVertexBufferAlignment);
ANGLE_TRY(StreamVertexData(
contextVk, &conversion->data, srcBytes, numVertices * dstFormatSize, 0, numVertices,
- binding.getStride(), srcFormatSize, vertexFormat.getVertexLoadFunction(compressed),
- &mCurrentArrayBuffers[attribIndex], &conversion->lastAllocationOffset, 1));
+ binding.getStride(), vertexFormat.getVertexLoadFunction(compressed),
+ &mCurrentArrayBuffers[attribIndex], &conversion->lastAllocationOffset));
ANGLE_TRY(srcBuffer->unmapImpl(contextVk));
ASSERT(conversion->dirty);
@@ -808,28 +837,41 @@ angle::Result VertexArrayVk::updateStreamedAttribs(const gl::Context *context,
// Instanced attrib
if (divisor > renderer->getMaxVertexAttribDivisor())
{
- // Emulated attrib
- BufferVk *bufferVk = nullptr;
+ // Divisor will be set to 1 & so update buffer to have 1 attrib per instance
+ size_t bytesToAllocate = instanceCount * stride;
+
if (binding.getBuffer().get() != nullptr)
{
// Map buffer to expand attribs for divisor emulation
- bufferVk = vk::GetImpl(binding.getBuffer().get());
- void *buffSrc = nullptr;
+ BufferVk *bufferVk = vk::GetImpl(binding.getBuffer().get());
+ void *buffSrc = nullptr;
ANGLE_TRY(bufferVk->mapImpl(contextVk, &buffSrc));
src = reinterpret_cast<const uint8_t *>(buffSrc) + binding.getOffset();
- }
- // Divisor will be set to 1 & so update buffer to have 1 attrib per instance
- size_t bytesToAllocate = instanceCount * stride;
- ANGLE_TRY(StreamVertexData(contextVk, &mDynamicVertexData, src, bytesToAllocate, 0,
- instanceCount, binding.getStride(), stride,
- vertexFormat.vertexLoadFunction,
- &mCurrentArrayBuffers[attribIndex],
- &mCurrentArrayBufferOffsets[attribIndex], divisor));
- if (bufferVk)
- {
+ uint32_t srcAttributeSize =
+ static_cast<uint32_t>(ComputeVertexAttributeTypeSize(attrib));
+
+ size_t numVertices = GetVertexCount(bufferVk, binding, srcAttributeSize);
+
+ ANGLE_TRY(StreamVertexDataWithDivisor(
+ contextVk, &mDynamicVertexData, src, bytesToAllocate, binding.getStride(),
+ stride, vertexFormat.vertexLoadFunction,
+ &mCurrentArrayBuffers[attribIndex],
+ &mCurrentArrayBufferOffsets[attribIndex], divisor,
+ numVertices));
+
ANGLE_TRY(bufferVk->unmapImpl(contextVk));
}
+ else
+ {
+ size_t numVertices = instanceCount;
+ ANGLE_TRY(StreamVertexDataWithDivisor(
+ contextVk, &mDynamicVertexData, src, bytesToAllocate, binding.getStride(),
+ stride, vertexFormat.vertexLoadFunction,
+ &mCurrentArrayBuffers[attribIndex],
+ &mCurrentArrayBufferOffsets[attribIndex], divisor,
+ numVertices));
+ }
}
else
{
@@ -838,10 +880,10 @@ angle::Result VertexArrayVk::updateStreamedAttribs(const gl::Context *context,
size_t bytesToAllocate = count * stride;
ANGLE_TRY(StreamVertexData(contextVk, &mDynamicVertexData, src, bytesToAllocate, 0,
- count, binding.getStride(), stride,
+ count, binding.getStride(),
vertexFormat.vertexLoadFunction,
&mCurrentArrayBuffers[attribIndex],
- &mCurrentArrayBufferOffsets[attribIndex], 1));
+ &mCurrentArrayBufferOffsets[attribIndex]));
}
}
else
@@ -856,8 +898,8 @@ angle::Result VertexArrayVk::updateStreamedAttribs(const gl::Context *context,
ANGLE_TRY(StreamVertexData(
contextVk, &mDynamicVertexData, src, bytesToAllocate, destOffset, vertexCount,
- binding.getStride(), stride, vertexFormat.vertexLoadFunction,
- &mCurrentArrayBuffers[attribIndex], &mCurrentArrayBufferOffsets[attribIndex], 1));
+ binding.getStride(), vertexFormat.vertexLoadFunction,
+ &mCurrentArrayBuffers[attribIndex], &mCurrentArrayBufferOffsets[attribIndex]));
}
mCurrentArrayBufferHandles[attribIndex] =
diff --git a/src/tests/gl_tests/RobustBufferAccessBehaviorTest.cpp b/src/tests/gl_tests/RobustBufferAccessBehaviorTest.cpp
index 08917a2de3385eb853bced1dd576aff46f34f709..db708b0a2a3ce90a370b43786af6884b2f992bef 100644
--- a/src/tests/gl_tests/RobustBufferAccessBehaviorTest.cpp
+++ b/src/tests/gl_tests/RobustBufferAccessBehaviorTest.cpp
@@ -564,6 +564,98 @@ TEST_P(RobustBufferAccessBehaviorTest, DynamicBuffer)
}
}
+// Tests out of bounds read by divisor emulation due to a user-provided offset.
+// Adapted from https://crbug.com/1285885.
+TEST_P(RobustBufferAccessBehaviorTest, IndexOutOfBounds)
+{
+ ANGLE_SKIP_TEST_IF(!initExtension());
+
+ constexpr char kVS[] = R"(precision highp float;
+attribute vec4 a_position;
+void main(void) {
+ gl_Position = a_position;
+})";
+
+ constexpr char kFS[] = R"(precision highp float;
+uniform sampler2D oTexture;
+uniform float oColor[3];
+void main(void) {
+ gl_FragData[0] = texture2DProj(oTexture, vec3(0.1,0.1,0.1));
+})";
+
+ GLfloat singleFloat = 1.0f;
+
+ GLBuffer buf;
+ glBindBuffer(GL_ARRAY_BUFFER, buf);
+ glBufferData(GL_ARRAY_BUFFER, 4, &singleFloat, GL_STATIC_DRAW);
+
+ ANGLE_GL_PROGRAM(program, kVS, kFS);
+ glBindAttribLocation(program, 0, "a_position");
+ glLinkProgram(program);
+ ASSERT_TRUE(CheckLinkStatusAndReturnProgram(program, true));
+
+ glEnableVertexAttribArray(0);
+
+ // Trying to exceed renderer->getMaxVertexAttribDivisor()
+ GLuint constexpr kDivisor = 4096;
+ glVertexAttribDivisor(0, kDivisor);
+
+ size_t outOfBoundsOffset = 0x50000000;
+ glVertexAttribPointer(0, 1, GL_FLOAT, false, 8, reinterpret_cast<void *>(outOfBoundsOffset));
+
+ glUseProgram(program);
+
+ glDrawArrays(GL_TRIANGLES, 0, 32);
+
+ // No assertions, just checking for crashes.
+}
+
+// Similar to the test above but index is first within bounds then goes out of bounds.
+TEST_P(RobustBufferAccessBehaviorTest, IndexGoingOutOfBounds)
+{
+ ANGLE_SKIP_TEST_IF(!initExtension());
+
+ constexpr char kVS[] = R"(precision highp float;
+attribute vec4 a_position;
+void main(void) {
+ gl_Position = a_position;
+})";
+
+ constexpr char kFS[] = R"(precision highp float;
+uniform sampler2D oTexture;
+uniform float oColor[3];
+void main(void) {
+ gl_FragData[0] = texture2DProj(oTexture, vec3(0.1,0.1,0.1));
+})";
+
+ GLBuffer buf;
+ glBindBuffer(GL_ARRAY_BUFFER, buf);
+ std::array<GLfloat, 2> buffer = {{0.2f, 0.2f}};
+ glBufferData(GL_ARRAY_BUFFER, sizeof(GLfloat) * buffer.size(), buffer.data(), GL_STATIC_DRAW);
+
+ ANGLE_GL_PROGRAM(program, kVS, kFS);
+ glBindAttribLocation(program, 0, "a_position");
+ glLinkProgram(program);
+ ASSERT_TRUE(CheckLinkStatusAndReturnProgram(program, true));
+
+ glEnableVertexAttribArray(0);
+
+ // Trying to exceed renderer->getMaxVertexAttribDivisor()
+ GLuint constexpr kDivisor = 4096;
+ glVertexAttribDivisor(0, kDivisor);
+
+ // 6 bytes remaining in the buffer from offset so only a single vertex can be read
+ glVertexAttribPointer(0, 1, GL_FLOAT, false, 8, reinterpret_cast<void *>(2));
+
+ glUseProgram(program);
+
+ // Each vertex is read `kDivisor` times so the last read goes out of bounds
+ GLsizei instanceCount = kDivisor + 1;
+ glDrawArraysInstanced(GL_TRIANGLES, 0, 32, instanceCount);
+
+ // No assertions, just checking for crashes.
+}
+
ANGLE_INSTANTIATE_TEST_ES2_AND_ES3_AND_ES31(RobustBufferAccessBehaviorTest);
} // namespace

51
patches/angle/m99_vulkan_streamvertexdatawithdivisor_write_beyond_buffer_boundary.patch Normal file
View File

@@ -0,0 +1,51 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Charlie Lao <cclao@google.com>
Date: Mon, 7 Feb 2022 15:08:15 -0800
Subject: M99: Vulkan: StreamVertexDataWithDivisor write beyond buffer boundary
StreamVertexDataWithDivisor() function is advancing dst with dstStride,
but then later on it is treating dst as if it never advanced, thus
result in write out of buffer boundary. This was hidden by VMA's memory
suballocation, which means it may result in some rendering artifacts.
Bug: angleproject:6990
Change-Id: Ic91e917cedd247dfe85b12a69ae26b21b7a4e67a
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3445528
Reviewed-by: Roman Lavrov <romanl@google.com>
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Commit-Queue: Charlie Lao <cclao@google.com>
(cherry picked from commit 5204587698099207ce8ae70779ef44ffae877996)
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3461417
Reviewed-by: Charlie Lao <cclao@google.com>
Commit-Queue: Roman Lavrov <romanl@google.com>
diff --git a/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp b/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp
index 1ca486af3ff004e11aea82c391b4c504b569096c..93378c4b24495872405fc06ea01e15254229ab63 100644
--- a/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp
+++ b/src/libANGLE/renderer/vulkan/VertexArrayVk.cpp
@@ -120,7 +120,7 @@ angle::Result StreamVertexDataWithDivisor(ContextVk *contextVk,
ASSERT(divisor > 0);
uint32_t sourceVertexUseCount = 0;
- for (size_t dataCopied = 0; dataCopied < clampedSize; dataCopied += destStride, dst += destStride)
+ for (size_t dataCopied = 0; dataCopied < clampedSize; dataCopied += destStride)
{
vertexLoadFunction(sourceData, sourceStride, 1, dst);
sourceVertexUseCount++;
@@ -129,6 +129,7 @@ angle::Result StreamVertexDataWithDivisor(ContextVk *contextVk,
sourceData += sourceStride;
sourceVertexUseCount = 0;
}
+ dst += destStride;
}
// Satisfy robustness constraints (only if extension enabled)
@@ -136,7 +137,7 @@ angle::Result StreamVertexDataWithDivisor(ContextVk *contextVk,
{
if (clampedSize < bytesToAllocate)
{
- memset(dst + clampedSize, 0, bytesToAllocate - clampedSize);
+ memset(dst, 0, bytesToAllocate - clampedSize);
}
}