[virglrenderer-devel] renderer: fix memory corruption when using glBufferSubData

[Gert Wollny]

Am Montag, den 11.06.2018, 11:47 -0400 schrieb Ramin Azarmehr:
> Bug: if in vrend_renderer.c the "use_sub_data" is set to 1 (default
> is 0) to use glBufferSubData() for transferring IOVs, some memory
> corruptions and artifacts occur. 
> 
> Reason: the second parameter in glBufferSubData() is the offset, but
> in vrend_read_from_iovec_cb() function in iov.c, the "count" is
> passed to it causing to possibly write beyond the buffer boundary (or
> at wrong offset). The following patch fixes the problem. Also, I've
> made simple changes to other functions in iov.c to make the code
> consistent.
It would probaly have been better to separate the chunk that fixes the
bug from the changes that make it more consistent to make it easier to
figure out what to focus on in the patch. 

Apart from that the patch is 
Reviewed-By: Gert Wollny <gert.wollny at collabora.com>


> P.S.: our experiments on both Intel Atom and aarch64 platforms shows
> that glBufferSubData() performs faster in function
> vrend_renderer_transfer_write_iov() than using the combination of
> glMapBufferRange()+memcpy(). This improvement could be attributed to
> better cache utilization (write-combining), NEON-assisted mem-copying 
> in aarch64, and etc. You may set use_sub_data=1 at your discretion.
> 
> ---
> diff --git a/src/iov.c b/src/iov.c
> index aae995b..273e296 100644
> --- a/src/iov.c
> +++ b/src/iov.c
> @@ -64,7 +64,7 @@ size_t vrend_read_from_iovec(const struct iovec
> *iov, int iovlen,
>      if (iov->iov_len > offset) {
>        len = iov->iov_len - offset;
>        
> -      if (count < iov->iov_len - offset) len = count;
> +      if (count < len) len = count;
>  
>        memcpy(buf, (char*)iov->iov_base + offset, len);
>        read += len;
> @@ -93,7 +93,7 @@ size_t vrend_write_to_iovec(const struct iovec
> *iov, int iovlen,
>      if (iov->iov_len > offset) {
>        len = iov->iov_len - offset;
>  
> -      if (count < iov->iov_len - offset) len = count;
> +      if (count < len) len = count;
>  
>        memcpy((char*)iov->iov_base + offset, buf, len);
>        written += len;
> @@ -120,11 +120,11 @@ size_t vrend_read_from_iovec_cb(const struct
> iovec *iov, int iovlen,
>  
>    while (count > 0 && iovlen > 0) {
>      if (iov->iov_len > offset) {
> -      len = iov->iov_len;
> -      
> -      if (count < iov->iov_len - offset) len = count;
> +      len = iov->iov_len - offset;
> +
> +      if (count < len) len = count;
>  
> -      (*iocb)(cookie, count, (char*)iov->iov_base + offset, len);
> +      (*iocb)(cookie, read, (char*)iov->iov_base + offset, len);
>        read += len;
>  
>        count -= len;
> _______________________________________________
> virglrenderer-devel mailing list
> virglrenderer-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/virglrenderer-devel