[Mesa-dev] [Mesa-stable] [PATCH] glsl: fix several use-after-free bugs

[Chris Forbes]

I think get_variable_being_redeclared() deleting its argument in one
specific case (sizing an unsized array via redeclaration) is really
nasty and we should get rid of it, but this does fix the fallout:

Reviewed-by: Chris Forbes <chrisf at ijw.co.nz>

On Sat, Oct 18, 2014 at 8:37 AM, Brian Paul <brianp at vmware.com> wrote:
> The get_variable_being_redeclared() function can free the 'var' argument.
> Thereafter, we cannot assume that 'var' is a valid  pointer.  This patch
> replaces 'var->name' with 'earlier->name' in two places and calls
> is_gl_identifier(var->name) before 'var' might get freed.
>
> This fixes several piglit GLSL crashes, including:
> spec/glsl-1.50/execution/geometry/clip-distance-in-param
> spec/glsl-1.50/execution/geometry/clip-distance-bulk-copy
> spec/glsl-1.50/compiler/gs-redeclares-pervertex-out-before-global-redeclaration.geom
>
> I'm not sure why these were not spotted sooner.
> A similar bug was previously fixed by f9cecca7a.
>
> Cc: <mesa-stable at lists.freedesktop.org>
> ---
>  src/glsl/ast_to_hir.cpp |   10 +++++++---
>  1 file changed, 7 insertions(+), 3 deletions(-)
>
> diff --git a/src/glsl/ast_to_hir.cpp b/src/glsl/ast_to_hir.cpp
> index 068af29..102ea55 100644
> --- a/src/glsl/ast_to_hir.cpp
> +++ b/src/glsl/ast_to_hir.cpp
> @@ -3767,7 +3767,7 @@ ast_declarator_list::hir(exec_list *instructions,
>               earlier->data.how_declared == ir_var_declared_in_block) {
>              _mesa_glsl_error(&loc, state,
>                               "`%s' has already been redeclared using "
> -                             "gl_PerVertex", var->name);
> +                             "gl_PerVertex", earlier->name);
>           }
>           earlier->data.how_declared = ir_var_declared_normally;
>        }
> @@ -5681,17 +5681,21 @@ ast_interface_block::hir(exec_list *instructions,
>
>           var->data.stream = this->layout.stream;
>
> +         /* Examine var name here since var may get deleted in the next call */
> +         bool var_is_gl_id = is_gl_identifier(var->name);
> +
>           if (redeclaring_per_vertex) {
>              ir_variable *earlier =
>                 get_variable_being_redeclared(var, loc, state,
>                                               true /* allow_all_redeclarations */);
> -            if (!is_gl_identifier(var->name) || earlier == NULL) {
> +            if (!var_is_gl_id || earlier == NULL) {
>                 _mesa_glsl_error(&loc, state,
>                                  "redeclaration of gl_PerVertex can only "
>                                  "include built-in variables");
>              } else if (earlier->data.how_declared == ir_var_declared_normally) {
>                 _mesa_glsl_error(&loc, state,
> -                                "`%s' has already been redeclared", var->name);
> +                                "`%s' has already been redeclared",
> +                                earlier->name);
>              } else {
>                 earlier->data.how_declared = ir_var_declared_in_block;
>                 earlier->reinit_interface_type(block_type);
> --
> 1.7.10.4
>
> _______________________________________________
> mesa-stable mailing list
> mesa-stable at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-stable