[Mesa-dev] [PATCH 5/5] glsl: Avoid buffer overflow when assigning attribute locations

[Ian Romanick]

On 03/11/2015 02:01 AM, Eduardo Lima Mitev wrote:
> From: Iago Toral Quiroga <itoral at igalia.com>
> 
> Shaders with excessive number of attributes (>16) can produce a crash
> due to buffer overflow in assign_attribute_or_color_locations. The
> overflow can happen because we declare a fixed size array that can hold
> up to 16 attributes and we don't check that we don't go beyond that
> limit.
> 
> This patch changes the limit from a fixed size of 16 element to
> MAX2(MAX_VERTEX_GENERIC_ATTRIBS, MAX_NV_FRAGMENT_PROGRAM_INPUTS), which
> seems more reasonable. It also makes sure that we don't process more than
> this amount of attributes, producing a linker error if the shader requires
> more than this.
> 
> Avoids crashes in 108 dEQP tests in these categories:
> dEQP-GLES3.functional.transform_feedback.array_element.separate.
> dEQP-GLES3.functional.transform_feedback.array_element.interleaved.*
> ---
>  src/glsl/linker.cpp | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
> index 0c44677..ffe8ede 100644
> --- a/src/glsl/linker.cpp
> +++ b/src/glsl/linker.cpp
> @@ -1983,9 +1983,11 @@ assign_attribute_or_color_locations(gl_shader_program *prog,
>  	 /* Reversed because we want a descending order sort below. */
>  	 return r->slots - l->slots;
>        }
> -   } to_assign[16];
> +   } to_assign[MAX2(MAX_VERTEX_GENERIC_ATTRIBS, MAX_NV_FRAGMENT_PROGRAM_INPUTS)];

I believe Marek recently landed a patch that removes
MAX_NV_FRAGMENT_PROGRAM_INPUTS.  It's also not correct. :) This function
operates on vertex shader inputs or fragment shader outputs.

>     unsigned num_attr = 0;
> +   unsigned max_attr = (target_index == MESA_SHADER_VERTEX) ?
> +      MAX_VERTEX_GENERIC_ATTRIBS : MAX_NV_FRAGMENT_PROGRAM_INPUTS;

      const unsigned max_attr = ...

>  
>     foreach_in_list(ir_instruction, node, sh->ir) {
>        ir_variable *const var = node->as_variable();
> @@ -2147,6 +2149,13 @@ assign_attribute_or_color_locations(gl_shader_program *prog,
>  	 continue;
>        }
>  
> +      if (num_attr >= max_attr) {
> +         linker_error(prog,
> +                      "Number of required attribute locations "
> +                      "exceeds allowed limit (limit=%d)", max_attr);

This message should be specialized for the stage (i.e., "vertex input"
vs. "fragment output").

> +         return false;
> +      }
> +
>        to_assign[num_attr].slots = slots;
>        to_assign[num_attr].var = var;
>        num_attr++;
>