Skip to content

Commit 9ac563e

Browse files
ShabbyXmibrunin
authored andcommitted
[Backport] CVE-2024-7532: Out of bounds memory access in ANGLE (2/2)
Manual cherry-pick of patch originally reviewed on https://chromium-review.googlesource.com/c/angle/angle/+/5738743: Prune trivial infinite loops in WebGL contexts Bug: chromium:350528343 Reviewed-on: https://chromium-review.googlesource.com/c/angle/angle/+/5738743 Change-Id: I4be19c1ffe41fc86889b49b4a0e29d8bc9c940ec Reviewed-on: https://codereview.qt-project.org/c/qt/qtwebengine-chromium/+/582141 Reviewed-by: Allan Sandfeld Jensen <[email protected]>
1 parent a72a550 commit 9ac563e

File tree

5 files changed

+282
-0
lines changed

5 files changed

+282
-0
lines changed

chromium/third_party/angle/src/compiler.gni

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,8 @@ angle_translator_sources = [
149149
"src/compiler/translator/tree_ops/MonomorphizeUnsupportedFunctions.h",
150150
"src/compiler/translator/tree_ops/PruneEmptyCases.cpp",
151151
"src/compiler/translator/tree_ops/PruneEmptyCases.h",
152+
"src/compiler/translator/tree_ops/PruneInfiniteLoops.cpp",
153+
"src/compiler/translator/tree_ops/PruneInfiniteLoops.h",
152154
"src/compiler/translator/tree_ops/PruneNoOps.cpp",
153155
"src/compiler/translator/tree_ops/PruneNoOps.h",
154156
"src/compiler/translator/tree_ops/RecordConstantPrecision.cpp",

chromium/third_party/angle/src/compiler/translator/Common.h

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
#include <string>
1515
#include <string_view>
1616
#include <unordered_map>
17+
#include <unordered_set>
1718
#include <vector>
1819

1920
#include "common/angleutils.h"
@@ -111,6 +112,22 @@ class TUnorderedMap : public std::unordered_map<K, D, H, CMP, pool_allocator<std
111112
{}
112113
};
113114

115+
template <class K, class H = std::hash<K>, class CMP = std::equal_to<K>>
116+
class TUnorderedSet : public std::unordered_set<K, H, CMP, pool_allocator<K>>
117+
{
118+
public:
119+
POOL_ALLOCATOR_NEW_DELETE
120+
typedef pool_allocator<K> tAllocator;
121+
122+
TUnorderedSet() : std::unordered_set<K, H, CMP, tAllocator>() {}
123+
// use correct two-stage name lookup supported in gcc 3.4 and above
124+
TUnorderedSet(const tAllocator &a)
125+
: std::unordered_set<K, H, CMP, tAllocator>(
126+
std::unordered_set<K, H, CMP, tAllocator>::key_compare(),
127+
a)
128+
{}
129+
};
130+
114131
template <class K, class D, class CMP = std::less<K>>
115132
class TMap : public std::map<K, D, CMP, pool_allocator<std::pair<const K, D>>>
116133
{

chromium/third_party/angle/src/compiler/translator/Compiler.cpp

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
#include "compiler/translator/tree_ops/InitializeVariables.h"
4242
#include "compiler/translator/tree_ops/MonomorphizeUnsupportedFunctions.h"
4343
#include "compiler/translator/tree_ops/PruneEmptyCases.h"
44+
#include "compiler/translator/tree_ops/PruneInfiniteLoops.h"
4445
#include "compiler/translator/tree_ops/PruneNoOps.h"
4546
#include "compiler/translator/tree_ops/RemoveArrayLengthMethod.h"
4647
#include "compiler/translator/tree_ops/RemoveDynamicIndexing.h"
@@ -1014,6 +1015,15 @@ bool TCompiler::checkAndSimplifyAST(TIntermBlock *root,
10141015
return false;
10151016
}
10161017

1018+
// Attempt to reject shaders with infinite loops in WebGL contexts.
1019+
if (IsWebGLBasedSpec(mShaderSpec))
1020+
{
1021+
if (!PruneInfiniteLoops(this, root, &mSymbolTable))
1022+
{
1023+
return false;
1024+
}
1025+
}
1026+
10171027
if (compileOptions.rescopeGlobalVariables)
10181028
{
10191029
if (!RescopeGlobalVariables(*this, *root))
Lines changed: 229 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,229 @@
1+
//
2+
// Copyright 2024 The ANGLE Project Authors. All rights reserved.
3+
// Use of this source code is governed by a BSD-style license that can be
4+
// found in the LICENSE file.
5+
//
6+
// PruneInfiniteLoops.cpp: The PruneInfiniteLoops function prunes:
7+
//
8+
// 1. while (true) { ... }
9+
//
10+
// 2. bool variable = true; /* variable is never accessed */
11+
// while (variable) { ... }
12+
//
13+
// In all cases, the loop must not have EOpBreak or EOpReturn inside to be allowed to prune.
14+
//
15+
// In all cases, for (...; condition; ...) is treated the same as while (condition).
16+
//
17+
// It quickly gets error-prone when trying to detect more complicated cases. For example, it's
18+
// temping to reject any |while (expression involving variable with no side effects)| because that's
19+
// either while(true) or while(false), which is prune-able either way. That detects loops like
20+
// while(variable == false), while(variable + 2 != 4). But for example
21+
// while(coherent_buffer[variable]) may indeed not result in an infinite loop. For now, we stick to
22+
// the basic case.
23+
24+
#include "compiler/translator/tree_ops/PruneInfiniteLoops.h"
25+
26+
#include "compiler/translator/Symbol.h"
27+
#include "compiler/translator/tree_util/IntermTraverse.h"
28+
29+
#include <stack>
30+
31+
namespace sh
32+
{
33+
34+
namespace
35+
{
36+
using VariableSet = TUnorderedSet<const TVariable *>;
37+
38+
class FindConstantVariablesTraverser : public TIntermTraverser
39+
{
40+
public:
41+
FindConstantVariablesTraverser(TSymbolTable *symbolTable)
42+
: TIntermTraverser(true, false, false, symbolTable)
43+
{}
44+
45+
const VariableSet &getConstVariables() const { return mConstVariables; }
46+
47+
private:
48+
bool visitDeclaration(Visit, TIntermDeclaration *decl) override
49+
{
50+
// Initially, assume every variable is a constant
51+
TIntermSequence *sequence = decl->getSequence();
52+
for (TIntermNode *node : *sequence)
53+
{
54+
TIntermSymbol *symbol = node->getAsSymbolNode();
55+
if (symbol == nullptr)
56+
{
57+
TIntermBinary *assign = node->getAsBinaryNode();
58+
ASSERT(assign != nullptr && assign->getOp() == EOpInitialize);
59+
60+
symbol = assign->getLeft()->getAsSymbolNode();
61+
ASSERT(symbol != nullptr);
62+
}
63+
64+
ASSERT(mConstVariables.find(&symbol->variable()) == mConstVariables.end());
65+
mConstVariables.insert(&symbol->variable());
66+
}
67+
68+
return false;
69+
}
70+
71+
bool visitLoop(Visit visit, TIntermLoop *loop) override
72+
{
73+
ASSERT(visit == PreVisit);
74+
75+
// For simplicity, for now only consider conditions that are just |variable|. In that case,
76+
// the condition is not visited, so that `visitSymbol` doesn't consider this a write.
77+
if (loop->getInit() != nullptr)
78+
{
79+
loop->getInit()->traverse(this);
80+
}
81+
if (loop->getExpression() != nullptr)
82+
{
83+
loop->getExpression()->traverse(this);
84+
}
85+
loop->getBody()->traverse(this);
86+
87+
TIntermTyped *condition = loop->getCondition();
88+
if (condition != nullptr &&
89+
(condition->getAsSymbolNode() == nullptr || loop->getType() == ELoopDoWhile))
90+
{
91+
condition->traverse(this);
92+
}
93+
94+
return false;
95+
}
96+
97+
void visitSymbol(TIntermSymbol *symbol) override
98+
{
99+
// Assume write for simplicity. AST makes it difficult to tell if this is read or write.
100+
mConstVariables.erase(&symbol->variable());
101+
}
102+
103+
VariableSet mConstVariables;
104+
};
105+
106+
struct LoopStats
107+
{
108+
bool hasBreak = false;
109+
bool hasReturn = false;
110+
};
111+
112+
class PruneInfiniteLoopsTraverser : public TIntermTraverser
113+
{
114+
public:
115+
PruneInfiniteLoopsTraverser(TSymbolTable *symbolTable, const VariableSet &constVariables)
116+
: TIntermTraverser(true, false, false, symbolTable), mConstVariables(constVariables)
117+
{}
118+
119+
private:
120+
bool visitLoop(Visit visit, TIntermLoop *loop) override;
121+
bool visitSwitch(Visit visit, TIntermSwitch *node) override;
122+
bool visitBranch(Visit visit, TIntermBranch *node) override;
123+
124+
void onScopeBegin() { mLoopStats.push({}); }
125+
126+
void onScopeEnd()
127+
{
128+
// Propagate |hasReturn| up the stack, it escapes every loop.
129+
ASSERT(!mLoopStats.empty());
130+
bool hasReturn = mLoopStats.top().hasReturn;
131+
mLoopStats.pop();
132+
133+
if (!mLoopStats.empty())
134+
{
135+
mLoopStats.top().hasReturn = mLoopStats.top().hasReturn || hasReturn;
136+
}
137+
}
138+
139+
bool hasLoopEscape()
140+
{
141+
ASSERT(!mLoopStats.empty());
142+
return mLoopStats.top().hasBreak || mLoopStats.top().hasReturn;
143+
}
144+
145+
const VariableSet &mConstVariables;
146+
std::stack<LoopStats> mLoopStats;
147+
};
148+
149+
bool PruneInfiniteLoopsTraverser::visitLoop(Visit visit, TIntermLoop *loop)
150+
{
151+
onScopeBegin();
152+
153+
// Nothing in the init, condition or expression of loops can alter the control flow, just visit
154+
// the body.
155+
loop->getBody()->traverse(this);
156+
157+
// Prune the loop if it has no breaks or returns, it's not do-while, and the condition is a
158+
// constant variable.
159+
TIntermTyped *condition = loop->getCondition();
160+
TIntermConstantUnion *constCondition = condition ? condition->getAsConstantUnion() : nullptr;
161+
TIntermSymbol *conditionSymbol = condition ? loop->getCondition()->getAsSymbolNode() : nullptr;
162+
163+
const bool isConditionConstant =
164+
condition == nullptr || constCondition != nullptr ||
165+
(conditionSymbol != nullptr &&
166+
mConstVariables.find(&conditionSymbol->variable()) != mConstVariables.end());
167+
168+
if (isConditionConstant && loop->getType() != ELoopDoWhile && !hasLoopEscape())
169+
{
170+
mMultiReplacements.emplace_back(getParentNode()->getAsBlock(), loop, TIntermSequence{});
171+
}
172+
173+
onScopeEnd();
174+
175+
return false;
176+
}
177+
178+
bool PruneInfiniteLoopsTraverser::visitSwitch(Visit visit, TIntermSwitch *node)
179+
{
180+
// Insert a LoopStats node for switch, just so that breaks inside the switch are not considered
181+
// loop breaks.
182+
onScopeBegin();
183+
184+
// Nothing in the switch expression that can alter the control flow, just visit the body.
185+
node->getStatementList()->traverse(this);
186+
187+
onScopeEnd();
188+
189+
return false;
190+
}
191+
192+
bool PruneInfiniteLoopsTraverser::visitBranch(Visit visit, TIntermBranch *node)
193+
{
194+
if (!mLoopStats.empty())
195+
{
196+
switch (node->getFlowOp())
197+
{
198+
case EOpReturn:
199+
mLoopStats.top().hasReturn = true;
200+
break;
201+
case EOpBreak:
202+
mLoopStats.top().hasBreak = true;
203+
break;
204+
case EOpContinue:
205+
case EOpKill:
206+
// Kill and continue don't let control flow escape from the loop
207+
break;
208+
default:
209+
UNREACHABLE();
210+
}
211+
}
212+
213+
// Only possible child is the value of a return statement, which has no significance.
214+
return false;
215+
}
216+
} // namespace
217+
218+
bool PruneInfiniteLoops(TCompiler *compiler, TIntermBlock *root, TSymbolTable *symbolTable)
219+
{
220+
FindConstantVariablesTraverser constVarTransverser(symbolTable);
221+
root->traverse(&constVarTransverser);
222+
223+
PruneInfiniteLoopsTraverser pruneTraverser(symbolTable,
224+
constVarTransverser.getConstVariables());
225+
root->traverse(&pruneTraverser);
226+
return pruneTraverser.updateTree(compiler, root);
227+
}
228+
229+
} // namespace sh
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
//
2+
// Copyright 2024 The ANGLE Project Authors. All rights reserved.
3+
// Use of this source code is governed by a BSD-style license that can be
4+
// found in the LICENSE file.
5+
//
6+
// PruneInfiniteLoops.h: Attempts to remove infinite loops, used with WebGL contexts.
7+
8+
#ifndef COMPILER_TRANSLATOR_TREEOPS_PRUNEINFINITELOOPS_H_
9+
#define COMPILER_TRANSLATOR_TREEOPS_PRUNEINFINITELOOPS_H_
10+
11+
#include "common/angleutils.h"
12+
13+
namespace sh
14+
{
15+
class TCompiler;
16+
class TIntermBlock;
17+
class TSymbolTable;
18+
19+
[[nodiscard]] bool PruneInfiniteLoops(TCompiler *compiler,
20+
TIntermBlock *root,
21+
TSymbolTable *symbolTable);
22+
} // namespace sh
23+
24+
#endif // COMPILER_TRANSLATOR_TREEOPS_PRUNEINFINITELOOPS_H_

0 commit comments

Comments
 (0)