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

[Iago Toral Quiroga]

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_DRAW_BUFFERS), 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.

v2:
  - With fragment shaders, this function processes outputs, not inputs (Ian)
  - Error message should tell if the problem is with vertex inputs or
    fragment outputs (Ian)
  - The function receives a parameter with the attribute limit (max_index),
    so no need to compute it inside
  - Assert if max_index is too large for the size of to_assign[]

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.*
---

The actual limit for fragment shader outputs is:
MAX2(ctx->Const.MaxDrawBuffers, ctx->Const.MaxDualSourceDrawBuffers)

but these values are implementation dependent, so no good for sizing
a static array. In the patch I am assuming that:

Const.MaxDualSourceDrawBuffers <= Const.MaxDrawBuffers <= MAX_DRAW_BUFFERS

if this is not a good idea I guess the right fix would be to drop the static
to_assign[] array and allocate it dynamically based on the value of
max_index.

 src/glsl/linker.cpp | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 9978380..15a54d5 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -1974,7 +1974,9 @@ 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_DRAW_BUFFERS)];
+
+   assert(max_index <= MAX2(MAX_VERTEX_GENERIC_ATTRIBS, MAX_DRAW_BUFFERS));
 
    unsigned num_attr = 0;
    unsigned total_attribs_size = 0;
@@ -2168,6 +2170,15 @@ assign_attribute_or_color_locations(gl_shader_program *prog,
 	 continue;
       }
 
+      if (num_attr >= max_index) {
+         linker_error(prog,
+                      "Number of required %s exceeds allowed limit (limit=%d)",
+                      target_index == MESA_SHADER_VERTEX ?
+                        "vertex inputs" : "fragment outputs",
+                      max_index);
+         return false;
+      }
+
       to_assign[num_attr].slots = slots;
       to_assign[num_attr].var = var;
       num_attr++;
-- 
2.1.0