On 11/22/2016 06:43 PM, Jason Ekstrand wrote: > On Tue, Nov 22, 2016 at 9:11 AM, Eduardo Lima Mitev <el...@igalia.com > <mailto:el...@igalia.com>> wrote: > > In get_tex_memcpy, when copying texture data directly from source > to destination (when row strides match for both src and dst), the > block size is currently calculated as 'bytes-per-row * image-height', > ignoring the given y-offset argument. > > This can cause a read past the end of the mapped buffer, leading to > a segfault. > > Fixes CTS test (from crash to pass): > * GL45-CTS/get_texture_sub_image/functional_test > --- > src/mesa/main/texgetimage.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/src/mesa/main/texgetimage.c b/src/mesa/main/texgetimage.c > index b900278..a783ed5 100644 > --- a/src/mesa/main/texgetimage.c > +++ b/src/mesa/main/texgetimage.c > @@ -654,7 +654,7 @@ get_tex_memcpy(struct gl_context *ctx, > > if (src) { > if (bytesPerRow == dstRowStride && bytesPerRow == > srcRowStride) { > - memcpy(dst, src, bytesPerRow * texImage->Height); > + memcpy(dst, src, bytesPerRow * (texImage->Height - > yoffset)); > > > Why not use the height parameter that gets passed in. That seems more > correct. >
Indeed, that's the correct thing to do here. Then I wonder if we should clamp height (and width) to the texture size minus y-offset (and x-offset). In case the region exceeds texture size, the extra memory will have undefined data, but we don't crash the driver. Or maybe that's caught earlier. Thanks for reviewing! Eduardo > } > else { > GLuint row; > -- > 2.10.2 > > _______________________________________________ > mesa-dev mailing list > mesa-dev@lists.freedesktop.org <mailto:mesa-dev@lists.freedesktop.org> > https://lists.freedesktop.org/mailman/listinfo/mesa-dev > <https://lists.freedesktop.org/mailman/listinfo/mesa-dev> > > _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev