Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore: cherry-pick e8cb0e7aa32 from angle #31235

Merged
merged 1 commit into from
Oct 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions patches/angle/.patches
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
fix_integer_overflow_in_blocklayoutencoder.patch
125 changes: 125 additions & 0 deletions patches/angle/fix_integer_overflow_in_blocklayoutencoder.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,125 @@
From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Alexis Hetu <sugoi@google.com>
Date: Wed, 15 Sep 2021 13:40:28 -0400
Subject: Fix integer overflow in BlockLayoutEncoder
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

BlockLayoutEncoder::mCurrentOffset's computation had the
possibility of causing integer overflows in multiple places,
so this CL adds CheckedNumeric variables in a number of
these occurrences in order to prevent integer overflows and
causing issues.

The issue in this case was an integer overflow causing the
code in ValidateTypeSizeLimitations.cpp to use an invalid
result from "layoutEncoder.getCurrentOffset()", which ended
up compiling a shader which would later cause an OOM error.

Bug: chromium:1248665
Change-Id: I688d669f21c6dc2957e43bdf91f8f8f08180a6f7
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3163356
Reviewed-by: Jamie Madill <jmadill@chromium.org>
Reviewed-by: Kenneth Russell <kbr@chromium.org>
Reviewed-by: Geoff Lang <geofflang@chromium.org>
Commit-Queue: Alexis Hétu <sugoi@chromium.org>
(cherry picked from commit 158ef351fc8b827c201e056a8ddba50fd4235671)
Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/3194392

diff --git a/src/compiler/translator/blocklayout.cpp b/src/compiler/translator/blocklayout.cpp
index 0539923bc75f5b88228dcb387c8aba4c701edc1b..1e6b143a1e1ee9a127b71685febd36416a8840f6 100644
--- a/src/compiler/translator/blocklayout.cpp
+++ b/src/compiler/translator/blocklayout.cpp
@@ -199,6 +199,13 @@ BlockMemberInfo BlockLayoutEncoder::encodeType(GLenum type,
return memberInfo;
}

+size_t BlockLayoutEncoder::getCurrentOffset() const
+{
+ angle::base::CheckedNumeric<size_t> checkedOffset(mCurrentOffset);
+ checkedOffset *= kBytesPerComponent;
+ return checkedOffset.ValueOrDefault(std::numeric_limits<size_t>::max());
+}
+
size_t BlockLayoutEncoder::getShaderVariableSize(const ShaderVariable &structVar, bool isRowMajor)
{
size_t currentOffset = mCurrentOffset;
@@ -226,7 +233,13 @@ size_t BlockLayoutEncoder::GetBlockRegisterElement(const BlockMemberInfo &info)

void BlockLayoutEncoder::align(size_t baseAlignment)
{
- mCurrentOffset = rx::roundUp<size_t>(mCurrentOffset, baseAlignment);
+ angle::base::CheckedNumeric<size_t> checkedOffset(mCurrentOffset);
+ checkedOffset += baseAlignment;
+ checkedOffset -= 1;
+ angle::base::CheckedNumeric<size_t> checkedAlignmentOffset = checkedOffset;
+ checkedAlignmentOffset %= baseAlignment;
+ checkedOffset -= checkedAlignmentOffset.ValueOrDefault(std::numeric_limits<size_t>::max());
+ mCurrentOffset = checkedOffset.ValueOrDefault(std::numeric_limits<size_t>::max());
}

// StubBlockEncoder implementation.
@@ -289,7 +302,7 @@ void Std140BlockEncoder::getBlockLayoutInfo(GLenum type,
baseAlignment = ComponentAlignment(numComponents);
}

- mCurrentOffset = rx::roundUp(mCurrentOffset, baseAlignment);
+ align(baseAlignment);

*matrixStrideOut = matrixStride;
*arrayStrideOut = arrayStride;
@@ -303,16 +316,23 @@ void Std140BlockEncoder::advanceOffset(GLenum type,
{
if (!arraySizes.empty())
{
- mCurrentOffset += arrayStride * gl::ArraySizeProduct(arraySizes);
+ angle::base::CheckedNumeric<size_t> checkedOffset(arrayStride);
+ checkedOffset *= gl::ArraySizeProduct(arraySizes);
+ checkedOffset += mCurrentOffset;
+ mCurrentOffset = checkedOffset.ValueOrDefault(std::numeric_limits<size_t>::max());
}
else if (gl::IsMatrixType(type))
{
- const int numRegisters = gl::MatrixRegisterCount(type, isRowMajorMatrix);
- mCurrentOffset += matrixStride * numRegisters;
+ angle::base::CheckedNumeric<size_t> checkedOffset(matrixStride);
+ checkedOffset *= gl::MatrixRegisterCount(type, isRowMajorMatrix);
+ checkedOffset += mCurrentOffset;
+ mCurrentOffset = checkedOffset.ValueOrDefault(std::numeric_limits<size_t>::max());
}
else
{
- mCurrentOffset += gl::VariableComponentCount(type);
+ angle::base::CheckedNumeric<size_t> checkedOffset(mCurrentOffset);
+ checkedOffset += gl::VariableComponentCount(type);
+ mCurrentOffset = checkedOffset.ValueOrDefault(std::numeric_limits<size_t>::max());
}
}

diff --git a/src/compiler/translator/blocklayout.h b/src/compiler/translator/blocklayout.h
index 726d76fa178f77a978ff2c82ec20fb8f1ad03f0b..ff90a2487830b365697ce41d660a689857b75319 100644
--- a/src/compiler/translator/blocklayout.h
+++ b/src/compiler/translator/blocklayout.h
@@ -80,7 +80,7 @@ class BlockLayoutEncoder
const std::vector<unsigned int> &arraySizes,
bool isRowMajorMatrix);

- size_t getCurrentOffset() const { return mCurrentOffset * kBytesPerComponent; }
+ size_t getCurrentOffset() const;
size_t getShaderVariableSize(const ShaderVariable &structVar, bool isRowMajor);

// Called when entering/exiting a structure variable.
diff --git a/src/tests/gl_tests/WebGLCompatibilityTest.cpp b/src/tests/gl_tests/WebGLCompatibilityTest.cpp
index f57e3c6e012788666186b532fad6fc2d07d9d4b7..35cadfd7312ab1cd77c7d153c6a25b5cd47d9fe8 100644
--- a/src/tests/gl_tests/WebGLCompatibilityTest.cpp
+++ b/src/tests/gl_tests/WebGLCompatibilityTest.cpp
@@ -5047,7 +5047,7 @@ void main()

constexpr char kVSArrayMuchTooLarge[] =
R"(varying vec4 color;
-const int array_size = 795418649;
+const int array_size = 556007917;
void main()
{
mat2 array[array_size];
4 changes: 3 additions & 1 deletion patches/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,7 @@

"src/electron/patches/Mantle": "src/third_party/squirrel.mac/vendor/Mantle",

"src/electron/patches/ReactiveObjC": "src/third_party/squirrel.mac/vendor/ReactiveObjC"
"src/electron/patches/ReactiveObjC": "src/third_party/squirrel.mac/vendor/ReactiveObjC",

"src/electron/patches/angle": "src/third_party/angle"
}