[Mesa-dev] [PATCH v2] glsl: fix heap-use-after-free in ast_declarator_list::hir()

[Bartosz Tomczyk]

Patch is:
Tested-by: Bartosz Tomczyk <bartosz.tomczyk86 at gmail.com>

I can confirm it fix use-after-free issue.

On Tue, Feb 7, 2017 at 1:47 PM, Samuel Iglesias Gonsálvez <
siglesias at igalia.com> wrote:

> The get_variable_being_redeclared() function can free 'var' because
> a re-declaration of an unsized array variable can establish the size, so
> we set the array type to the 'earlier' declaration and free 'var' as it is
> not needed anymore.
>
> However, the same 'var' is referenced later in ast_declarator_list::hir().
>
> This patch fixes it by assign the pointer 'var' to the pointer 'earlier'.
>
> This error was detected by Address Sanitizer.
>
> v2:
>
> * Pointer-to-pointer assignment (Bartosz Tomczyk)
>
> Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=99677
> Signed-off-by: Samuel Iglesias Gonsálvez <siglesias at igalia.com>
> ---
>
> Another possibility is to use reference-to-pointer but it is a C++
> thing. IIRC, we agreed on avoiding C++-specific features to make it
> easy for C developers, but I have no strong opinion for either option.
>
>  src/compiler/glsl/ast_to_hir.cpp | 10 ++++++----
>  1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/src/compiler/glsl/ast_to_hir.cpp b/src/compiler/glsl/ast_to_
> hir.cpp
> index b31b61d1ed6..93ba1d510fa 100644
> --- a/src/compiler/glsl/ast_to_hir.cpp
> +++ b/src/compiler/glsl/ast_to_hir.cpp
> @@ -3958,10 +3958,12 @@ apply_type_qualifier_to_variable(const struct
> ast_type_qualifier *qual,
>   * is a redeclaration, \c NULL otherwise.
>   */
>  static ir_variable *
> -get_variable_being_redeclared(ir_variable *var, YYLTYPE loc,
> +get_variable_being_redeclared(ir_variable **var_pointer, YYLTYPE loc,
>                                struct _mesa_glsl_parse_state *state,
>                                bool allow_all_redeclarations)
>  {
> +   ir_variable *var = *var_pointer;
> +
>     /* Check if this declaration is actually a re-declaration, either to
>      * resize an array or add qualifiers to an existing variable.
>      *
> @@ -3999,7 +4001,7 @@ get_variable_being_redeclared(ir_variable *var,
> YYLTYPE loc,
>
>        earlier->type = var->type;
>        delete var;
> -      var = NULL;
> +      *var_pointer = earlier;
>     } else if ((state->ARB_fragment_coord_conventions_enable ||
>                state->is_version(150, 0))
>                && strcmp(var->name, "gl_FragCoord") == 0
> @@ -5207,7 +5209,7 @@ ast_declarator_list::hir(exec_list *instructions,
>        bool var_is_gl_id = is_gl_identifier(var->name);
>
>        ir_variable *earlier =
> -         get_variable_being_redeclared(var, decl->get_location(), state,
> +         get_variable_being_redeclared(&var, decl->get_location(), state,
>                                         false /* allow_all_redeclarations
> */);
>        if (earlier != NULL) {
>           if (var_is_gl_id &&
> @@ -7873,7 +7875,7 @@ ast_interface_block::hir(exec_list *instructions,
>
>           if (redeclaring_per_vertex) {
>              ir_variable *earlier =
> -               get_variable_being_redeclared(var, loc, state,
> +               get_variable_being_redeclared(&var, loc, state,
>                                               true /*
> allow_all_redeclarations */);
>              if (!var_is_gl_id || earlier == NULL) {
>                 _mesa_glsl_error(&loc, state,
> --
> 2.11.0
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170207/089389c2/attachment-0001.html>