Enforce basic limits on function stack size in SkSL.

Functions that declare variables totaling more than 100,000 slots will
now generate an error.

This is only a partial mitigation to the problem, as a sophisticated
attack could still chain/nest multiple functions together to consume
extremely large amounts of stack. However, this mitigation is still more
sophisticated than our peers; both WebGL and glslang are susceptible to
similar problems, and in the general case (ES3+ with full flow control)
it's intractable.

Change-Id: I153c75267c017a23f59fe9e59f6e391197ee6101
Bug: oss-fuzz:40304, oss-fuzz:40694
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/467759
Commit-Queue: John Stiles <johnstiles@google.com>
Auto-Submit: John Stiles <johnstiles@google.com>
Reviewed-by: Brian Osman <brianosman@google.com>
This commit is contained in:
John Stiles 2021-11-04 13:32:00 -04:00 committed by SkCQ
parent 37afdbc22e
commit 7cde28909f
8 changed files with 54 additions and 33 deletions

View File

@ -121,6 +121,7 @@ sksl_error_tests = [
"/sksl/errors/OverflowInt64Literal.sksl",
"/sksl/errors/OverflowParamArraySize.sksl",
"/sksl/errors/OverflowUintLiteral.sksl",
"/sksl/errors/ProgramTooLarge_Stack.sksl",
"/sksl/errors/PrototypeInFuncBody.sksl",
"/sksl/errors/PrivateTypes.sksl",
"/sksl/errors/RedeclareBasicType.sksl",

View File

@ -31,6 +31,11 @@ using StatementArray = SkSTArray<2, std::unique_ptr<Statement>>;
// default threshold value is arbitrary, but tends to work well in practice.
static constexpr int kDefaultInlineThreshold = 50;
// A hard upper limit on the number of variable slots allowed in a function/global scope.
// This is an arbitrary limit, but is needed to prevent code generation from taking unbounded
// amounts of time or space.
static constexpr int kVariableSlotLimit = 100000;
// The SwizzleComponent namespace is used both by the SkSL::Swizzle expression, and the DSL swizzle.
// This namespace is injected into SkSL::dsl so that `using namespace SkSL::dsl` enables DSL code
// like `Swizzle(var, X, Y, ONE)` to compile without any extra qualifications.

View File

@ -0,0 +1,12 @@
struct S {
half4 ah4[1];
half ah[99999];
half4 h4;
half h;
};
void main() {
int small;
S big_chungus;
S no_report; // we don't need to report overflows past the first
}

View File

@ -6,6 +6,7 @@
*/
#include "include/sksl/DSLCore.h"
#include "src/core/SkSafeMath.h"
#include "src/sksl/SkSLAnalysis.h"
#include "src/sksl/SkSLCompiler.h"
#include "src/sksl/SkSLContext.h"
@ -142,6 +143,23 @@ std::unique_ptr<FunctionDefinition> FunctionDefinition::Convert(const Context& c
bool visitStatement(Statement& stmt) override {
switch (stmt.kind()) {
case Statement::Kind::kVarDeclaration: {
// We count the number of slots used, but don't consider the precision of the
// base type. In practice, this reflects what GPUs really do pretty well.
// (i.e., RelaxedPrecision math doesn't mean your variable takes less space.)
// We also don't attempt to reclaim slots at the end of a Block.
size_t prevSlotsUsed = fSlotsUsed;
fSlotsUsed = SkSafeMath::Add(
fSlotsUsed, stmt.as<VarDeclaration>().var().type().slotCount());
// To avoid overzealous error reporting, only trigger the error at the first
// place where the stack limit is exceeded.
if (prevSlotsUsed < kVariableSlotLimit && fSlotsUsed >= kVariableSlotLimit) {
fContext.fErrors->error(stmt.fLine, "variable '" +
stmt.as<VarDeclaration>().var().name() +
"' exceeds the stack size limit");
}
break;
}
case Statement::Kind::kReturn: {
// Early returns from a vertex main() function will bypass sk_Position
// normalization, so SkASSERT that we aren't doing that. If this becomes an
@ -224,6 +242,8 @@ std::unique_ptr<FunctionDefinition> FunctionDefinition::Convert(const Context& c
IntrinsicSet* fReferencedIntrinsics;
// how deeply nested we are in breakable constructs (for, do, switch).
int fBreakableLevel = 0;
// number of slots consumed by all variables declared in the function
size_t fSlotsUsed = 0;
// how deeply nested we are in continuable constructs (for, do).
// We keep a stack (via a forward_list) in order to disallow continue inside of switch.
std::forward_list<int> fContinuableLevel{0};
@ -239,7 +259,7 @@ std::unique_ptr<FunctionDefinition> FunctionDefinition::Convert(const Context& c
if (Analysis::CanExitWithoutReturningValue(function, *body)) {
context.fErrors->error(function.fLine, "function '" + function.name() +
"' can exit without returning a value");
"' can exit without returning a value");
}
return std::make_unique<FunctionDefinition>(line, &function, builtin, std::move(body),

View File

@ -0,0 +1,4 @@
### Compilation failed:
error: 10: variable 'big_chungus' exceeds the stack size limit
1 error

View File

@ -1,17 +1,4 @@
OpCapability Shader
%1 = OpExtInstImport "GLSL.std.450"
OpMemoryModel Logical GLSL450
OpEntryPoint Fragment %main "main" %sk_Clockwise
OpExecutionMode %main OriginUpperLeft
OpName %sk_Clockwise "sk_Clockwise"
OpName %main "main"
OpDecorate %sk_Clockwise BuiltIn FrontFacing
%bool = OpTypeBool
%_ptr_Input_bool = OpTypePointer Input %bool
%sk_Clockwise = OpVariable %_ptr_Input_bool Input
%void = OpTypeVoid
%7 = OpTypeFunction %void
%main = OpFunction %void None %7
%8 = OpLabel
OpReturn
OpFunctionEnd
### Compilation failed:
error: 2: variable 'a' exceeds the stack size limit
1 error

View File

@ -1,3 +1,4 @@
### Compilation failed:
void main() {
}
error: 2: variable 'a' exceeds the stack size limit
1 error

View File

@ -1,13 +1,4 @@
#include <metal_stdlib>
#include <simd/simd.h>
using namespace metal;
struct Inputs {
};
struct Outputs {
half4 sk_FragColor [[color(0)]];
};
fragment Outputs fragmentMain(Inputs _in [[stage_in]], bool _frontFacing [[front_facing]], float4 _fragCoord [[position]]) {
Outputs _out;
(void)_out;
return _out;
}
### Compilation failed:
error: 2: variable 'a' exceeds the stack size limit
1 error