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

[Emil Velikov]

Hi Tomasz

On 13 June 2016 at 11:53, Tomasz Figa  wrote:
> The images struct is an uninitialized local variable on the stack. If the
> callback returns 0, the struct might not have been updated and so should
> be considered uninitialized. 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.
>
> 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.
>
> v2: fix typos in commit message,
> fix indentation,
> remove unnecessary parentheses and pointer dereference to keep line
> length reasonable.
>
The patch is perfectly reasonable and is
Reviewed-by: Emil Velikov 

I've added the stable tag and pushed this to master. Can you please
roll an identical fix for the i915 driver ?

Thanks
Emil
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

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

[Tomasz Figa]

The images struct is an uninitialized local variable on the stack. If the
callback returns 0, the struct might not have been updated and so should
be considered uninitialized. 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.

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.

v2: fix typos in commit message,
fix indentation,
remove unnecessary parentheses and pointer dereference to keep line
length reasonable.

Signed-off-by: Tomasz Figa 
---
 src/mesa/drivers/dri/i965/brw_context.c | 15 +--
 1 file changed, 9 insertions(+), 6 deletions(-)

diff --git a/src/mesa/drivers/dri/i965/brw_context.c 
b/src/mesa/drivers/dri/i965/brw_context.c
index 7bbc128..0861b6e 100644
--- a/src/mesa/drivers/dri/i965/brw_context.c
+++ b/src/mesa/drivers/dri/i965/brw_context.c
@@ -1645,6 +1645,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);
@@ -1664,12 +1665,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,
-driGLFormatToImageFormat(format),
->dri2.stamp,
-drawable->loaderPrivate,
-buffer_mask,
-);
+   ret = screen->image.loader->getBuffers(drawable,
+  driGLFormatToImageFormat(format),
+  >dri2.stamp,
+  drawable->loaderPrivate,
+  buffer_mask,
+  );
+   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@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev