Skip to content
Permalink
Browse files
Cherry-pick ANGLE: D3D11: Skip blits if there is no intersection of d…
…est areas

https://bugs.webkit.org/show_bug.cgi?id=225190
<rdar://77084155>

Patch by Kimmo Kinnunen <kkinnunen@apple.com> on 2021-05-27
Reviewed by David Kilzer.

Cherry-pick ANGLE commit b574643ef28c92fcea5122dd7a72acb42a514eed
Fixes a security issue on D3D11.
Potential a correctness issue on some OpenGL drivers.
No effect on Metal, but the nodiscard part is still useful.

Upstream description:
D3D11: Skip blits if there is no intersection of dest areas

Blit11 would clip the destination rectangle with the destination size
but ignore the result. gl::ClipRectangle returns false when the
rectangles do not intersect at all, indicating the blit can be skipped.

This could lead to an out-of-bounds write to the GPU memory for the
destination texture.

Mark ClipRectangle as nodiscard to prevent future issues.
* src/libANGLE/angletypes.h:
* src/libANGLE/renderer/d3d/d3d11/Blit11.cpp:
* src/libANGLE/renderer/gl/FramebufferGL.cpp:
(rx::FramebufferGL::clipSrcRegion):
* src/libANGLE/renderer/metal/ContextMtl.mm:
(rx::ContextMtl::updateScissor):
* src/libANGLE/renderer/vulkan/ContextVk.cpp:
(rx::ContextVk::updateScissor):
* src/tests/gl_tests/BlitFramebufferANGLETest.cpp:
(TEST_P):

Canonical link: https://commits.webkit.org/238196@main
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@278152 268f45cc-cd09-0410-ab3c-d52691b4dbfc
  • Loading branch information
kkinnunen-apple authored and webkit-commit-queue committed May 27, 2021
1 parent c80f807 commit 26698c8ddb577fe33cd3dff931773a653d427ef6
@@ -1,3 +1,38 @@
2021-05-27 Kimmo Kinnunen <kkinnunen@apple.com>

Cherry-pick ANGLE: D3D11: Skip blits if there is no intersection of dest areas
https://bugs.webkit.org/show_bug.cgi?id=225190
<rdar://77084155>

Reviewed by David Kilzer.

Cherry-pick ANGLE commit b574643ef28c92fcea5122dd7a72acb42a514eed
Fixes a security issue on D3D11.
Potential a correctness issue on some OpenGL drivers.
No effect on Metal, but the nodiscard part is still useful.

Upstream description:
D3D11: Skip blits if there is no intersection of dest areas

Blit11 would clip the destination rectangle with the destination size
but ignore the result. gl::ClipRectangle returns false when the
rectangles do not intersect at all, indicating the blit can be skipped.

This could lead to an out-of-bounds write to the GPU memory for the
destination texture.

Mark ClipRectangle as nodiscard to prevent future issues.
* src/libANGLE/angletypes.h:
* src/libANGLE/renderer/d3d/d3d11/Blit11.cpp:
* src/libANGLE/renderer/gl/FramebufferGL.cpp:
(rx::FramebufferGL::clipSrcRegion):
* src/libANGLE/renderer/metal/ContextMtl.mm:
(rx::ContextMtl::updateScissor):
* src/libANGLE/renderer/vulkan/ContextVk.cpp:
(rx::ContextVk::updateScissor):
* src/tests/gl_tests/BlitFramebufferANGLETest.cpp:
(TEST_P):

2021-05-26 Don Olmstead <don.olmstead@sony.com>

[CMake] Support USE_ANGLE_EGL on additional platforms
@@ -84,7 +84,9 @@ bool operator==(const Rectangle &a, const Rectangle &b);
bool operator!=(const Rectangle &a, const Rectangle &b);

// Calculate the intersection of two rectangles. Returns false if the intersection is empty.
bool ClipRectangle(const Rectangle &source, const Rectangle &clip, Rectangle *intersection);
ANGLE_NO_DISCARD bool ClipRectangle(const Rectangle &source,
const Rectangle &clip,
Rectangle *intersection);
// Calculate the smallest rectangle that covers both rectangles. This rectangle may cover areas
// not covered by the two rectangles, for example in this situation:
//
@@ -141,7 +141,10 @@ void StretchedBlitNearest(const gl::Box &sourceArea,
uint8_t *destData)
{
gl::Rectangle clippedDestArea(destArea.x, destArea.y, destArea.width, destArea.height);
gl::ClipRectangle(clippedDestArea, clipRect, &clippedDestArea);
if (!gl::ClipRectangle(clippedDestArea, clipRect, &clippedDestArea))
{
return;
}

// Determine if entire rows can be copied at once instead of each individual pixel. There
// must be no out of bounds lookups, whole rows copies, and no scale.
@@ -1117,7 +1117,10 @@ angle::Result FramebufferGL::clipSrcRegion(const gl::Context *context,
// If pixels lying outside the read framebuffer, adjust src region
// and dst region to appropriate in-bounds regions respectively.
gl::Rectangle realSourceRegion;
ClipRectangle(bounds.sourceRegion, bounds.sourceBounds, &realSourceRegion);
if (!ClipRectangle(bounds.sourceRegion, bounds.sourceBounds, &realSourceRegion))
{
return angle::Result::Stop;
}
GLuint xOffset = realSourceRegion.x - bounds.sourceRegion.x;
GLuint yOffset = realSourceRegion.y - bounds.sourceRegion.y;

@@ -1820,7 +1820,10 @@ bool NeedToInvertDepthRange(float near, float far)

// Clip the render area to the viewport.
gl::Rectangle viewportClippedRenderArea;
gl::ClipRectangle(renderArea, glState.getViewport(), &viewportClippedRenderArea);
if (!gl::ClipRectangle(renderArea, glState.getViewport(), &viewportClippedRenderArea))
{
viewportClippedRenderArea = gl::Rectangle();
}

gl::Rectangle scissoredArea = ClipRectToScissor(getState(), viewportClippedRenderArea, false);
if (framebufferMtl->flipY())
@@ -2616,8 +2616,11 @@ void ContextVk::updateScissor(const gl::State &glState)

// Clip the render area to the viewport.
gl::Rectangle viewportClippedRenderArea;
gl::ClipRectangle(renderArea, getCorrectedViewport(glState.getViewport()),
&viewportClippedRenderArea);
if (!gl::ClipRectangle(renderArea, getCorrectedViewport(glState.getViewport()),
&viewportClippedRenderArea))
{
viewportClippedRenderArea = gl::Rectangle();
}

gl::Rectangle scissoredArea = ClipRectToScissor(getState(), viewportClippedRenderArea, false);
gl::Rectangle rotatedScissoredArea;
@@ -2437,6 +2437,30 @@ TEST_P(BlitFramebufferTest, BlitFramebufferSizeOverflow2)
EXPECT_GL_ERROR(GL_INVALID_VALUE);
}

// Test an edge case in D3D11 stencil blitting on the CPU that does not properly clip the
// destination regions
TEST_P(BlitFramebufferTest, BlitFramebufferStencilClipNoIntersection)
{
GLFramebuffer framebuffers[2];
glBindFramebuffer(GL_READ_FRAMEBUFFER, framebuffers[0]);
glBindFramebuffer(GL_DRAW_FRAMEBUFFER, framebuffers[1]);

GLRenderbuffer renderbuffers[2];
glBindRenderbuffer(GL_RENDERBUFFER, renderbuffers[0]);
glRenderbufferStorage(GL_RENDERBUFFER, GL_DEPTH24_STENCIL8, 4, 4);
glFramebufferRenderbuffer(GL_READ_FRAMEBUFFER, GL_STENCIL_ATTACHMENT, GL_RENDERBUFFER,
renderbuffers[0]);

glBindRenderbuffer(GL_RENDERBUFFER, renderbuffers[1]);
glRenderbufferStorage(GL_RENDERBUFFER, GL_DEPTH24_STENCIL8, 4, 4);
glFramebufferRenderbuffer(GL_DRAW_FRAMEBUFFER, GL_STENCIL_ATTACHMENT, GL_RENDERBUFFER,
renderbuffers[1]);

glBlitFramebuffer(0, 0, 4, 4, 1 << 24, 1 << 24, 1 << 25, 1 << 25, GL_STENCIL_BUFFER_BIT,
GL_NEAREST);
EXPECT_GL_NO_ERROR();
}

// Use this to select which configurations (e.g. which renderer, which GLES major version) these
// tests should be run against.
ANGLE_INSTANTIATE_TEST(BlitFramebufferANGLETest,

0 comments on commit 26698c8

Please sign in to comment.