@samitouri / QOSamiQemu / commits / ab7183ed4e

hw/display/qxl: validate primary surface stride against width

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> Reviewed-by: Akihiko Odaki <odaki@rsg.ci.i.u-tokyo.ac.jp> Message-ID: <20260806094028.640676-1-marcandre.lureau@redhat.com>

Marc-André Lureau committed Aug 6, 2026 at 13:40 UTC ab7183ed4eecb4727532e3ffe5953d127e102c72

3 files changed +101 -26

hw/display/qxl-render.c

+40 -26

@@ -27,6 +27,7 @@

27 static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)

28 {

29 DisplaySurface *surface = qemu_console_surface(qxl->vga.con);

30 + int dst_stride = surface_stride(surface);

31 uint8_t *dst = surface_data(surface);

32 uint8_t *src;

33 int len, i;

@@ -45,14 +46,14 @@ static void qxl_blit(PCIQXLDevice *qxl, QXLRect *rect)

46 } else {

47 src += rect->top * qxl->guest_primary.abs_stride;

48 }

48 - dst += rect->top * qxl->guest_primary.abs_stride;

49 + dst += rect->top * dst_stride;

50 src += rect->left * qxl->guest_primary.bytes_pp;

51 dst += rect->left * qxl->guest_primary.bytes_pp;

52 len = (rect->right - rect->left) * qxl->guest_primary.bytes_pp;

54 for (i = rect->top; i < rect->bottom; i++) {

55 memcpy(dst, src, len);

55 - dst += qxl->guest_primary.abs_stride;

56 + dst += dst_stride;

57 src += qxl->guest_primary.qxl_stride;

58 }

59 }

@@ -61,30 +62,13 @@ void qxl_render_resize(PCIQXLDevice *qxl)

62 {

63 QXLSurfaceCreate *sc = &qxl->guest_primary.surface;

64 - qxl->guest_primary.qxl_stride = sc->stride;

65 - qxl->guest_primary.abs_stride = abs(sc->stride);

65 + qxl->guest_primary.qxl_stride = le32_to_cpu(sc->stride);

66 + qxl->guest_primary.abs_stride = abs(qxl->guest_primary.qxl_stride);

67 qxl->guest_primary.resized++;

67 - switch (sc->format) {

68 - case SPICE_SURFACE_FMT_16_555:

69 - qxl->guest_primary.bytes_pp = 2;

70 - qxl->guest_primary.bits_pp = 15;

71 - break;

72 - case SPICE_SURFACE_FMT_16_565:

73 - qxl->guest_primary.bytes_pp = 2;

74 - qxl->guest_primary.bits_pp = 16;

75 - break;

76 - case SPICE_SURFACE_FMT_32_xRGB:

77 - case SPICE_SURFACE_FMT_32_ARGB:

78 - qxl->guest_primary.bytes_pp = 4;

79 - qxl->guest_primary.bits_pp = 32;

80 - break;

81 - default:

82 - fprintf(stderr, "%s: unhandled format: %x\n", __func__,

83 - qxl->guest_primary.surface.format);

84 - qxl->guest_primary.bytes_pp = 4;

85 - qxl->guest_primary.bits_pp = 32;

86 - break;

87 - }

68 + /* fallback to default bpp if format is unknown */

69 + qxl_format_bpp(qxl, le32_to_cpu(sc->format),

70 + &qxl->guest_primary.bytes_pp,

71 + &qxl->guest_primary.bits_pp);

72 }

74 static void qxl_set_rect_to_surface(PCIQXLDevice *qxl, QXLRect *area)

@@ -101,15 +85,45 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl)

85 DisplaySurface *surface;

86 int width = qxl->guest_head0_width ?: qxl->guest_primary.surface.width;

87 int height = qxl->guest_head0_height ?: qxl->guest_primary.surface.height;

88 + uint64_t map_height;

89 int i;

91 + if (width <= 0 || height <= 0) {

92 + goto end;

93 + }

94 +

95 + if (qxl->guest_primary.bytes_pp > 0) {

96 + int max_width = qxl->guest_primary.abs_stride

97 + / qxl->guest_primary.bytes_pp;

98 + width = MIN(width, max_width);

99 + }

100 +

101 + if (qxl->guest_primary.qxl_stride < 0) {

102 + /* qxl_blit() uses the primary height to find the first scanline. */

103 + height = MIN(height, (int)qxl->guest_primary.surface.height);

104 + }

105 +

106 + if (qxl->guest_primary.abs_stride > 0) {

107 + int max_height = qxl->vgamem_size / qxl->guest_primary.abs_stride;

108 + height = MIN(height, max_height);

109 + }

110 +

111 + /*

112 + * height limits the visible update, while map_height is the guest memory

113 + * span validated by qxl_phys2virt(). With a negative stride qxl_blit()

114 + * addresses scanlines from the declared primary height, so a shorter

115 + * monitor still requires validating the full primary surface.

116 + */

117 + map_height = qxl->guest_primary.qxl_stride < 0 ?

118 + qxl->guest_primary.surface.height : height;

119 +

120 if (qxl->guest_primary.resized) {

121 qxl->guest_primary.resized = 0;

122 qxl->guest_primary.data = qxl_phys2virt(qxl,

123 qxl->guest_primary.surface.mem,

124 MEMSLOT_GROUP_GUEST,

125 qxl->guest_primary.abs_stride

112 - * height);

126 + * map_height);

127 if (!qxl->guest_primary.data) {

128 goto end;

129 }

hw/display/qxl.c

+59

@@ -1489,6 +1489,47 @@ static void qxl_create_guest_primary_complete(PCIQXLDevice *qxl)

1489 qxl_render_resize(qxl);

1490 }

1491

1492 +/*

1493 + * Convert a SpiceSurfaceFormat to bytes per pixel and bits per pixel.

1494 + *

1495 + * Only valid for surface suitable for rendering.

1496 + */

1497 +bool qxl_format_bpp(PCIQXLDevice *qxl, SpiceSurfaceFmt format,

1498 + uint32_t *bytes_pp, uint32_t *bits_pp)

1499 +{

1500 + uint32_t bypp = 4;

1501 + uint32_t bipp = 32;

1502 + bool ret = true;

1503 +

1504 + switch (format) {

1505 + case SPICE_SURFACE_FMT_16_555:

1506 + bypp = 2;

1507 + bipp = 15;

1508 + break;

1509 + case SPICE_SURFACE_FMT_16_565:

1510 + bypp = 2;

1511 + bipp = 16;

1512 + break;

1513 + case SPICE_SURFACE_FMT_32_xRGB:

1514 + case SPICE_SURFACE_FMT_32_ARGB:

1515 + bypp = 4;

1516 + bipp = 32;

1517 + break;

1518 + default:

1519 + ret = false;

1520 + qxl_set_guest_bug(qxl, "%s: unhandled format: %x", __func__, format);

1521 + }

1522 +

1523 + if (bytes_pp != NULL) {

1524 + *bytes_pp = bypp;

1525 + }

1526 + if (bits_pp != NULL) {

1527 + *bits_pp = bipp;

1528 + }

1529 +

1530 + return ret;

1531 +}

1532 +

1533 static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,

1534 qxl_async_io async)

1535 {

@@ -1496,6 +1537,7 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,

1537 QXLSurfaceCreate *sc = &qxl->guest_primary.surface;

1538 uint32_t requested_height = le32_to_cpu(sc->height);

1539 int requested_stride = le32_to_cpu(sc->stride);

1540 + uint32_t bytes_pp;

1541

1542 if (requested_stride == INT32_MIN ||

1543 abs(requested_stride) * (uint64_t)requested_height

@@ -1532,6 +1574,23 @@ static void qxl_create_guest_primary(PCIQXLDevice *qxl, int loadvm,

1574 return;

1575 }

1576

1577 + if (!qxl_format_bpp(qxl, surface.format, &bytes_pp, NULL)) {

1578 + return;

1579 + }

1580 +

1581 + if (surface.width == 0 || surface.height == 0) {

1582 + qxl_set_guest_bug(qxl, "%s: zero dimension %ux%u",

1583 + __func__, surface.width, surface.height);

1584 + return;

1585 + }

1586 +

1587 + if ((uint64_t)surface.width * bytes_pp > abs(surface.stride)) {

1588 + qxl_set_guest_bug(qxl, "%s: stride too small for width:"

1589 + " stride %d width %u bpp %u",

1590 + __func__, surface.stride, surface.width, bytes_pp);

1591 + return;

1592 + }

1593 +

1594 surface.mouse_mode = true;

1595 surface.group_id = MEMSLOT_GROUP_GUEST;

1596 if (loadvm) {

hw/display/qxl.h

@@ -181,6 +181,8 @@ void qxl_spice_oom(PCIQXLDevice *qxl);

181 void qxl_spice_reset_memslots(PCIQXLDevice *qxl);

182 void qxl_spice_reset_image_cache(PCIQXLDevice *qxl);

183 void qxl_spice_reset_cursor(PCIQXLDevice *qxl);

184 +bool qxl_format_bpp(PCIQXLDevice *qxl, SpiceSurfaceFmt format,

185 + uint32_t *bytes_pp, uint32_t *bits_pp);

186

187 /* qxl-logger.c */

188 int qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id);