Bug 71574

Summary: Long expressions in shaders crash Mesa
Product: Mesa Reporter: Kevin Rogovin <kevin.rogovin>
Component: glsl-compilerAssignee: Ian Romanick <idr>
Status: NEW --- QA Contact: Intel 3D Bugs Mailing List <intel-3d-bugs>
Severity: normal    
Priority: medium CC: eero.t.tamminen
Version: 9.1   
Hardware: Other   
OS: All   
Whiteboard:
i915 platform: i915 features:
Attachments: vertex shader
Fragment shader of program.
Very long expression in vertex shader that crashes Mesa
Incredibly long expression in vertex shader that crashes Mesa

Description Kevin Rogovin 2013-11-13 13:03:51 UTC
Created attachment 89137 [details]
vertex shader

A long expression in a GLSL shader will crash Mesa, the crash occurs after parsing in generating the HIR from the AST.

Attached is a vertex shader with such a long expression (10,000 add terms).

The bug issue is that although such a shader is not to be found in "real" applications, a malicious website can use WebGL to crash Mesa which would likely crash the browser.
Comment 1 Kevin Rogovin 2013-11-13 13:07:31 UTC
Created attachment 89139 [details]
Fragment shader of program.
Comment 2 Kevin Rogovin 2013-11-13 13:09:51 UTC
*** Bug 71575 has been marked as a duplicate of this bug. ***
Comment 3 Kevin Rogovin 2014-12-31 11:56:23 UTC
A minor note: the crash occurs in src/glsl/ast_to_hir.cpp and looks to be a stack overflow. The cause is as follows:

 - The code that generates the AST is a bison generated parser. The parser uses its own stack instead of an OS provided stack. That stack has a much larger maximum size than an OS stack (since the parser can just realloc to whatever size it needs). In particular the depth of the AST can be quite larger (for example in this case on order of 30,000).

 - The code that generates the HIR from the AST uses recursion and thus uses an OS provided stack. A deep AST will then trigger a stack overflow.

The wrong way to fix this is to increase the stack size so that this shader does not trigger a crash. The correct way to fix the issue is during AST generation in the parser code is to have a stack depth value for the nodes (computed as the maximum of the stack depth of the child nodes). Then at HIR generation from AST to check the stack depth of the root tree and to emit a failure message if the value is too large, here too large being a value decided upon by making sure the OS provided stack is big enough and in addition, if the depth is great the shader is likely not going to be compilable in a reasonable amount of time (if at all). My hunch of arbitrariness is to set the max depth to be somewhere in the range of 1000 to 5000.
Comment 4 Juan A. Suarez 2015-11-25 11:51:03 UTC
I've been testing this shaders with latest upstream version (commit 315c4c315) and right now doesn't crash when loading them.


It is true that it takes quite a lot of time to link them (order of several minutes), but it finalizes without any crash.

So suggesting to close it as fixed.
Comment 5 Kevin Rogovin 2015-11-25 12:09:28 UTC
Created attachment 120111 [details]
Very long expression in vertex shader that crashes Mesa
Comment 6 Kevin Rogovin 2015-11-25 12:14:20 UTC
Created attachment 120112 [details]
Incredibly long expression in vertex shader that crashes Mesa

Ugh.

The attached shader (120111: Very long expression in vertex shader that crashes Mesa) does not crash Mesa. The shader I wanted to attach, with an expression length of 25,000 elements DOES crash Mesa, but it is quite large: 2.5MB.
Comment 7 Kevin Rogovin 2015-11-25 12:18:03 UTC
It is debatable whether the shaders that are crashing Mesa really matter as they would not happen in the wild.

The cause is the following. A tree is built via bison generated code. That code implements its own stack. However, the tree is walked via recursion using the OS-provided stack. The crash happens essentially (I think) because of stack overflow.

Increasing the stack size of walking the tree will just make the bug happen with even larger expression. The right thing to do is for the Bison generated code to generate a tree depth value for the tree and the code that walks a tree checks the depth value and rejects it as "too complicated" if the tree depth is too great.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.