[Mesa-dev] [PATCH v2] mesa: return early with error when shader source count is 0

[Nicolai Hähnle]

On 04.05.2017 23:18, Bartosz Tomczyk wrote:
> malloc can return valid pointer for zero size allocation,
> which causes OOB access later on
>
> v2: Return error if count is 0, clear previous shader source
> ---
>  src/mesa/main/shaderapi.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/src/mesa/main/shaderapi.c b/src/mesa/main/shaderapi.c
> index c41f006eb7..b39b7fd1c4 100644
> --- a/src/mesa/main/shaderapi.c
> +++ b/src/mesa/main/shaderapi.c
> @@ -1009,7 +1009,8 @@ shader_source(struct gl_shader *sh, const GLchar *source)
>     }
>
>  #ifdef DEBUG
> -   sh->SourceChecksum = util_hash_crc32(sh->Source, strlen(sh->Source));
> +   sh->SourceChecksum = sh->Source ?
> +      util_hash_crc32(sh->Source, strlen(sh->Source)) : 0xFFFFFFFF;
>  #endif
>  }
>
> @@ -1780,7 +1781,8 @@ _mesa_ShaderSource(GLuint shaderObj, GLsizei count,
>     if (!sh)
>        return;
>
> -   if (string == NULL) {
> +   if (string == NULL || count == 0) {
> +      shader_source(sh, NULL);
>        _mesa_error(ctx, GL_INVALID_VALUE, "glShaderSourceARB");
>        return;
>     }

NAK, for two reasons:

1. Neither of these error conditions is actually documented in the 
OpenGL spec as far as I can tell; the whole if-statement should be 
removed. As far as I can tell, passing string == NULL && count == 0 is 
valid, and should be equivalent to setting an empty source string.

2. When an OpenGL error other than GL_OUT_OF_MEMORY is flagged, the 
state of the GL context *must* remain unchanged.

I think replacing the whole thing with

    if (count == 0) {
       shader_source(sh, NULL);
       return;
    }

is fine.

Cheers,
Nicolai
-- 
Lerne, wie die Welt wirklich ist,
Aber vergiss niemals, wie sie sein sollte.