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

[Eduardo Lima Mitev]

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.

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>
> 
>