[PATCH v2 2/2] hw/display: check frame buffer can hold blob

Alex Bennée posted 2 patches 1 week, 4 days ago
[PATCH v2 2/2] hw/display: check frame buffer can hold blob
Posted by Alex Bennée 1 week, 4 days ago
Coverity reports (CID 1564769, 1564770) that we potentially overflow
by doing some 32x32 multiplies for something that ends up in a 64 bit
value. Fix this by first using stride for all lines and casting input
to uint64_t to ensure a 64 bit multiply is used.

Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>

---
v2
  - just use stride * height
  - tweak comment
---
 include/hw/virtio/virtio-gpu.h | 2 +-
 hw/display/virtio-gpu.c        | 3 +--
 2 files changed, 2 insertions(+), 3 deletions(-)

diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h
index 924eb8737e..8c977beebd 100644
--- a/include/hw/virtio/virtio-gpu.h
+++ b/include/hw/virtio/virtio-gpu.h
@@ -340,7 +340,7 @@ void virtio_gpu_update_cursor_data(VirtIOGPU *g,
  * blob_size: size of scanout blob data
  *
  * This will check we have enough space for the frame taking into
- * account that stride for all but the last line.
+ * account that stride.
  *
  * Returns true on success, otherwise logs guest error and returns false
  */
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index e7ca8fd1cf..7d22d03bbf 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -742,8 +742,7 @@ bool virtio_gpu_scanout_blob_to_fb(struct virtio_gpu_framebuffer *fb,
     fb->offset = ss->offsets[0] + ss->r.x * fb->bytes_pp + ss->r.y * fb->stride;
 
     fbend = fb->offset;
-    fbend += fb->stride * (ss->r.height - 1);
-    fbend += fb->bytes_pp * ss->r.width;
+    fbend += (uint64_t) fb->stride * ss->r.height;
 
     if (fbend > blob_size) {
         qemu_log_mask(LOG_GUEST_ERROR,
-- 
2.39.5


Re: [PATCH v2 2/2] hw/display: check frame buffer can hold blob
Posted by Dmitry Osipenko 1 week, 2 days ago
On 11/12/24 02:00, Alex Bennée wrote:
> Coverity reports (CID 1564769, 1564770) that we potentially overflow
> by doing some 32x32 multiplies for something that ends up in a 64 bit
> value. Fix this by first using stride for all lines and casting input
> to uint64_t to ensure a 64 bit multiply is used.
> 
> Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
> Cc: Dmitry Osipenko <dmitry.osipenko@collabora.com>

Reviewed-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>
Tested-by: Dmitry Osipenko <dmitry.osipenko@collabora.com>

-- 
Best regards,
Dmitry