[Mesa-dev] [PATCH] i965: Check return value of screen->image.loader->getBuffers

[Anuj Phogat]

On Tue, May 31, 2016 at 12:24 AM, Tomasz Figa <tfiga at chromium.org> wrote:
> The images struct is an unitialized local variable on the stack. If the
> callback returns 0, the struct might not have been updated and so should
> be considered unitialized. Currently the code ignores the return value,
> which (depending on stack contents) might end up in reading a non-zero
> value from images.image_mask and dereferencing further fields.
Typo at two places in spelling of 'uninitialized'.

>
> Another solution would be to initialize image_mask with 0, but checking
> the return value seems more sensible and it is what Gallium is doing.
>
> Signed-off-by: Tomasz Figa <tfiga at chromium.org>
> ---
>  src/mesa/drivers/dri/i965/brw_context.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/src/mesa/drivers/dri/i965/brw_context.c b/src/mesa/drivers/dri/i965/brw_context.c
> index 2504dce..eda54ad 100644
> --- a/src/mesa/drivers/dri/i965/brw_context.c
> +++ b/src/mesa/drivers/dri/i965/brw_context.c
> @@ -1608,6 +1608,7 @@ intel_update_image_buffers(struct brw_context *brw, __DRIdrawable *drawable)
>     struct __DRIimageList images;
>     unsigned int format;
>     uint32_t buffer_mask = 0;
> +   int ret;
>
>     front_rb = intel_get_renderbuffer(fb, BUFFER_FRONT_LEFT);
>     back_rb = intel_get_renderbuffer(fb, BUFFER_BACK_LEFT);
> @@ -1627,12 +1628,14 @@ intel_update_image_buffers(struct brw_context *brw, __DRIdrawable *drawable)
>     if (back_rb)
>        buffer_mask |= __DRI_IMAGE_BUFFER_BACK;
>
> -   (*screen->image.loader->getBuffers) (drawable,
> +   ret = (*screen->image.loader->getBuffers) (drawable,
>                                          driGLFormatToImageFormat(format),
>                                          &drawable->dri2.stamp,
>                                          drawable->loaderPrivate,
>                                          buffer_mask,
>                                          &images);
> +   if (!ret)
> +      return;
>
>     if (images.image_mask & __DRI_IMAGE_BUFFER_FRONT) {
>        drawable->w = images.front->width;
> --
> 2.8.0.rc3.226.g39d4020
>
> _______________________________________________
> mesa-dev mailing list
> mesa-dev at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Rest looks good to me.

Reviewed-by: Anuj Phogat <anuj.phogat at gmail.com>