[Mesa-dev] [PATCH] mesa: Move transform feedback error check to reduce array overflow risk.

[Paul Berry]

Previous to this patch, we didn't do the limit check for
MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS until the end of the
store_tfeedback_info() function, *after* storing all of the transform
feedback info in the gl_transform_feedback_info::Outputs array.  This
meant that the limit check wouldn't prevent us from overflowing the
array and corrupting memory.

This patch moves the limit check to the top of tfeedback_decl::store()
so that there is no risk of overflowing the array.  It also adds
assertions to verify that the checks for
MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS and
MAX_TRANSFORM_FEEDBACK_SEPARATE_COMPONENTS are sufficient to avoid
array overflow.

Note: strictly speaking this patch isn't necessary, since the maximum
possible number of varyings is MAX_VARYING (16), whereas the size of
the Outputs array is MAX_PROGRAM_OUTPUTS (64), so it's impossible to
have enough varyings to overflow the array.  However it seems prudent
to do the limit check before the array access in case these limits
change in the future.
---
 src/glsl/linker.cpp |   52 +++++++++++++++++++++++++++++++-------------------
 1 files changed, 32 insertions(+), 20 deletions(-)

diff --git a/src/glsl/linker.cpp b/src/glsl/linker.cpp
index 39a9c46..0d85aee 100644
--- a/src/glsl/linker.cpp
+++ b/src/glsl/linker.cpp
@@ -1388,7 +1388,7 @@ public:
    static bool is_same(const tfeedback_decl &x, const tfeedback_decl &y);
    bool assign_location(struct gl_context *ctx, struct gl_shader_program *prog,
                         ir_variable *output_var);
-   bool store(struct gl_shader_program *prog,
+   bool store(struct gl_context *ctx, struct gl_shader_program *prog,
               struct gl_transform_feedback_info *info, unsigned buffer,
 	      unsigned varying) const;
 
@@ -1631,7 +1631,7 @@ tfeedback_decl::assign_location(struct gl_context *ctx,
  * is returned.
  */
 bool
-tfeedback_decl::store(struct gl_shader_program *prog,
+tfeedback_decl::store(struct gl_context *ctx, struct gl_shader_program *prog,
                       struct gl_transform_feedback_info *info,
                       unsigned buffer, unsigned varying) const
 {
@@ -1647,6 +1647,35 @@ tfeedback_decl::store(struct gl_shader_program *prog,
                    this->orig_name);
       return false;
    }
+
+   /* From GL_EXT_transform_feedback:
+    *   A program will fail to link if:
+    *
+    *     * the total number of components to capture is greater than
+    *       the constant MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS_EXT
+    *       and the buffer mode is INTERLEAVED_ATTRIBS_EXT.
+    */
+   if (prog->TransformFeedback.BufferMode == GL_INTERLEAVED_ATTRIBS &&
+       info->BufferStride[buffer] + this->num_components() >
+       ctx->Const.MaxTransformFeedbackInterleavedComponents) {
+      linker_error(prog, "The MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS "
+                   "limit has been exceeded.");
+      return false;
+   }
+
+   /* Verify that the checks on MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS
+    * and MAX_TRANSFORM_FEEDBACK_SEPARATE_COMPONENTS are sufficient to prevent
+    * overflow of info->Outputs[].  In worst case we generate one entry in
+    * Outputs[] per component so a conservative check is to verify that the
+    * size of the array is greater than or equal to both
+    * MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS and
+    * MAX_TRANSFORM_FEEDBACK_SEPARATE_COMPONENTS.
+    */
+   assert(Elements(info->Outputs) >=
+          ctx->Const.MaxTransformFeedbackInterleavedComponents);
+   assert(Elements(info->Outputs) >=
+          ctx->Const.MaxTransformFeedbackSeparateComponents);
+
    unsigned translated_size = this->size;
    if (this->is_clip_distance_mesa)
       translated_size = (translated_size + 3) / 4;
@@ -1943,7 +1972,6 @@ store_tfeedback_info(struct gl_context *ctx, struct gl_shader_program *prog,
                      unsigned num_tfeedback_decls,
                      tfeedback_decl *tfeedback_decls)
 {
-   unsigned total_tfeedback_components = 0;
    bool separate_attribs_mode =
       prog->TransformFeedback.BufferMode == GL_SEPARATE_ATTRIBS;
 
@@ -1962,25 +1990,9 @@ store_tfeedback_info(struct gl_context *ctx, struct gl_shader_program *prog,
 
    for (unsigned i = 0; i < num_tfeedback_decls; ++i) {
       unsigned buffer = separate_attribs_mode ? i : 0;
-      if (!tfeedback_decls[i].store(prog, &prog->LinkedTransformFeedback,
+      if (!tfeedback_decls[i].store(ctx, prog, &prog->LinkedTransformFeedback,
                                     buffer, i))
          return false;
-      total_tfeedback_components += tfeedback_decls[i].num_components();
-   }
-
-   /* From GL_EXT_transform_feedback:
-    *   A program will fail to link if:
-    *
-    *     * the total number of components to capture is greater than
-    *       the constant MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS_EXT
-    *       and the buffer mode is INTERLEAVED_ATTRIBS_EXT.
-    */
-   if (prog->TransformFeedback.BufferMode == GL_INTERLEAVED_ATTRIBS &&
-       total_tfeedback_components >
-       ctx->Const.MaxTransformFeedbackInterleavedComponents) {
-      linker_error(prog, "The MAX_TRANSFORM_FEEDBACK_INTERLEAVED_COMPONENTS "
-                   "limit has been exceeded.");
-      return false;
    }
 
    return true;
-- 
1.7.6.5