[PATCH] ui/pixman: fix zero rowstride in qemu_pixman_image_new_shareable()

marcandre.lureau@redhat.com posted 1 patch 1 month, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/20260611113614.1935094-1-marcandre.lureau@redhat.com
Maintainers: "Marc-André Lureau" <marcandre.lureau@redhat.com>
include/ui/pixman-minimal.h      | 25 +++-------------
include/ui/qemu-pixman-helpers.h | 49 ++++++++++++++++++++++++++++++++
include/ui/qemu-pixman.h         |  1 +
ui/qemu-pixman.c                 |  8 +++++-
4 files changed, 61 insertions(+), 22 deletions(-)
create mode 100644 include/ui/qemu-pixman-helpers.h
[PATCH] ui/pixman: fix zero rowstride in qemu_pixman_image_new_shareable()
Posted by marcandre.lureau@redhat.com 1 month, 2 weeks ago
From: Marc-André Lureau <marcandre.lureau@redhat.com>

qemu_create_displaysurface_from() callers such as xlnx_dp.c pass
linesize=0 with data=NULL, relying on pixman to compute the stride.
Since 1ff788db978 ("ui: use a shareable type"), the data=NULL path
goes through qemu_pixman_image_new_shareable() which computes
size = height * rowstride_bytes, resulting in a zero-size allocation
and an abort in qemu_memfd_alloc().

Introduce qemu-pixman-helpers.h with overflow-safe stride and buffer
size computation (matching pixman's create_bits() formula), and use it
from both qemu_pixman_image_new_shareable() and pixman-minimal's
create_bits().

Reported-by: Peter Maydell <peter.maydell@linaro.org>
Fixes: 1ff788db9781 ("ui: use a shareable type")
Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/ui/pixman-minimal.h      | 25 +++-------------
 include/ui/qemu-pixman-helpers.h | 49 ++++++++++++++++++++++++++++++++
 include/ui/qemu-pixman.h         |  1 +
 ui/qemu-pixman.c                 |  8 +++++-
 4 files changed, 61 insertions(+), 22 deletions(-)
 create mode 100644 include/ui/qemu-pixman-helpers.h

diff --git a/include/ui/pixman-minimal.h b/include/ui/pixman-minimal.h
index 6dd7de1c7e7..680cc809ace 100644
--- a/include/ui/pixman-minimal.h
+++ b/include/ui/pixman-minimal.h
@@ -113,6 +113,8 @@ typedef struct pixman_color {
     uint16_t    alpha;
 } pixman_color_t;
 
+#include "qemu-pixman-helpers.h"
+
 static inline uint32_t *create_bits(pixman_format_code_t format,
                                     int width,
                                     int height,
@@ -120,28 +122,9 @@ static inline uint32_t *create_bits(pixman_format_code_t format,
 {
     int stride = 0;
     size_t buf_size = 0;
-    int bpp = PIXMAN_FORMAT_BPP(format);
-
-    /*
-     * Calculate the following while checking for overflow truncation:
-     * stride = ((width * bpp + 0x1f) >> 5) * sizeof(uint32_t);
-     */
-
-    if (unlikely(__builtin_mul_overflow(width, bpp, &stride))) {
-        return NULL;
-    }
-
-    if (unlikely(__builtin_add_overflow(stride, 0x1f, &stride))) {
-        return NULL;
-    }
-
-    stride >>= 5;
-
-    stride *= sizeof(uint32_t);
 
-    if (unlikely(__builtin_mul_overflow((size_t) height,
-                                        (size_t) stride,
-                                        &buf_size))) {
+    if (!qemu_pixman_image_calc_size(format, width, height,
+                                     &stride, &buf_size)) {
         return NULL;
     }
 
diff --git a/include/ui/qemu-pixman-helpers.h b/include/ui/qemu-pixman-helpers.h
new file mode 100644
index 00000000000..3e247a37946
--- /dev/null
+++ b/include/ui/qemu-pixman-helpers.h
@@ -0,0 +1,49 @@
+/* SPDX-License-Identifier: MIT */
+/*
+ * Pixman stride and buffer size helpers.
+ * Expects PIXMAN_FORMAT_BPP() and pixman_format_code_t to be
+ * already defined (by either <pixman.h> or "pixman-minimal.h").
+ */
+
+#ifndef QEMU_PIXMAN_HELPERS_H
+#define QEMU_PIXMAN_HELPERS_H
+
+/*
+ * Compute the row stride for a pixman image, aligned to sizeof(uint32_t),
+ * as pixman does. Returns -1 on integer overflow.
+ */
+static inline int qemu_pixman_stride(pixman_format_code_t format, int width)
+{
+    int stride;
+
+    if (unlikely(__builtin_mul_overflow(width, PIXMAN_FORMAT_BPP(format),
+                                        &stride)) ||
+        unlikely(__builtin_add_overflow(stride, 31, &stride))) {
+        return -1;
+    }
+    return (stride / 32) * sizeof(uint32_t);
+}
+
+/*
+ * Compute stride and buffer size for a pixman image.
+ * If *rowstride_bytes is 0, compute it from format and width
+ * (aligned to sizeof(uint32_t), as pixman does).
+ * Returns false on integer overflow.
+ */
+static inline bool qemu_pixman_image_calc_size(pixman_format_code_t format,
+                                               int width, int height,
+                                               int *rowstride_bytes,
+                                               size_t *buf_size)
+{
+    if (!*rowstride_bytes) {
+        *rowstride_bytes = qemu_pixman_stride(format, width);
+        if (*rowstride_bytes < 0) {
+            return false;
+        }
+    }
+
+    return likely(!__builtin_mul_overflow((size_t)height,
+                                          (size_t)*rowstride_bytes, buf_size));
+}
+
+#endif /* QEMU_PIXMAN_HELPERS_H */
diff --git a/include/ui/qemu-pixman.h b/include/ui/qemu-pixman.h
index 4bc7a59698e..1d042c7f634 100644
--- a/include/ui/qemu-pixman.h
+++ b/include/ui/qemu-pixman.h
@@ -8,6 +8,7 @@
 
 #ifdef CONFIG_PIXMAN
 #include <pixman.h>
+#include "qemu-pixman-helpers.h"
 #else
 #include "pixman-minimal.h"
 #endif
diff --git a/ui/qemu-pixman.c b/ui/qemu-pixman.c
index aea09755b94..9dc606f692a 100644
--- a/ui/qemu-pixman.c
+++ b/ui/qemu-pixman.c
@@ -320,12 +320,18 @@ qemu_pixman_image_new_shareable(pixman_image_t **image,
                                 Error **errp)
 {
     ERRP_GUARD();
-    size_t size = height * rowstride_bytes;
+    size_t size;
     void *bits = NULL;
 
     g_return_val_if_fail(image != NULL, false);
     g_return_val_if_fail(handle != NULL, false);
 
+    if (!qemu_pixman_image_calc_size(format, width, height,
+                                     &rowstride_bytes, &size)) {
+        error_setg(errp, "Image dimensions overflow");
+        return false;
+    }
+
     bits = qemu_pixman_shareable_alloc(name, size, handle, errp);
     if (!bits) {
         return false;
-- 
2.54.0


Re: [PATCH] ui/pixman: fix zero rowstride in qemu_pixman_image_new_shareable()
Posted by Philippe Mathieu-Daudé 1 month, 2 weeks ago
On 11/6/26 13:36, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> qemu_create_displaysurface_from() callers such as xlnx_dp.c pass
> linesize=0 with data=NULL, relying on pixman to compute the stride.
> Since 1ff788db978 ("ui: use a shareable type"), the data=NULL path
> goes through qemu_pixman_image_new_shareable() which computes
> size = height * rowstride_bytes, resulting in a zero-size allocation
> and an abort in qemu_memfd_alloc().
> 
> Introduce qemu-pixman-helpers.h with overflow-safe stride and buffer
> size computation (matching pixman's create_bits() formula), and use it
> from both qemu_pixman_image_new_shareable() and pixman-minimal's
> create_bits().
> 
> Reported-by: Peter Maydell <peter.maydell@linaro.org>
> Fixes: 1ff788db9781 ("ui: use a shareable type")
> Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> ---
>   include/ui/pixman-minimal.h      | 25 +++-------------
>   include/ui/qemu-pixman-helpers.h | 49 ++++++++++++++++++++++++++++++++
>   include/ui/qemu-pixman.h         |  1 +
>   ui/qemu-pixman.c                 |  8 +++++-
>   4 files changed, 61 insertions(+), 22 deletions(-)
>   create mode 100644 include/ui/qemu-pixman-helpers.h

Nice.
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>