[Mesa-dev] [PATCH] main/getteximage: Take y-offset into account for memcpy size

[Jason Ekstrand]

On Nov 22, 2016 11:03, "Eduardo Lima Mitev" <elima at igalia.com> wrote:
>
> On 11/22/2016 06:43 PM, Jason Ekstrand wrote:
> > On Tue, Nov 22, 2016 at 9:11 AM, Eduardo Lima Mitev <elima at igalia.com
> > <mailto:elima at 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.

That's probably caught earlier but it would be worth a crawl through to
make sure.

> Thanks for reviewing!
>
> Eduardo
>
> >               }
> >               else {
> >                  GLuint row;
> >     --
> >     2.10.2
> >
> >     _______________________________________________
> >     mesa-dev mailing list
> >     mesa-dev at lists.freedesktop.org <mailto:
mesa-dev at lists.freedesktop.org>
> >     https://lists.freedesktop.org/mailman/listinfo/mesa-dev
> >     <https://lists.freedesktop.org/mailman/listinfo/mesa-dev>
> >
> >
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20161122/b005a8f4/attachment.html>