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