On 3 September 2018 at 12:39, Gert Wollny <[email protected]> wrote:
> Am Montag, den 03.09.2018, 10:52 +0100 schrieb Emil Velikov:
>> Hi Gert,
>>
>> On 3 September 2018 at 09:17, Gert Wollny <[email protected]>
>> wrote:
>> > The ressource storage must already be resized when the count is
>> > equal to
>> > the allocated size and the space for the handles must be resized
>> > accordingly.
>> >
>>
>> s/ressource/resource/
>>
>> > Fixes:
>> >   Crashes with
>> >   piglit/bin/map_buffer_range-invalidate CopyBufferSubData \
>> >                                increment-offset -auto -fbo
>> >
>> > Invalid write of size 8
>> > at 0xB72E4CF: virgl_drm_add_res (virgl_drm_winsys.c:629)
>> > by 0xB72E4CF: virgl_drm_emit_res (virgl_drm_winsys.c:663)
>> > by 0xB72A44A: virgl_encode_resource_copy_region
>> > (virgl_encode.c:776)
>> > by 0xB40CD12: st_copy_buffer_subdata (st_cb_bufferobjects.c:585)
>> > by 0xB244A3B: _mesa_CopyBufferSubData (bufferobj.c:2940)
>> > by 0x109A1E: upload (invalidate.c:169)
>> > by 0x109C2F: piglit_display (invalidate.c:215)
>> > by 0x4F80FBE: run_test (piglit_fbo_framework.c:52)
>> > by 0x4F66E5F: piglit_gl_test_run (piglit-framework-gl.c:229)
>> > by 0x10949D: main (invalidate.c:47)
>> > Address 0xbe07d30 is 0 bytes after a block of size 4,096 alloc'd
>> > at 0x4C31B25: calloc (in /usr/lib/valgrind/vgpreload_memcheck-
>> > amd64-
>> > linux.so)
>> > by 0xB72DAAF: virgl_drm_cmd_buf_create (virgl_drm_winsys.c:567)
>> >
>> > and
>> >
>> > Invalid write of size 4
>> > at 0xB72E4F0: virgl_drm_add_res (virgl_drm_winsys.c:631)
>> > by 0xB72E4F0: virgl_drm_emit_res (virgl_drm_winsys.c:663)
>> > by 0xB72A44A: virgl_encode_resource_copy_region
>> > (virgl_encode.c:776)
>> > by 0xB40CD12: st_copy_buffer_subdata (st_cb_bufferobjects.c:585)
>> > by 0xB244A3B: _mesa_CopyBufferSubData (bufferobj.c:2940)
>> > by 0x109A1E: upload (invalidate.c:169)
>> > by 0x109C2F: piglit_display (invalidate.c:215)
>> > by 0x4F80FBE: run_test (piglit_fbo_framework.c:52)
>> > by 0x4F66E5F: piglit_gl_test_run (piglit-framework-gl.c:229)
>> > by 0x10949D: main (invalidate.c:47)
>> > Address 0xbe08570 is 0 bytes after a block of size 2,048 alloc'd
>> > at 0x4C2FB0F: malloc (in /usr/lib/valgrind/vgpreload_memcheck-
>> > amd64-
>> > linux.so)
>> > by 0xB72DAC8: virgl_drm_cmd_buf_create (virgl_drm_winsys.c:572)
>> >
>> > Signed-off-by: Gert Wollny <[email protected]>
>> > ---
>> >  src/gallium/winsys/virgl/drm/virgl_drm_winsys.c | 7 ++++++-
>> >  1 file changed, 6 insertions(+), 1 deletion(-)
>> >
>> > diff --git a/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
>> > b/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
>> > index aad6430c41..2976b46484 100644
>> > --- a/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
>> > +++ b/src/gallium/winsys/virgl/drm/virgl_drm_winsys.c
>> > @@ -617,13 +617,18 @@ static void virgl_drm_add_res(struct
>> > virgl_drm_winsys *qdws,
>> >  {
>> >     unsigned hash = res->res_handle & (sizeof(cbuf-
>> > >is_handle_added)-1);
>> >
>> > -   if (cbuf->cres > cbuf->nres) {
>> > +   if (cbuf->cres >= cbuf->nres) {
>>
>> I'd leave the check as-is.
> But it's not correct,  the access cbuf->res_bo[cbuf->cres] is past the
> end when (cbuf->cres == cbuf->nres). Maybe I should put it in a
> separate patch to make this clearer (i.e. the first valgrind trace is
> because od the ">" and the second because res_hlist is not re-
> allocated.
>
That sounds reasonable - I would squash the off-by-one in both drm and
vtest at once.
Or keep it them separate if you prefer.

Either way, thanks for the correction.
Emil
_______________________________________________
mesa-dev mailing list
[email protected]
https://lists.freedesktop.org/mailman/listinfo/mesa-dev

Reply via email to