hw/display/qxl.h | 2 ++ hw/display/qxl-render.c | 66 +++++++++++++++++++++++++---------------- hw/display/qxl.c | 59 ++++++++++++++++++++++++++++++++++++ 3 files changed, 101 insertions(+), 26 deletions(-)
From: Marc-André Lureau <marcandre.lureau@redhat.com>
The existing validation in qxl_create_guest_primary() checks that
abs(stride) * height fits in vgamem_size and that stride is 4-byte
aligned, but never checks that abs(stride) is large enough to hold one
row of pixels for the declared width and format.
A malicious guest can create a primary surface with a stride much
smaller than width * bytes_per_pixel (e.g. stride=4 for a 64-wide 32bpp
surface). The spice server rejects this via red_validate_surface(), but
the return is void and QEMU unconditionally proceeds to set up the local
rendering state. On the next display refresh, VNC or SDL reads width *
bytes_pp per scanline from a region backed by only stride bytes per
row, causing a host-side out-of-bounds read.
Add three checks in qxl_create_guest_primary() before creating the
surface:
- reject unknown surface formats
- reject zero width or height
- reject surfaces where abs(stride) < width * bytes_per_pixel
Also fix three related issues in qxl-render.c:
- qxl_blit() used abs_stride to advance the dst pointer into the
DisplaySurface, but when stride is negative the DisplaySurface is a
packed buffer whose stride may be smaller. Use surface_stride()
instead.
- qxl_render_update_area_unlocked() uses guest_head0_width (set via
QXL_IO_MONITORS_CONFIG_ASYNC) without validating it against
abs_stride, bypassing the new validation. Clamp the effective width
to abs_stride / bytes_pp to prevent out-of-bounds access while
tolerating the normal transient where the monitor config arrives
before the primary surface is resized to match.
- Similarly, guest_head0_height bypasses qxl_create_guest_primary()
validation. Without clamping, abs_stride * height can overrun
vgamem_size, and the product can also overflow 32 bits (e.g.
abs_stride=16 MiB, height=256 wraps to zero), defeating the
qxl_phys2virt() bounds check. Clamp height to
vgamem_size / abs_stride to prevent both.
While touch it, fix some endianness issues.
Fixes: CVE-2026-16271
Fixes: a19cbfb34642 ("spice: add qxl device")
Fixes: 979f7ef8966b ("qxl: use guest_monitor_config for local renderer.")
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3637
Reported-by: huntr bubble
Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
---
v5:
- fix negative stride mapping in qxl_render_update_area_unlocked()
- fix fields endianness in qxl_render_resize()
---
hw/display/qxl.h | 2 ++
hw/display/qxl-render.c | 66 +++++++++++++++++++++++++----------------
hw/display/qxl.c | 59 ++++++++++++++++++++++++++++++++++++
3 files changed, 101 insertions(+), 26 deletions(-)
diff --git a/hw/display/qxl.h b/hw/display/qxl.h
index 48d664f77736..6f5b96fe86cc 100644
--- a/hw/display/qxl.h
+++ b/hw/display/qxl.h
@@ -181,6 +181,8 @@ void qxl_spice_oom(PCIQXLDevice *qxl);
void qxl_spice_reset_memslots(PCIQXLDevice *qxl);
void qxl_spice_reset_image_cache(PCIQXLDevice *qxl);
void qxl_spice_reset_cursor(PCIQXLDevice *qxl);
+bool qxl_format_bpp(PCIQXLDevice *qxl, SpiceSurfaceFmt format,
+ uint32_t *bytes_pp, uint32_t *bits_pp);
/* qxl-logger.c */
int qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id);
diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
index 4799c9e8befd..348ddba76890 100644
--- a/hw/display/qxl-render.c
+++ b/hw/display/qxl-render.c
@@ -27,6 +27,7 @@
static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
{
DisplaySurface *surface = qemu_console_surface(qxl->vga.con);
+ int dst_stride = surface_stride(surface);
uint8_t *dst = surface_data(surface);
uint8_t *src;
int len, i;
@@ -45,14 +46,14 @@ static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
} else {
src += rect->top * qxl->guest_primary.abs_stride;
}
- dst += rect->top * qxl->guest_primary.abs_stride;
+ dst += rect->top * dst_stride;
src += rect->left * qxl->guest_primary.bytes_pp;
dst += rect->left * qxl->guest_primary.bytes_pp;
len = (rect->right - rect->left) * qxl->guest_primary.bytes_pp;
for (i = rect->top; i < rect->bottom; i++) {
memcpy(dst, src, len);
- dst += qxl->guest_primary.abs_stride;
+ dst += dst_stride;
src += qxl->guest_primary.qxl_stride;
}
}
@@ -61,30 +62,13 @@ void qxl_render_resize(PCIQXLDevice *qxl)
{
QXLSurfaceCreate *sc = &qxl->guest_primary.surface;
- qxl->guest_primary.qxl_stride = sc->stride;
- qxl->guest_primary.abs_stride = abs(sc->stride);
+ qxl->guest_primary.qxl_stride = le32_to_cpu(sc->stride);
+ qxl->guest_primary.abs_stride = abs(qxl->guest_primary.qxl_stride);
qxl->guest_primary.resized++;
- switch (sc->format) {
- case SPICE_SURFACE_FMT_16_555:
- qxl->guest_primary.bytes_pp = 2;
- qxl->guest_primary.bits_pp = 15;
- break;
- case SPICE_SURFACE_FMT_16_565:
- qxl->guest_primary.bytes_pp = 2;
- qxl->guest_primary.bits_pp = 16;
- break;
- case SPICE_SURFACE_FMT_32_xRGB:
- case SPICE_SURFACE_FMT_32_ARGB:
- qxl->guest_primary.bytes_pp = 4;
- qxl->guest_primary.bits_pp = 32;
- break;
- default:
- fprintf(stderr, "%s: unhandled format: %x\n", __func__,
- qxl->guest_primary.surface.format);
- qxl->guest_primary.bytes_pp = 4;
- qxl->guest_primary.bits_pp = 32;
- break;
- }
+ /* fallback to default bpp if format is unknown */
+ qxl_format_bpp(qxl, le32_to_cpu(sc->format),
+ &qxl->guest_primary.bytes_pp,
+ &qxl->guest_primary.bits_pp);
}
static void qxl_set_rect_to_surface(PCIQXLDevice *qxl, QXLRect *area)
@@ -101,15 +85,45 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
DisplaySurface *surface;
int width = qxl->guest_head0_width ?: qxl->guest_primary.surface.width;
int height = qxl->guest_head0_height ?: qxl->guest_primary.surface.height;
+ uint64_t map_height;
int i;
+ if (width <= 0 || height <= 0) {
+ goto end;
+ }
+
+ if (qxl->guest_primary.bytes_pp > 0) {
+ int max_width = qxl->guest_primary.abs_stride
+ / qxl->guest_primary.bytes_pp;
+ width = MIN(width, max_width);
+ }
+
+ if (qxl->guest_primary.qxl_stride < 0) {
+ /* qxl_blit() uses the primary height to find the first scanline. */
+ height = MIN(height, (int)qxl->guest_primary.surface.height);
+ }
+
+ if (qxl->guest_primary.abs_stride > 0) {
+ int max_height = qxl->vgamem_size / qxl->guest_primary.abs_stride;
+ height = MIN(height, max_height);
+ }
+
+ /*
+ * height limits the visible update, while map_height is the guest memory
+ * span validated by qxl_phys2virt(). With a negative stride qxl_blit()
+ * addresses scanlines from the declared primary height, so a shorter
+ * monitor still requires validating the full primary surface.
+ */
+ map_height = qxl->guest_primary.qxl_stride < 0 ?
+ qxl->guest_primary.surface.height : height;
+
if (qxl->guest_primary.resized) {
qxl->guest_primary.resized = 0;
qxl->guest_primary.data = qxl_phys2virt(qxl,
qxl->guest_primary.surface.mem,
MEMSLOT_GROUP_GUEST,
qxl->guest_primary.abs_stride
- * height);
+ * map_height);
if (!qxl->guest_primary.data) {
goto end;
}
diff --git a/hw/display/qxl.c b/hw/display/qxl.c
index b7d871b9ee33..384b8767b8e6 100644
--- a/hw/display/qxl.c
+++ b/hw/display/qxl.c
@@ -1489,6 +1489,47 @@ static void qxl_create_guest_primary_complete(PCIQXLDevice *qxl)
qxl_render_resize(qxl);
}
+/*
+ * Convert a SpiceSurfaceFormat to bytes per pixel and bits per pixel.
+ *
+ * Only valid for surface suitable for rendering.
+ */
+bool qxl_format_bpp(PCIQXLDevice *qxl, SpiceSurfaceFmt format,
+ uint32_t *bytes_pp, uint32_t *bits_pp)
+{
+ uint32_t bypp = 4;
+ uint32_t bipp = 32;
+ bool ret = true;
+
+ switch (format) {
+ case SPICE_SURFACE_FMT_16_555:
+ bypp = 2;
+ bipp = 15;
+ break;
+ case SPICE_SURFACE_FMT_16_565:
+ bypp = 2;
+ bipp = 16;
+ break;
+ case SPICE_SURFACE_FMT_32_xRGB:
+ case SPICE_SURFACE_FMT_32_ARGB:
+ bypp = 4;
+ bipp = 32;
+ break;
+ default:
+ ret = false;
+ qxl_set_guest_bug(qxl, "%s: unhandled format: %x", __func__, format);
+ }
+
+ if (bytes_pp != NULL) {
+ *bytes_pp = bypp;
+ }
+ if (bits_pp != NULL) {
+ *bits_pp = bipp;
+ }
+
+ return ret;
+}
+
static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
qxl_async_io async)
{
@@ -1496,6 +1537,7 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
QXLSurfaceCreate *sc = &qxl->guest_primary.surface;
uint32_t requested_height = le32_to_cpu(sc->height);
int requested_stride = le32_to_cpu(sc->stride);
+ uint32_t bytes_pp;
if (requested_stride == INT32_MIN ||
abs(requested_stride) * (uint64_t)requested_height
@@ -1532,6 +1574,23 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
return;
}
+ if (!qxl_format_bpp(qxl, surface.format, &bytes_pp, NULL)) {
+ return;
+ }
+
+ if (surface.width == 0 || surface.height == 0) {
+ qxl_set_guest_bug(qxl, "%s: zero dimension %ux%u",
+ __func__, surface.width, surface.height);
+ return;
+ }
+
+ if ((uint64_t)surface.width * bytes_pp > abs(surface.stride)) {
+ qxl_set_guest_bug(qxl, "%s: stride too small for width:"
+ " stride %d width %u bpp %u",
+ __func__, surface.stride, surface.width, bytes_pp);
+ return;
+ }
+
surface.mouse_mode = true;
surface.group_id = MEMSLOT_GROUP_GUEST;
if (loadvm) {
--
2.55.0
On 2026/08/06 18:40, marcandre.lureau@redhat.com wrote:
> From: Marc-André Lureau <marcandre.lureau@redhat.com>
>
> The existing validation in qxl_create_guest_primary() checks that
> abs(stride) * height fits in vgamem_size and that stride is 4-byte
> aligned, but never checks that abs(stride) is large enough to hold one
> row of pixels for the declared width and format.
>
> A malicious guest can create a primary surface with a stride much
> smaller than width * bytes_per_pixel (e.g. stride=4 for a 64-wide 32bpp
> surface). The spice server rejects this via red_validate_surface(), but
> the return is void and QEMU unconditionally proceeds to set up the local
> rendering state. On the next display refresh, VNC or SDL reads width *
> bytes_pp per scanline from a region backed by only stride bytes per
> row, causing a host-side out-of-bounds read.
>
> Add three checks in qxl_create_guest_primary() before creating the
> surface:
> - reject unknown surface formats
> - reject zero width or height
> - reject surfaces where abs(stride) < width * bytes_per_pixel
>
> Also fix three related issues in qxl-render.c:
> - qxl_blit() used abs_stride to advance the dst pointer into the
> DisplaySurface, but when stride is negative the DisplaySurface is a
> packed buffer whose stride may be smaller. Use surface_stride()
> instead.
> - qxl_render_update_area_unlocked() uses guest_head0_width (set via
> QXL_IO_MONITORS_CONFIG_ASYNC) without validating it against
> abs_stride, bypassing the new validation. Clamp the effective width
> to abs_stride / bytes_pp to prevent out-of-bounds access while
> tolerating the normal transient where the monitor config arrives
> before the primary surface is resized to match.
> - Similarly, guest_head0_height bypasses qxl_create_guest_primary()
> validation. Without clamping, abs_stride * height can overrun
> vgamem_size, and the product can also overflow 32 bits (e.g.
> abs_stride=16 MiB, height=256 wraps to zero), defeating the
> qxl_phys2virt() bounds check. Clamp height to
> vgamem_size / abs_stride to prevent both.
>
> While touch it, fix some endianness issues.
>
> Fixes: CVE-2026-16271
> Fixes: a19cbfb34642 ("spice: add qxl device")
> Fixes: 979f7ef8966b ("qxl: use guest_monitor_config for local renderer.")
> Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3637
> Reported-by: huntr bubble
> Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
I believe this version comprehensively fixes the memory safety issue:
Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp>
Regards,
Akihiko Odaki
> ---
> v5:
> - fix negative stride mapping in qxl_render_update_area_unlocked()
> - fix fields endianness in qxl_render_resize()
> ---
> hw/display/qxl.h | 2 ++
> hw/display/qxl-render.c | 66 +++++++++++++++++++++++++----------------
> hw/display/qxl.c | 59 ++++++++++++++++++++++++++++++++++++
> 3 files changed, 101 insertions(+), 26 deletions(-)
>
> diff --git a/hw/display/qxl.h b/hw/display/qxl.h
> index 48d664f77736..6f5b96fe86cc 100644
> --- a/hw/display/qxl.h
> +++ b/hw/display/qxl.h
> @@ -181,6 +181,8 @@ void qxl_spice_oom(PCIQXLDevice *qxl);
> void qxl_spice_reset_memslots(PCIQXLDevice *qxl);
> void qxl_spice_reset_image_cache(PCIQXLDevice *qxl);
> void qxl_spice_reset_cursor(PCIQXLDevice *qxl);
> +bool qxl_format_bpp(PCIQXLDevice *qxl, SpiceSurfaceFmt format,
> + uint32_t *bytes_pp, uint32_t *bits_pp);
>
> /* qxl-logger.c */
> int qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id);
> diff --git a/hw/display/qxl-render.c b/hw/display/qxl-render.c
> index 4799c9e8befd..348ddba76890 100644
> --- a/hw/display/qxl-render.c
> +++ b/hw/display/qxl-render.c
> @@ -27,6 +27,7 @@
> static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
> {
> DisplaySurface *surface = qemu_console_surface(qxl->vga.con);
> + int dst_stride = surface_stride(surface);
> uint8_t *dst = surface_data(surface);
> uint8_t *src;
> int len, i;
> @@ -45,14 +46,14 @@ static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)
> } else {
> src += rect->top * qxl->guest_primary.abs_stride;
> }
> - dst += rect->top * qxl->guest_primary.abs_stride;
> + dst += rect->top * dst_stride;
> src += rect->left * qxl->guest_primary.bytes_pp;
> dst += rect->left * qxl->guest_primary.bytes_pp;
> len = (rect->right - rect->left) * qxl->guest_primary.bytes_pp;
>
> for (i = rect->top; i < rect->bottom; i++) {
> memcpy(dst, src, len);
> - dst += qxl->guest_primary.abs_stride;
> + dst += dst_stride;
> src += qxl->guest_primary.qxl_stride;
> }
> }
> @@ -61,30 +62,13 @@ void qxl_render_resize(PCIQXLDevice *qxl)
> {
> QXLSurfaceCreate *sc = &qxl->guest_primary.surface;
>
> - qxl->guest_primary.qxl_stride = sc->stride;
> - qxl->guest_primary.abs_stride = abs(sc->stride);
> + qxl->guest_primary.qxl_stride = le32_to_cpu(sc->stride);
> + qxl->guest_primary.abs_stride = abs(qxl->guest_primary.qxl_stride);
> qxl->guest_primary.resized++;
> - switch (sc->format) {
> - case SPICE_SURFACE_FMT_16_555:
> - qxl->guest_primary.bytes_pp = 2;
> - qxl->guest_primary.bits_pp = 15;
> - break;
> - case SPICE_SURFACE_FMT_16_565:
> - qxl->guest_primary.bytes_pp = 2;
> - qxl->guest_primary.bits_pp = 16;
> - break;
> - case SPICE_SURFACE_FMT_32_xRGB:
> - case SPICE_SURFACE_FMT_32_ARGB:
> - qxl->guest_primary.bytes_pp = 4;
> - qxl->guest_primary.bits_pp = 32;
> - break;
> - default:
> - fprintf(stderr, "%s: unhandled format: %x\n", __func__,
> - qxl->guest_primary.surface.format);
> - qxl->guest_primary.bytes_pp = 4;
> - qxl->guest_primary.bits_pp = 32;
> - break;
> - }
> + /* fallback to default bpp if format is unknown */
> + qxl_format_bpp(qxl, le32_to_cpu(sc->format),
> + &qxl->guest_primary.bytes_pp,
> + &qxl->guest_primary.bits_pp);
> }
>
> static void qxl_set_rect_to_surface(PCIQXLDevice *qxl, QXLRect *area)
> @@ -101,15 +85,45 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
> DisplaySurface *surface;
> int width = qxl->guest_head0_width ?: qxl->guest_primary.surface.width;
> int height = qxl->guest_head0_height ?: qxl->guest_primary.surface.height;
> + uint64_t map_height;
> int i;
>
> + if (width <= 0 || height <= 0) {
> + goto end;
> + }
> +
> + if (qxl->guest_primary.bytes_pp > 0) {
> + int max_width = qxl->guest_primary.abs_stride
> + / qxl->guest_primary.bytes_pp;
> + width = MIN(width, max_width);
> + }
> +
> + if (qxl->guest_primary.qxl_stride < 0) {
> + /* qxl_blit() uses the primary height to find the first scanline. */
> + height = MIN(height, (int)qxl->guest_primary.surface.height);
> + }
> +
> + if (qxl->guest_primary.abs_stride > 0) {
> + int max_height = qxl->vgamem_size / qxl->guest_primary.abs_stride;
> + height = MIN(height, max_height);
> + }
> +
> + /*
> + * height limits the visible update, while map_height is the guest memory
> + * span validated by qxl_phys2virt(). With a negative stride qxl_blit()
> + * addresses scanlines from the declared primary height, so a shorter
> + * monitor still requires validating the full primary surface.
> + */
> + map_height = qxl->guest_primary.qxl_stride < 0 ?
> + qxl->guest_primary.surface.height : height;
> +
> if (qxl->guest_primary.resized) {
> qxl->guest_primary.resized = 0;
> qxl->guest_primary.data = qxl_phys2virt(qxl,
> qxl->guest_primary.surface.mem,
> MEMSLOT_GROUP_GUEST,
> qxl->guest_primary.abs_stride
> - * height);
> + * map_height);
> if (!qxl->guest_primary.data) {
> goto end;
> }
> diff --git a/hw/display/qxl.c b/hw/display/qxl.c
> index b7d871b9ee33..384b8767b8e6 100644
> --- a/hw/display/qxl.c
> +++ b/hw/display/qxl.c
> @@ -1489,6 +1489,47 @@ static void qxl_create_guest_primary_complete(PCIQXLDevice *qxl)
> qxl_render_resize(qxl);
> }
>
> +/*
> + * Convert a SpiceSurfaceFormat to bytes per pixel and bits per pixel.
> + *
> + * Only valid for surface suitable for rendering.
> + */
> +bool qxl_format_bpp(PCIQXLDevice *qxl, SpiceSurfaceFmt format,
> + uint32_t *bytes_pp, uint32_t *bits_pp)
> +{
> + uint32_t bypp = 4;
> + uint32_t bipp = 32;
> + bool ret = true;
> +
> + switch (format) {
> + case SPICE_SURFACE_FMT_16_555:
> + bypp = 2;
> + bipp = 15;
> + break;
> + case SPICE_SURFACE_FMT_16_565:
> + bypp = 2;
> + bipp = 16;
> + break;
> + case SPICE_SURFACE_FMT_32_xRGB:
> + case SPICE_SURFACE_FMT_32_ARGB:
> + bypp = 4;
> + bipp = 32;
> + break;
> + default:
> + ret = false;
> + qxl_set_guest_bug(qxl, "%s: unhandled format: %x", __func__, format);
> + }
> +
> + if (bytes_pp != NULL) {
> + *bytes_pp = bypp;
> + }
> + if (bits_pp != NULL) {
> + *bits_pp = bipp;
> + }
> +
> + return ret;
> +}
> +
> static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
> qxl_async_io async)
> {
> @@ -1496,6 +1537,7 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
> QXLSurfaceCreate *sc = &qxl->guest_primary.surface;
> uint32_t requested_height = le32_to_cpu(sc->height);
> int requested_stride = le32_to_cpu(sc->stride);
> + uint32_t bytes_pp;
>
> if (requested_stride == INT32_MIN ||
> abs(requested_stride) * (uint64_t)requested_height
> @@ -1532,6 +1574,23 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,
> return;
> }
>
> + if (!qxl_format_bpp(qxl, surface.format, &bytes_pp, NULL)) {
> + return;
> + }
> +
> + if (surface.width == 0 || surface.height == 0) {
> + qxl_set_guest_bug(qxl, "%s: zero dimension %ux%u",
> + __func__, surface.width, surface.height);
> + return;
> + }
> +
> + if ((uint64_t)surface.width * bytes_pp > abs(surface.stride)) {
> + qxl_set_guest_bug(qxl, "%s: stride too small for width:"
> + " stride %d width %u bpp %u",
> + __func__, surface.stride, surface.width, bytes_pp);
> + return;
> + }
> +
> surface.mouse_mode = true;
> surface.group_id = MEMSLOT_GROUP_GUEST;
> if (loadvm) {
© 2016 - 2026 Red Hat, Inc.