[Piglit] [PATCH 1/2] oes_compressed_paletted_texture: prevent accessing buffer out of bounds

[Ian Romanick]

On 07/27/2015 02:13 PM, Nanley Chery wrote:
> From: Nanley Chery <nanley.g.chery at intel.com>
> 
> The buffer variable is too small to accomodate running the
> [Compressed]TexImage commands with the GL_PALETTE8_RGBA8_OES format
> (which needs 1024 bytes instead of the 768 allocated). Rely on the GL
               ^^^^
You mean (32 bits * 256 entries) + (16 * 16) = 1280, right? :)

> behavior of allocating unspecified memory when a NULL pointer is passed.

I'm a little bit skeptical about this.

    *   Should glCompressedTexSubImage2D be allowed for modifying paletted
        texture data.

        RESOLVED:  No, this would then require implementations that do not
        support paletted formats internally to also store the palette
        per texture.  This can be a memory overhead on platforms that are
        memory constrained.

I have a feeling that some implementations would generate an error for
this even though the spec does not call for one.

It also shouldn't matter for most (all?) of these cases.  These are all
trying to get the implementation to generate an error based on other
parameters to the functions.  The error should be generated before
dereferencing the pointer.

I think it's better to just increase the size of the buffer to (4 *
256) + (16 * 16).

> Signed-off-by: Nanley Chery <nanley.g.chery at intel.com>
> Cc: Ian Romanick <ian.d.romanick at intel.com>
> ---
>  .../oes_compressed_paletted_texture-api.c                     | 11 +++++------
>  1 file changed, 5 insertions(+), 6 deletions(-)
> 
> diff --git a/tests/spec/oes_compressed_paletted_texture/oes_compressed_paletted_texture-api.c b/tests/spec/oes_compressed_paletted_texture/oes_compressed_paletted_texture-api.c
> index 13b0bba..4d9f5b7 100644
> --- a/tests/spec/oes_compressed_paletted_texture/oes_compressed_paletted_texture-api.c
> +++ b/tests/spec/oes_compressed_paletted_texture/oes_compressed_paletted_texture-api.c
> @@ -63,7 +63,6 @@ piglit_display(void)
>  void
>  piglit_init(int argc, char **argv)
>  {
> -	GLubyte buffer[512 + (16 * 16)];
>  	GLuint tex;
>  	GLsizei size;
>  	unsigned i;
> @@ -98,7 +97,7 @@ piglit_init(int argc, char **argv)
>  	for (i = 0; i < ARRAY_SIZE(t); i++) {
>  		glTexImage2D(GL_TEXTURE_2D, 0, t[i].internal_format,
>  			     16, 16, 0,
> -			     t[i].internal_format, t[i].type, buffer);
> +			     t[i].internal_format, t[i].type, NULL);
>  #if defined(PIGLIT_USE_OPENGL_ES1) || defined(PIGLIT_USE_OPENGL_ES2)
>  		{
>  			GLenum error = glGetError();
> @@ -131,13 +130,13 @@ piglit_init(int argc, char **argv)
>  		 */
>  		glCompressedTexImage2D(GL_TEXTURE_2D, 0, t[i].internal_format,
>  					  16, 16, 0,
> -					  size + t[i].palette_size - 1, buffer);
> +					  size + t[i].palette_size - 1, NULL);
>  		if (!piglit_check_gl_error(GL_INVALID_VALUE))
>  			piglit_report_result(PIGLIT_FAIL);
>  
>  		glCompressedTexImage2D(GL_TEXTURE_2D, 0, t[i].internal_format,
>  					  16, 16, 0,
> -					  size + t[i].palette_size, buffer);
> +					  size + t[i].palette_size, NULL);
>  		if (!piglit_check_gl_error(GL_NO_ERROR))
>  			piglit_report_result(PIGLIT_FAIL);
>  
> @@ -155,7 +154,7 @@ piglit_init(int argc, char **argv)
>  		size = (8 * 8) >> t[i].shift;
>  		glCompressedTexImage2D(GL_TEXTURE_2D, 1, t[i].internal_format,
>  				       8, 8, 0,
> -				       size + t[i].palette_size, buffer);
> +				       size + t[i].palette_size, NULL);
>  		if (!piglit_check_gl_error(GL_INVALID_VALUE))
>  			piglit_report_result(PIGLIT_FAIL);
>  
> @@ -184,7 +183,7 @@ piglit_init(int argc, char **argv)
>  		size = (17 * 17) >> t[i].shift;
>  		glCompressedTexImage2D(GL_TEXTURE_2D, 0, t[i].internal_format,
>  				       16, 16, 1,
> -				       size + t[i].palette_size, buffer);
> +				       size + t[i].palette_size, NULL);
>  #if defined(PIGLIT_USE_OPENGL_ES1) || defined(PIGLIT_USE_OPENGL_ES2)
>  		if (!piglit_check_gl_error(GL_INVALID_VALUE))
>  			piglit_report_result(PIGLIT_FAIL);
>