[Mesa-dev] [PATCH] mesa: prevent SIGSEGV passing GL_NONE to _mesa_ReadBuffer()

[Brian Paul]

On 01/23/2014 07:31 AM, Ahmed Allam wrote:
> From: Ahmed Allam <ahmabdabd at hotmail.com>
>
>
> Fixes: https://bugs.freedesktop.org/show_bug.cgi?id=73956
>
> Signed-off-by: Ahmed Allam <ahmabdabd at hotmail.com>
> ---
>   src/mesa/main/buffers.c | 4 +++-
>   1 file changed, 3 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/main/buffers.c b/src/mesa/main/buffers.c
> index 2bdbf41..6c846cb 100644
> --- a/src/mesa/main/buffers.c
> +++ b/src/mesa/main/buffers.c
> @@ -623,7 +623,9 @@ _mesa_ReadBuffer(GLenum buffer)
>
>      if (buffer == GL_NONE) {
>         /* This is legal--it means that no buffer should be bound for
> reading. */
> -      srcBuffer = -1;
> +      _mesa_error(ctx, GL_INVALID_ENUM,
> +                           "glReadBuffer(buffer=0x%x)", buffer);
> +     return;
>      }
>      else {
>         /* general case / window-system framebuffer */


According to the GL 3.2 spec, GL_NONE is a legal value for 
glReadBuffer() and should not generate an error (as the comment says.) 
And I just verified it with NVIDIA's driver.

The spec says "ReadPixels generates an INVALID_OPERATION error if it 
attempts to select a color buffer while READ_BUFFER is NONE".  That's 
what we do.  However, it looks like NVIDIA's driver is doing something 
else.  It's actually doing glRead/CopyPixels w/out raising an error.

Anyway, can you try this patch:

diff --git a/src/mesa/state_tracker/st_cb_fbo.c 
b/src/mesa/state_tracker/st_cb_fbo.c
index 637f7ee..eca04b8 100644
--- a/src/mesa/state_tracker/st_cb_fbo.c
+++ b/src/mesa/state_tracker/st_cb_fbo.c
@@ -700,7 +700,8 @@ st_ReadBuffer(struct gl_context *ctx, GLenum buffer)
     (void) buffer;

     /* add the renderbuffer on demand */
-   st_manager_add_color_renderbuffer(st, fb, fb->_ColorReadBufferIndex);
+   if (fb->_ColorReadBufferIndex >= 0)
+      st_manager_add_color_renderbuffer(st, fb, fb->_ColorReadBufferIndex);
  }


-Brian