[Mesa-dev] [PATCH] mesa: add bounds checking for uniform array access

[Stéphane Marchesin]

On Fri, Dec 14, 2012 at 12:52 PM, Frank Henigman <fjhenigman at google.com> wrote:
> No piglet regressions and now passes glsl-uniform-out-of-bounds-2.
>
>
> validate_uniform_parameters now checks that the array index is
> valid.  This means if an index is out of bounds, glGetUniform* now
> fails with GL_INVALID_OPERATION, as it should.
> _mesa_uniform and _mesa_uniform_matrix also call
> validate_uniform_parameters so the bounds checks there became
> redundant and were removed.
>
> The test in glGetUniformLocation is modified to check array bounds
> so it now returns GL_INVALID_INDEX (-1) if you ask for the location
> of a non-existent array element, as it should.
>
> Signed-off-by: Frank Henigman <fjhenigman at google.com>

Reviewed-by: Stéphane Marchesin <marcheu at chromium.org>

> ---
>  src/mesa/main/uniform_query.cpp |   26 +++++++++++++-------------
>  1 files changed, 13 insertions(+), 13 deletions(-)
>
> diff --git a/src/mesa/main/uniform_query.cpp b/src/mesa/main/uniform_query.cpp
> index b6b73d1..142ad1f 100644
> --- a/src/mesa/main/uniform_query.cpp
> +++ b/src/mesa/main/uniform_query.cpp
> @@ -241,11 +241,14 @@ validate_uniform_parameters(struct gl_context *ctx,
>        return false;
>     }
>
> -   /* This case should be impossible.  The implication is that a call like
> -    * glGetUniformLocation(prog, "foo[8]") was successful but "foo" is not an
> -    * array.
> +   /* If the uniform is an array, check that array_index is in bounds.
> +    * If not an array, check that array_index is zero.
> +    * array_index is unsigned so no need to check for less than zero.
>      */
> -   if (*array_index != 0 && shProg->UniformStorage[*loc].array_elements == 0) {
> +   unsigned limit = shProg->UniformStorage[*loc].array_elements;
> +   if (limit == 0)
> +      limit = 1;
> +   if (*array_index >= limit) {
>        _mesa_error(ctx, GL_INVALID_OPERATION, "%s(location=%d)",
>                   caller, location);
>        return false;
> @@ -732,9 +735,6 @@ _mesa_uniform(struct gl_context *ctx, struct gl_shader_program *shProg,
>      * will have already generated an error.
>      */
>     if (uni->array_elements != 0) {
> -      if (offset >= uni->array_elements)
> -        return;
> -
>        count = MIN2(count, (int) (uni->array_elements - offset));
>     }
>
> @@ -889,9 +889,6 @@ _mesa_uniform_matrix(struct gl_context *ctx, struct gl_shader_program *shProg,
>      * will have already generated an error.
>      */
>     if (uni->array_elements != 0) {
> -      if (offset >= uni->array_elements)
> -        return;
> -
>        count = MIN2(count, (int) (uni->array_elements - offset));
>     }
>
> @@ -1025,10 +1022,13 @@ _mesa_get_uniform_location(struct gl_context *ctx,
>     if (!found)
>        return GL_INVALID_INDEX;
>
> -   /* Since array_elements is 0 for non-arrays, this causes look-ups of 'a[0]'
> -    * to (correctly) fail if 'a' is not an array.
> +   /* If the uniform is an array, fail if the index is out of bounds.
> +    * (A negative index is caught above.)  This also fails if the uniform
> +    * is not an array, but the user is trying to index it, because
> +    * array_elements is zero and offset >= 0.
>      */
> -   if (array_lookup && shProg->UniformStorage[location].array_elements == 0) {
> +   if (array_lookup
> +        && offset >= shProg->UniformStorage[location].array_elements) {
>        return GL_INVALID_INDEX;
>     }
>
> --
> 1.7.7.3
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/mesa-dev