Fixed all OOB accesses in VertexProgram and PixelProgram
A lot of arrays in VertexProgram and PixelProgram have fixed sizes,
so programs that have more nested loops or ifs or deeper call stacks
can cause OOB accesses, which causes security issues in Chromium.
Index clamping was added to prevent any OOB memory accesses here.
This could eventually be fixed properly by first verifying these sizes
and giving shader compile errors when these limits are exceeded.
Bug chromium:915197 chromium:915206 chromium:915218 b/116373662
Change-Id: I2d0710ed0ce6585f139cba49d5b5d8c909ae6391
Reviewed-on: https://swiftshader-review.googlesource.com/c/23568
Tested-by: Alexis Hétu <sugoi@google.com>
Reviewed-by: Corentin Wallez <cwallez@google.com>
diff --git a/src/Common/Types.hpp b/src/Common/Types.hpp
index 837df46..fac6d36 100644
--- a/src/Common/Types.hpp
+++ b/src/Common/Types.hpp
@@ -15,6 +15,7 @@
#ifndef sw_Types_hpp
#define sw_Types_hpp
+#include <assert.h>
#include <limits>
#include <type_traits>
@@ -151,6 +152,46 @@
return v;
}
+ template <int limit> class BoundedIndex
+ {
+ public:
+ BoundedIndex(int index) : index(index) {}
+
+ inline int operator++(int) { return index++; }
+ inline int operator--(int) { return index--; }
+ inline void operator=(int i) { index = i; }
+
+ inline bool operator==(int i) { return index == i; }
+ inline bool operator!=(int i) { return index != i; }
+ inline bool operator<(int i) { return index < i; }
+ inline bool operator>(int i) { return index > i; }
+ inline bool operator<=(int i) { return index <= i; }
+ inline bool operator>=(int i) { return index >= i; }
+
+ inline operator int()
+ {
+ if(index < 0)
+ {
+#if !defined(NDEBUG) || defined(DCHECK_ALWAYS_ON)
+ assert(false);
+#endif
+ return 0;
+ }
+ else if(index >= limit)
+ {
+#if !defined(NDEBUG) || defined(DCHECK_ALWAYS_ON)
+ assert(false);
+#endif
+ return limit - 1;
+ }
+
+ return index;
+ }
+
+ private:
+ int index = 0;
+ };
+
// The OFFSET macro is a generalization of the offsetof() macro defined in <cstddef>.
// It allows e.g. getting the offset of array elements, even when indexed dynamically.
// We cast the address '32' and subtract it again, because null-dereference is undefined behavior.
diff --git a/src/Main/Config.hpp b/src/Main/Config.hpp
index 017e38b..d2ba6ff 100644
--- a/src/Main/Config.hpp
+++ b/src/Main/Config.hpp
@@ -97,6 +97,11 @@
MAX_TEXTURE_LOD = MIPMAP_LEVELS - 2, // Trilinear accesses lod+1
RENDERTARGETS = 8,
NUM_TEMPORARY_REGISTERS = 4096,
+ MAX_SHADER_CALL_SITES = 2048,
+ MAX_SHADER_NESTED_LOOPS = 4,
+ MAX_SHADER_NESTED_IFS = 24 + 24,
+ MAX_SHADER_CALL_STACK_SIZE = 16,
+ MAX_SHADER_ENABLE_STACK_SIZE = 1 + 24,
};
}
diff --git a/src/Shader/PixelProgram.cpp b/src/Shader/PixelProgram.cpp
index 671b0ef..bc879a3 100644
--- a/src/Shader/PixelProgram.cpp
+++ b/src/Shader/PixelProgram.cpp
@@ -838,7 +838,7 @@
return Int4(0xFFFFFFFF);
}
- Int4 enable = instruction->analysisBranch ? Int4(enableStack[enableIndex]) : Int4(0xFFFFFFFF);
+ Int4 enable = instruction->analysisBranch ? Int4(enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))]) : Int4(0xFFFFFFFF);
if(shader->containsBreakInstruction() && instruction->analysisBreak)
{
@@ -1353,7 +1353,7 @@
void PixelProgram::BREAK()
{
- enableBreak = enableBreak & ~enableStack[enableIndex];
+ enableBreak = enableBreak & ~enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))];
}
void PixelProgram::BREAKC(Vector4f &src0, Vector4f &src1, Control control)
@@ -1389,14 +1389,14 @@
void PixelProgram::BREAK(Int4 &condition)
{
- condition &= enableStack[enableIndex];
+ condition &= enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))];
enableBreak = enableBreak & ~condition;
}
void PixelProgram::CONTINUE()
{
- enableContinue = enableContinue & ~enableStack[enableIndex];
+ enableContinue = enableContinue & ~enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))];
}
void PixelProgram::TEST()
@@ -1419,7 +1419,7 @@
if(callRetBlock[labelIndex].size() > 1)
{
- callStack[stackIndex++] = UInt(callSiteIndex);
+ callStack[Min(stackIndex++, Int(MAX_SHADER_CALL_STACK_SIZE))] = UInt(callSiteIndex);
}
Int4 restoreLeave = enableLeave;
@@ -1459,7 +1459,7 @@
if(callRetBlock[labelIndex].size() > 1)
{
- callStack[stackIndex++] = UInt(callSiteIndex);
+ callStack[Min(stackIndex++, Int(MAX_SHADER_CALL_STACK_SIZE))] = UInt(callSiteIndex);
}
Int4 restoreLeave = enableLeave;
@@ -1479,7 +1479,7 @@
condition = ~condition;
}
- condition &= enableStack[enableIndex];
+ condition &= enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))];
if(!labelBlock[labelIndex])
{
@@ -1488,11 +1488,11 @@
if(callRetBlock[labelIndex].size() > 1)
{
- callStack[stackIndex++] = UInt(callSiteIndex);
+ callStack[Min(stackIndex++, Int(MAX_SHADER_CALL_STACK_SIZE))] = UInt(callSiteIndex);
}
enableIndex++;
- enableStack[enableIndex] = condition;
+ enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))] = condition;
Int4 restoreLeave = enableLeave;
Bool notAllFalse = SignMask(condition) != 0;
@@ -1512,12 +1512,12 @@
if(isConditionalIf[ifDepth])
{
- Int4 condition = ~enableStack[enableIndex] & enableStack[enableIndex - 1];
+ Int4 condition = ~enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))] & enableStack[Min(enableIndex - 1, Int(MAX_SHADER_ENABLE_STACK_SIZE))];
Bool notAllFalse = SignMask(condition) != 0;
branch(notAllFalse, falseBlock, endBlock);
- enableStack[enableIndex] = ~enableStack[enableIndex] & enableStack[enableIndex - 1];
+ enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))] = ~enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))] & enableStack[Min(enableIndex - 1, Int(MAX_SHADER_ENABLE_STACK_SIZE))];
}
else
{
@@ -1671,10 +1671,10 @@
void PixelProgram::IF(Int4 &condition)
{
- condition &= enableStack[enableIndex];
+ condition &= enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))];
enableIndex++;
- enableStack[enableIndex] = condition;
+ enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))] = condition;
BasicBlock *trueBlock = Nucleus::createBasicBlock();
BasicBlock *falseBlock = Nucleus::createBasicBlock();
@@ -1778,10 +1778,10 @@
const Vector4f &src = fetchRegister(temporaryRegister);
Int4 condition = As<Int4>(src.x);
- condition &= enableStack[enableIndex - 1];
+ condition &= enableStack[Min(enableIndex - 1, Int(MAX_SHADER_ENABLE_STACK_SIZE))];
if(shader->containsLeaveInstruction()) condition &= enableLeave;
if(shader->containsBreakInstruction()) condition &= enableBreak;
- enableStack[enableIndex] = condition;
+ enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))] = condition;
Bool notAllFalse = SignMask(condition) != 0;
branch(notAllFalse, loopBlock, endBlock);
@@ -1854,7 +1854,7 @@
void PixelProgram::LEAVE()
{
- enableLeave = enableLeave & ~enableStack[enableIndex];
+ enableLeave = enableLeave & ~enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))];
// FIXME: Return from function if all instances left
// FIXME: Use enableLeave in other control-flow constructs
diff --git a/src/Shader/PixelProgram.hpp b/src/Shader/PixelProgram.hpp
index 3c3a06f..b8548d7 100644
--- a/src/Shader/PixelProgram.hpp
+++ b/src/Shader/PixelProgram.hpp
@@ -26,7 +26,7 @@
PixelProgram(const PixelProcessor::State &state, const PixelShader *shader) :
PixelRoutine(state, shader), r(shader->indirectAddressableTemporaries)
{
- for(int i = 0; i < 2048; ++i)
+ for(int i = 0; i < MAX_SHADER_CALL_SITES; ++i)
{
labelBlock[i] = 0;
}
@@ -67,17 +67,17 @@
// DX9 specific variables
Vector4f p0;
- Array<Int, 4> aL;
- Array<Int, 4> increment;
- Array<Int, 4> iteration;
+ Array<Int, MAX_SHADER_NESTED_LOOPS> aL;
+ Array<Int, MAX_SHADER_NESTED_LOOPS> increment;
+ Array<Int, MAX_SHADER_NESTED_LOOPS> iteration;
Int loopDepth; // FIXME: Add support for switch
Int stackIndex; // FIXME: Inc/decrement callStack
- Array<UInt, 16> callStack;
+ Array<UInt, MAX_SHADER_CALL_STACK_SIZE> callStack;
// Per pixel based on conditions reached
Int enableIndex;
- Array<Int4, 1 + 24> enableStack;
+ Array<Int4, MAX_SHADER_ENABLE_STACK_SIZE> enableStack;
Int4 enableBreak;
Int4 enableContinue;
Int4 enableLeave;
@@ -153,18 +153,18 @@
void RET();
void LEAVE();
- int ifDepth = 0;
- int loopRepDepth = 0;
- int currentLabel = -1;
+ BoundedIndex<MAX_SHADER_NESTED_IFS> ifDepth = 0;
+ BoundedIndex<MAX_SHADER_NESTED_LOOPS> loopRepDepth = 0;
+ BoundedIndex<MAX_SHADER_CALL_SITES> currentLabel = -1;
bool scalar = false;
- BasicBlock *ifFalseBlock[24 + 24];
- BasicBlock *loopRepTestBlock[4];
- BasicBlock *loopRepEndBlock[4];
- BasicBlock *labelBlock[2048];
- std::vector<BasicBlock*> callRetBlock[2048];
+ BasicBlock *ifFalseBlock[MAX_SHADER_NESTED_IFS];
+ BasicBlock *loopRepTestBlock[MAX_SHADER_NESTED_LOOPS];
+ BasicBlock *loopRepEndBlock[MAX_SHADER_NESTED_LOOPS];
+ BasicBlock *labelBlock[MAX_SHADER_CALL_SITES];
+ std::vector<BasicBlock*> callRetBlock[MAX_SHADER_CALL_SITES];
BasicBlock *returnBlock;
- bool isConditionalIf[24 + 24];
+ bool isConditionalIf[MAX_SHADER_NESTED_IFS];
std::vector<Int4> restoreContinue;
};
}
diff --git a/src/Shader/Shader.cpp b/src/Shader/Shader.cpp
index 36192c9..0e06703 100644
--- a/src/Shader/Shader.cpp
+++ b/src/Shader/Shader.cpp
@@ -1877,13 +1877,13 @@
// This is used to know what basic block to return to.
void Shader::analyzeCallSites()
{
- int callSiteIndex[2048] = {0};
+ int callSiteIndex[MAX_SHADER_CALL_SITES] = {0};
for(auto &inst : instruction)
{
if(inst->opcode == OPCODE_CALL || inst->opcode == OPCODE_CALLNZ)
{
- int label = inst->dst.label;
+ int label = sw::min(inst->dst.label, unsigned int(MAX_SHADER_CALL_SITES));
inst->dst.callSite = callSiteIndex[label]++;
}
diff --git a/src/Shader/VertexProgram.cpp b/src/Shader/VertexProgram.cpp
index bde7531..05f8dfb 100644
--- a/src/Shader/VertexProgram.cpp
+++ b/src/Shader/VertexProgram.cpp
@@ -26,7 +26,7 @@
VertexProgram::VertexProgram(const VertexProcessor::State &state, const VertexShader *shader)
: VertexRoutine(state, shader), shader(shader), r(shader->indirectAddressableTemporaries)
{
- for(int i = 0; i < 2048; i++)
+ for(int i = 0; i < MAX_SHADER_CALL_SITES; i++)
{
labelBlock[i] = 0;
}
@@ -979,7 +979,7 @@
return Int4(0xFFFFFFFF);
}
- Int4 enable = instruction->analysisBranch ? Int4(enableStack[enableIndex]) : Int4(0xFFFFFFFF);
+ Int4 enable = instruction->analysisBranch ? Int4(enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))]) : Int4(0xFFFFFFFF);
if(shader->containsBreakInstruction() && instruction->analysisBreak)
{
@@ -1058,7 +1058,7 @@
void VertexProgram::BREAK()
{
- enableBreak = enableBreak & ~enableStack[enableIndex];
+ enableBreak = enableBreak & ~enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))];
}
void VertexProgram::BREAKC(Vector4f &src0, Vector4f &src1, Control control)
@@ -1094,14 +1094,14 @@
void VertexProgram::BREAK(Int4 &condition)
{
- condition &= enableStack[enableIndex];
+ condition &= enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))];
enableBreak = enableBreak & ~condition;
}
void VertexProgram::CONTINUE()
{
- enableContinue = enableContinue & ~enableStack[enableIndex];
+ enableContinue = enableContinue & ~enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))];
}
void VertexProgram::TEST()
@@ -1124,7 +1124,7 @@
if(callRetBlock[labelIndex].size() > 1)
{
- callStack[stackIndex++] = UInt(callSiteIndex);
+ callStack[Min(stackIndex++, Int(MAX_SHADER_CALL_STACK_SIZE))] = UInt(callSiteIndex);
}
Int4 restoreLeave = enableLeave;
@@ -1164,7 +1164,7 @@
if(callRetBlock[labelIndex].size() > 1)
{
- callStack[stackIndex++] = UInt(callSiteIndex);
+ callStack[Min(stackIndex++, Int(MAX_SHADER_CALL_STACK_SIZE))] = UInt(callSiteIndex);
}
Int4 restoreLeave = enableLeave;
@@ -1184,7 +1184,7 @@
condition = ~condition;
}
- condition &= enableStack[enableIndex];
+ condition &= enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))];
if(!labelBlock[labelIndex])
{
@@ -1193,11 +1193,11 @@
if(callRetBlock[labelIndex].size() > 1)
{
- callStack[stackIndex++] = UInt(callSiteIndex);
+ callStack[Min(stackIndex++, Int(MAX_SHADER_CALL_STACK_SIZE))] = UInt(callSiteIndex);
}
enableIndex++;
- enableStack[enableIndex] = condition;
+ enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))] = condition;
Int4 restoreLeave = enableLeave;
Bool notAllFalse = SignMask(condition) != 0;
@@ -1217,12 +1217,12 @@
if(isConditionalIf[ifDepth])
{
- Int4 condition = ~enableStack[enableIndex] & enableStack[enableIndex - 1];
+ Int4 condition = ~enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))] & enableStack[Min(enableIndex - 1, Int(MAX_SHADER_ENABLE_STACK_SIZE))];
Bool notAllFalse = SignMask(condition) != 0;
branch(notAllFalse, falseBlock, endBlock);
- enableStack[enableIndex] = ~enableStack[enableIndex] & enableStack[enableIndex - 1];
+ enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))] = ~enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))] & enableStack[Min(enableIndex - 1, Int(MAX_SHADER_ENABLE_STACK_SIZE))];
}
else
{
@@ -1376,10 +1376,10 @@
void VertexProgram::IF(Int4 &condition)
{
- condition &= enableStack[enableIndex];
+ condition &= enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))];
enableIndex++;
- enableStack[enableIndex] = condition;
+ enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))] = condition;
BasicBlock *trueBlock = Nucleus::createBasicBlock();
BasicBlock *falseBlock = Nucleus::createBasicBlock();
@@ -1484,10 +1484,10 @@
const Vector4f &src = fetchRegister(temporaryRegister);
Int4 condition = As<Int4>(src.x);
- condition &= enableStack[enableIndex - 1];
+ condition &= enableStack[Min(enableIndex - 1, Int(MAX_SHADER_ENABLE_STACK_SIZE))];
if(shader->containsLeaveInstruction()) condition &= enableLeave;
if(shader->containsBreakInstruction()) condition &= enableBreak;
- enableStack[enableIndex] = condition;
+ enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))] = condition;
Bool notAllFalse = SignMask(condition) != 0;
branch(notAllFalse, loopBlock, endBlock);
@@ -1560,7 +1560,7 @@
void VertexProgram::LEAVE()
{
- enableLeave = enableLeave & ~enableStack[enableIndex];
+ enableLeave = enableLeave & ~enableStack[Min(enableIndex, Int(MAX_SHADER_ENABLE_STACK_SIZE))];
// FIXME: Return from function if all instances left
// FIXME: Use enableLeave in other control-flow constructs
diff --git a/src/Shader/VertexProgram.hpp b/src/Shader/VertexProgram.hpp
index 33f3c5c..379abce 100644
--- a/src/Shader/VertexProgram.hpp
+++ b/src/Shader/VertexProgram.hpp
@@ -39,18 +39,18 @@
RegisterArray<NUM_TEMPORARY_REGISTERS> r; // Temporary registers
Vector4f a0;
- Array<Int, 4> aL;
+ Array<Int, MAX_SHADER_NESTED_LOOPS> aL;
Vector4f p0;
- Array<Int, 4> increment;
- Array<Int, 4> iteration;
+ Array<Int, MAX_SHADER_NESTED_LOOPS> increment;
+ Array<Int, MAX_SHADER_NESTED_LOOPS> iteration;
Int loopDepth;
Int stackIndex; // FIXME: Inc/decrement callStack
- Array<UInt, 16> callStack;
+ Array<UInt, MAX_SHADER_CALL_STACK_SIZE> callStack;
Int enableIndex;
- Array<Int4, 1 + 24> enableStack;
+ Array<Int4, MAX_SHADER_ENABLE_STACK_SIZE> enableStack;
Int4 enableBreak;
Int4 enableContinue;
Int4 enableLeave;
@@ -122,18 +122,18 @@
Vector4f sampleTexture(const Src &s, Vector4f &uvwq, Float4 &lod, Vector4f &dsx, Vector4f &dsy, Vector4f &offset, SamplerFunction function);
Vector4f sampleTexture(int sampler, Vector4f &uvwq, Float4 &lod, Vector4f &dsx, Vector4f &dsy, Vector4f &offset, SamplerFunction function);
- int ifDepth = 0;
- int loopRepDepth = 0;
- int currentLabel = -1;
+ BoundedIndex<MAX_SHADER_NESTED_IFS> ifDepth = 0;
+ BoundedIndex<MAX_SHADER_NESTED_LOOPS> loopRepDepth = 0;
+ BoundedIndex<MAX_SHADER_CALL_SITES> currentLabel = -1;
bool scalar = false;
- BasicBlock *ifFalseBlock[24 + 24];
- BasicBlock *loopRepTestBlock[4];
- BasicBlock *loopRepEndBlock[4];
- BasicBlock *labelBlock[2048];
- std::vector<BasicBlock*> callRetBlock[2048];
+ BasicBlock *ifFalseBlock[MAX_SHADER_NESTED_IFS];
+ BasicBlock *loopRepTestBlock[MAX_SHADER_NESTED_LOOPS];
+ BasicBlock *loopRepEndBlock[MAX_SHADER_NESTED_LOOPS];
+ BasicBlock *labelBlock[MAX_SHADER_CALL_SITES];
+ std::vector<BasicBlock*> callRetBlock[MAX_SHADER_CALL_SITES];
BasicBlock *returnBlock;
- bool isConditionalIf[24 + 24];
+ bool isConditionalIf[MAX_SHADER_NESTED_IFS];
std::vector<Int4> restoreContinue;
};
}
