Mesa (main): util/vbuf: fix buffer overrun in attribute conversions

[GitLab Mirror]

Module: Mesa
Branch: main
Commit: 3c5b7dca30edc7365839944447476e0b8b56bf1f
URL:    http://cgit.freedesktop.org/mesa/mesa/commit/?id=3c5b7dca30edc7365839944447476e0b8b56bf1f

Author: Mike Blumenkrantz <michael.blumenkrantz at gmail.com>
Date:   Sun May 23 15:16:31 2021 -0400

util/vbuf: fix buffer overrun in attribute conversions

using the stride to calculate the buffer map size here is not correct, as
the stride is not necessarily equal to the attribute size, it's only the distance
between elements. for the case of overlapping elements (cts does this), the result
is that the attribute conversion will read past the end of the mapped src region

this is usually fine for drivers when they directly map the vertex buffer, as the memory
past the requested region is usually made available, but in the case where the readback
occurs using a staging resource sized exactly to the map region, this overflows and fails
silently, not even triggering a valgrind error because gpu memory lol

instead, add the size of the largest possible element, which will automatically be clamped
and ensure any staging buffers are correctly sized

Cc: mesa-stable at lists.freedesktop.org

Reviewed-by: Marek Olšák <marek.olsak at amd.com>
Part-of: <https://gitlab.freedesktop.org/mesa/mesa/-/merge_requests/10963>

---

 src/gallium/auxiliary/util/u_vbuf.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/src/gallium/auxiliary/util/u_vbuf.c b/src/gallium/auxiliary/util/u_vbuf.c
index 02a6ba00300..eed1c506641 100644
--- a/src/gallium/auxiliary/util/u_vbuf.c
+++ b/src/gallium/auxiliary/util/u_vbuf.c
@@ -441,7 +441,19 @@ u_vbuf_translate_buffers(struct u_vbuf *mgr, struct translate_key *key,
             static uint64_t dummy_buf[4] = { 0 };
             tr->set_buffer(tr, i, dummy_buf, 0, 0);
             continue;
-        }
+         }
+
+         if (vb->stride) {
+            /* the stride cannot be used to calculate the map size of the buffer,
+             * as it only determines the bytes between elements, not the size of elements
+             * themselves, meaning that if stride < element_size, the mapped size will
+             * be too small and conversion will overrun the map buffer
+             *
+             * instead, add the size of the largest possible attribute to ensure the map is large enough
+             */
+            unsigned last_offset = offset + size - vb->stride;
+            size = MAX2(size, last_offset + sizeof(double)*4);
+         }
 
          if (offset + size > vb->buffer.resource->width0) {
             /* Don't try to map past end of buffer.  This often happens when