[Mesa-dev] [PATCH] nir: prevent use-after-free condition in should_lower_phi()

Tue Jun 2 04:58:41 PDT 2015

On 06/02/2015 01:42 PM, Eduardo Lima Mitev wrote:
> lower_phis_to_scalar() pass recurses the instruction dependence graph to
> determine if all the sources of a given instruction are scalarizable.
> To prevent cycles, it temporary marks the phi instruction before recursing in,
> then updates the entry with the resulting value. However, it does not consider
> that the entry value may have changed after a recursion pass, hence causing
> a use-after-free situation and a crash.
> 
> This patch fixes this by reloading the entry corresponding to the 'phi'
> after recursing and before updating its value.
> 
> The crash can be reproduced ~20% of times with the dEQP test:
> 
> dEQP-GLES3.functional.shaders.loops.while_constant_iterations.nested_sequence_fragment

This is the corresponding memory error as generated by valgrind:

==16154== Invalid write of size 8
==16154==    at 0x9B52472: should_lower_phi (nir_lower_phis_to_scalar.c:156)
==16154==    by 0x9B52300: is_phi_src_scalarizable
(nir_lower_phis_to_scalar.c:75)
==16154==    by 0x9B5243F: should_lower_phi (nir_lower_phis_to_scalar.c:151)
==16154==    by 0x9B52541: lower_phis_to_scalar_block
(nir_lower_phis_to_scalar.c:184)
==16154==    by 0x9B43A86: foreach_cf_node (nir.c:1995)
==16154==    by 0x9B43A01: foreach_loop (nir.c:1981)
==16154==    by 0x9B43ADA: foreach_cf_node (nir.c:1999)
==16154==    by 0x9B43B4B: nir_foreach_block (nir.c:2014)
==16154==    by 0x9B529C4: lower_phis_to_scalar_impl
(nir_lower_phis_to_scalar.c:268)
==16154==    by 0x9B52A2B: nir_lower_phis_to_scalar
(nir_lower_phis_to_scalar.c:288)
==16154==    by 0x9C59DB9: nir_optimize (brw_nir.c:41)
==16154==    by 0x9C5A113: brw_create_nir (brw_nir.c:116)
==16154==    by 0x9C685A2: brw_link_shader (brw_shader.cpp:301)
==16154==    by 0x9A6CD75: _mesa_glsl_link_shader (ir_to_mesa.cpp:2964)
==16154==    by 0x990D0C6: link_program (shaderapi.c:946)
==16154==    by 0x990E1B2: _mesa_LinkProgram (shaderapi.c:1411)
==16154==    by 0x723F076: shared_dispatch_stub_509 (glapi_mapi_tmp.h:19522)
==16154==    by 0x102164E: glu::Program::link() (in
/opt/devel/deqp/modules/gles3/deqp-gles3)
==16154==    by 0x1022010:
glu::ShaderProgram::ShaderProgram(glu::RenderContext const&,
glu::ProgramSources const&) (in /opt/devel/deqp/modules/gles3/deqp-gles3)
==16154==    by 0x12D8CDA: deqp::gls::ShaderRenderCase::init() (in
/opt/devel/deqp/modules/gles3/deqp-gles3)
==16154==  Address 0xd1a3880 is 64 bytes inside a block of size 168 free'd
==16154==    at 0x4C29730: free (vg_replace_malloc.c:468)
==16154==    by 0x9B173E2: unsafe_free (ralloc.c:255)
==16154==    by 0x9B172CC: ralloc_free (ralloc.c:218)
==16154==    by 0x9B1691F: _mesa_hash_table_rehash (hash_table.c:262)
==16154==    by 0x9B1696A: hash_table_insert (hash_table.c:273)
==16154==    by 0x9B16B70: _mesa_hash_table_insert (hash_table.c:342)
==16154==    by 0x9B52416: should_lower_phi (nir_lower_phis_to_scalar.c:146)
==16154==    by 0x9B52300: is_phi_src_scalarizable
(nir_lower_phis_to_scalar.c:75)
==16154==    by 0x9B5243F: should_lower_phi (nir_lower_phis_to_scalar.c:151)
==16154==    by 0x9B52300: is_phi_src_scalarizable
(nir_lower_phis_to_scalar.c:75)
==16154==    by 0x9B5243F: should_lower_phi (nir_lower_phis_to_scalar.c:151)
==16154==    by 0x9B52541: lower_phis_to_scalar_block
(nir_lower_phis_to_scalar.c:184)
==16154==    by 0x9B43A86: foreach_cf_node (nir.c:1995)
==16154==    by 0x9B43A01: foreach_loop (nir.c:1981)
==16154==    by 0x9B43ADA: foreach_cf_node (nir.c:1999)
==16154==    by 0x9B43B4B: nir_foreach_block (nir.c:2014)
==16154==    by 0x9B529C4: lower_phis_to_scalar_impl
(nir_lower_phis_to_scalar.c:268)
==16154==    by 0x9B52A2B: nir_lower_phis_to_scalar
(nir_lower_phis_to_scalar.c:288)
==16154==    by 0x9C59DB9: nir_optimize (brw_nir.c:41)
==16154==    by 0x9C5A113: brw_create_nir (brw_nir.c:116)

And this is a stacktrace (though not very helpful):

Program terminated with signal 11, Segmentation fault.
#0  0x00007f3f7e4a5963 in malloc_consolidate (av=av at entry=0x7f3f7e7cf620
<main_arena>) at malloc.c:4157
4157	malloc.c: No such file or directory.
(gdb) bt
#0  0x00007f3f7e4a5963 in malloc_consolidate (av=av at entry=0x7f3f7e7cf620
<main_arena>) at malloc.c:4157
#1  0x00007f3f7e4a6c98 in _int_malloc (av=av at entry=0x7f3f7e7cf620
<main_arena>, bytes=bytes at entry=1080) at malloc.c:3423
#2  0x00007f3f7e4a97dc in __libc_calloc (n=<optimized out>,
elem_size=<optimized out>) at malloc.c:3219
#3  0x00007f3f7baa9f6f in ralloc_size (ctx=0x42acb20, size=1032) at
ralloc.c:113
#4  0x00007f3f7baa9ffa in rzalloc_size (ctx=0x42acb20, size=1032) at
ralloc.c:134
#5  0x00007f3f7baaa238 in rzalloc_array_size (ctx=0x42acb20, size=24,
count=43) at ralloc.c:196
#6  0x00007f3f7baa97c7 in _mesa_hash_table_rehash (ht=0x42acb20,
new_size_index=4) at hash_table.c:243
#7  0x00007f3f7baa996b in hash_table_insert (ht=0x42acb20,
hash=3370383618, key=0x46a7b20, data=0x42ae350) at hash_table.c:273
#8  0x00007f3f7baa9b71 in _mesa_hash_table_insert (ht=0x42acb20,
key=0x46a7b20, data=0x42ae350) at hash_table.c:342
#9  0x00007f3f7bafb7cf in validate_var_decl (var=0x46a7b20,
is_global=false, state=0x7fffd86c3cb0) at nir/nir_validate.c:785
#10 0x00007f3f7bafbce9 in validate_function_impl (impl=0x42ae350,
state=0x7fffd86c3cb0) at nir/nir_validate.c:863
#11 0x00007f3f7bafbef3 in validate_function_overload
(overload=0x42ab390, state=0x7fffd86c3cb0) at nir/nir_validate.c:898
#12 0x00007f3f7bafbf63 in validate_function (func=0x42ab320,
state=0x7fffd86c3cb0) at nir/nir_validate.c:907
#13 0x00007f3f7bafc33f in nir_validate_shader (shader=0x46a6310) at
nir/nir_validate.c:979
#14 0x00007f3f7bbecdc6 in nir_optimize (nir=0x46a6310) at brw_nir.c:42
#15 0x00007f3f7bbed114 in brw_create_nir (brw=0x7f3f807d0040,
shader_prog=0x42a9970, prog=0x46a6a60, stage=MESA_SHADER_FRAGMENT) at
brw_nir.c:116
#16 0x00007f3f7bbfb5a3 in brw_link_shader (ctx=0x7f3f807d0040,
shProg=0x42a9970) at brw_shader.cpp:301
#17 0x00007f3f7b9ffd76 in _mesa_glsl_link_shader (ctx=0x7f3f807d0040,
prog=0x42a9970) at program/ir_to_mesa.cpp:2964
#18 0x00007f3f7b8a00c7 in link_program (ctx=0x7f3f807d0040, program=1)
at main/shaderapi.c:946
#19 0x00007f3f7b8a11b3 in _mesa_LinkProgram (programObj=1) at
main/shaderapi.c:1411
#20 0x00007f3f7dfcc077 in shared_dispatch_stub_509 (program=1) at
shared-glapi/glapi_mapi_tmp.h:19522
#21 0x000000000102164f in glu::Program::link() ()
#22 0x0000000001022011 in
glu::ShaderProgram::ShaderProgram(glu::RenderContext const&,
glu::ProgramSources const&) ()
#23 0x00000000012d8cdb in deqp::gls::ShaderRenderCase::init() ()
#24 0x0000000000fd4bc2 in
tcu::TestCaseWrapper::initTestCase(tcu::TestCase*) ()
#25 0x0000000000a9a75f in
deqp::gles3::TestCaseWrapper::initTestCase(tcu::TestCase*) ()
#26 0x0000000000fd62f0 in
tcu::TestExecutor::enterTestCase(tcu::TestCase*, char const*) ()
#27 0x0000000000fd6801 in tcu::TestExecutor::iterate() ()
#28 0x0000000000fad951 in tcu::App::iterate() ()
#29 0x0000000000a970c9 in main ()

> ---
>  src/glsl/nir/nir_lower_phis_to_scalar.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/src/glsl/nir/nir_lower_phis_to_scalar.c b/src/glsl/nir/nir_lower_phis_to_scalar.c
> index 4bdb800..a57d253 100644
> --- a/src/glsl/nir/nir_lower_phis_to_scalar.c
> +++ b/src/glsl/nir/nir_lower_phis_to_scalar.c
> @@ -153,6 +153,11 @@ should_lower_phi(nir_phi_instr *phi, struct lower_phis_to_scalar_state *state)
>           break;
>     }
>  
> +   /* The hash table entry for 'phi' may have changed while recursing the
> +    * dependence graph, so we need to reset it */
> +   entry = _mesa_hash_table_search(state->phi_table, phi);
> +   assert(entry);
> +
>     entry->data = (void *)(intptr_t)scalarizable;
>  
>     return scalarizable;
> 