On Aug 31, 2015 6:48 AM, "Chris Wilson" <<a href="mailto:chris@chris-wilson.co.uk">chris@chris-wilson.co.uk</a>> wrote: > > From: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>> > > If the user is specifying a subregion of a buffer using SKIP_ROWS and > SKIP_PIXELS, we must compute the buffer size carefully as the end of the > last row may be much shorter than stride*image_height*depth. The current > code tries to memcpy from beyond the end of the user data, for example > causing: > > ==28136== Invalid read of size 8 > ==28136== at 0x4C2D94E: memcpy@@GLIBC_2.14 (vg_replace_strmem.c:915) > ==28136== by 0xB4ADFE3: brw_bo_write (brw_batch.c:1856) > ==28136== by 0xB5B3531: brw_buffer_data (intel_buffer_objects.c:208) > ==28136== by 0xB0F6275: _mesa_buffer_data (bufferobj.c:1600) > ==28136== by 0xB0F6346: _mesa_BufferData (bufferobj.c:1631) > ==28136== by 0xB37A1EE: create_texture_for_pbo (meta_tex_subimage.c:103) > ==28136== by 0xB37A467: _mesa_meta_pbo_TexSubImage (meta_tex_subimage.c:176) > ==28136== by 0xB5C8D61: intelTexSubImage (intel_tex_subimage.c:195) > ==28136== by 0xB254AB4: _mesa_texture_sub_image (teximage.c:3654) > ==28136== by 0xB254C9F: texsubimage (teximage.c:3712) > ==28136== by 0xB2550E9: _mesa_TexSubImage2D (teximage.c:3853) > ==28136== by 0x401CA0: UploadTexSubImage2D (teximage.c:171) > ==28136== Address 0xd8bfbe0 is 0 bytes after a block of size 1,024 alloc'd > ==28136== at 0x4C28C20: malloc (vg_replace_malloc.c:296) > ==28136== by 0x402014: PerfDraw (teximage.c:270) > ==28136== by 0x402648: Draw (glmain.c:182) > ==28136== by 0x8385E63: ??? (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) > ==28136== by 0x83896C8: fgEnumWindows (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) > ==28136== by 0x838641C: glutMainLoopEvent (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) > ==28136== by 0x8386C1C: glutMainLoop (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) > ==28136== by 0x4019C1: main (glmain.c:262) > ==28136== > ==28136== Invalid read of size 8 > ==28136== at 0x4C2D940: memcpy@@GLIBC_2.14 (vg_replace_strmem.c:915) > ==28136== by 0xB4ADFE3: brw_bo_write (brw_batch.c:1856) > ==28136== by 0xB5B3531: brw_buffer_data (intel_buffer_objects.c:208) > ==28136== by 0xB0F6275: _mesa_buffer_data (bufferobj.c:1600) > ==28136== by 0xB0F6346: _mesa_BufferData (bufferobj.c:1631) > ==28136== by 0xB37A1EE: create_texture_for_pbo (meta_tex_subimage.c:103) > ==28136== by 0xB37A467: _mesa_meta_pbo_TexSubImage (meta_tex_subimage.c:176) > ==28136== by 0xB5C8D61: intelTexSubImage (intel_tex_subimage.c:195) > ==28136== by 0xB254AB4: _mesa_texture_sub_image (teximage.c:3654) > ==28136== by 0xB254C9F: texsubimage (teximage.c:3712) > ==28136== by 0xB2550E9: _mesa_TexSubImage2D (teximage.c:3853) > ==28136== by 0x401CA0: UploadTexSubImage2D (teximage.c:171) > ==28136== Address 0xd8bfbe8 is 8 bytes after a block of size 1,024 alloc'd > ==28136== at 0x4C28C20: malloc (vg_replace_malloc.c:296) > ==28136== by 0x402014: PerfDraw (teximage.c:270) > ==28136== by 0x402648: Draw (glmain.c:182) > ==28136== by 0x8385E63: ??? (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) > ==28136== by 0x83896C8: fgEnumWindows (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) > ==28136== by 0x838641C: glutMainLoopEvent (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) > ==28136== by 0x8386C1C: glutMainLoop (in /usr/lib/x86_64-linux-gnu/libglut.so.3.9.0) > ==28136== by 0x4019C1: main (glmain.c:262) > ==28136== > > Fixes: 7f396189f073d626c5f7a2c232dac92b65f5a23f How can this fix a SHA? What repo is this SHA in? > Cc: Jason Ekstrand <<a href="mailto:jason.ekstrand@intel.com">jason.ekstrand@intel.com</a>> > Cc: Neil Roberts <<a href="mailto:neil@linux.intel.com">neil@linux.intel.com</a>> > --- > src/mesa/drivers/common/meta_tex_subimage.c | 35 +++++++++++++++++------------ > 1 file changed, 21 insertions(+), 14 deletions(-) > > diff --git a/src/mesa/drivers/common/meta_tex_subimage.c b/src/mesa/drivers/common/meta_tex_subimage.c > index 16d8f5d..e2351c6 100644 > --- a/src/mesa/drivers/common/meta_tex_subimage.c > +++ b/src/mesa/drivers/common/meta_tex_subimage.c > @@ -46,8 +46,9 @@ > #include "varray.h" > > static struct gl_texture_image * > -create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, > - GLenum pbo_target, int width, int height, > +create_texture_for_pbo(struct gl_context *ctx, > + bool create_pbo, GLenum pbo_target, > + int dims, int width, int height, int depth, > GLenum format, GLenum type, const void *pixels, > const struct gl_pixelstore_attrib *packing, > GLuint *tmp_pbo, GLuint *tmp_tex) > @@ -73,8 +74,12 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, > return NULL; > > /* Account for SKIP_PIXELS, SKIP_ROWS, ALIGNMENT, and SKIP_IMAGES */ > - pixels = _mesa_image_address3d(packing, pixels, > - width, height, format, type, 0, 0, 0); > + uint32_t first_pixel = _mesa_image_offset(dims, packing, width, height, > + format, type, > + 0, 0, 0); > + uint32_t last_pixel = _mesa_image_offset(dims, packing, width, height, We should probably use size_t or ptrdiff_t here instead of uint32_t. > + format, type, > + depth-1, height-1, width); > row_stride = _mesa_image_row_stride(packing, width, format, type); > > if (_mesa_is_bufferobj(packing->BufferObj)) { > @@ -97,14 +102,18 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, > * data to avoid unnecessary data copying in _mesa_BufferData(). > */ > if (is_pixel_pack) > - _mesa_BufferData(pbo_target, row_stride * height, NULL, > + _mesa_BufferData(pbo_target, > + last_pixel - first_pixel, > + NULL, I'm trying to decide if the null here is correct... It's certainly the same as what we had before but I have a nagging feeling that this should be (void *)first_pixel. Other than that I really like the approach. It's nice and clean and guarantees that we get the right size. > GL_STREAM_READ); > else > - _mesa_BufferData(pbo_target, row_stride * height, pixels, > + _mesa_BufferData(pbo_target, > + last_pixel - first_pixel, > + (char *)pixels + first_pixel, > GL_STREAM_DRAW); > > buffer_obj = packing->BufferObj; > - pixels = NULL; > + first_pixel = 0; > > _mesa_BindBuffer(pbo_target, 0); > } > @@ -126,7 +135,7 @@ create_texture_for_pbo(struct gl_context *ctx, bool create_pbo, > read_only = pbo_target == GL_PIXEL_UNPACK_BUFFER; > if (!ctx->Driver.SetTextureStorageForBufferObject(ctx, tex_obj, > buffer_obj, > - (intptr_t)pixels, > + first_pixel, > row_stride, > read_only)) { > _mesa_DeleteTextures(1, tmp_tex); > @@ -147,7 +156,7 @@ _mesa_meta_pbo_TexSubImage(struct gl_context *ctx, GLuint dims, > const struct gl_pixelstore_attrib *packing) > { > GLuint pbo = 0, pbo_tex = 0, fbos[2] = { 0, 0 }; > - int full_height, image_height; > + int image_height; > struct gl_texture_image *pbo_tex_image; > GLenum status; > bool success = false; > @@ -171,11 +180,10 @@ _mesa_meta_pbo_TexSubImage(struct gl_context *ctx, GLuint dims, > * property. > */ > image_height = packing->ImageHeight == 0 ? height : packing->ImageHeight; > - full_height = image_height * (depth - 1) + height; > > pbo_tex_image = create_texture_for_pbo(ctx, create_pbo, > GL_PIXEL_UNPACK_BUFFER, > - width, full_height, > + dims, width, height, depth, > format, type, pixels, packing, > &pbo, &pbo_tex); > if (!pbo_tex_image) > @@ -277,7 +285,7 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx, GLuint dims, > const struct gl_pixelstore_attrib *packing) > { > GLuint pbo = 0, pbo_tex = 0, fbos[2] = { 0, 0 }; > - int full_height, image_height; > + int image_height; > struct gl_texture_image *pbo_tex_image; > struct gl_renderbuffer *rb = NULL; > GLenum dstBaseFormat = _mesa_unpack_format_to_base_format(format); > @@ -324,10 +332,9 @@ _mesa_meta_pbo_GetTexSubImage(struct gl_context *ctx, GLuint dims, > * property. > */ > image_height = packing->ImageHeight == 0 ? height : packing->ImageHeight; > - full_height = image_height * (depth - 1) + height; > > pbo_tex_image = create_texture_for_pbo(ctx, false, GL_PIXEL_PACK_BUFFER, > - width, full_height * depth, > + dims, width, height, depth, > format, type, pixels, packing, > &pbo, &pbo_tex); > if (!pbo_tex_image) > -- > 2.5.1 > > _______________________________________________ > mesa-dev mailing list > <a href="mailto:mesa-dev@lists.freedesktop.org">mesa-dev@lists.freedesktop.org</a> > <a href="http://lists.freedesktop.org/mailman/listinfo/mesa-dev">http://lists.freedesktop.org/mailman/listinfo/mesa-dev</a>