friedkiwi/qemu
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

hw/display/qxl: fix TOCTOU in cursor chunk data_size handling

Browse Source 
Snapshot chunk.data_size into a host-local variable before passing it to
qxl_phys2virt() for validation, and pass it through qxl_cursor() and
qxl_unpack_chunks() so that no subsequent code re-reads the field.

Without this, a racing vCPU can inflate data_size between the
qxl_phys2virt() validation and the memcpy in qxl_unpack_chunks(),
causing a source read past the validated region. In practice the read
stays within the guest's own VRAM mmap, so the impact is limited.

Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/3757
Reported-by: Feifan Qian <bea1e@proton.me>
Signed-off-by: Marc-Andre Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
Message-ID: <20260710134352.2313675-1-marcandre.lureau@redhat.com>
Signed-off-by: Philippe Mathieu-Daudé <philmd@oss.qualcomm.com>
This commit is contained in:
Marc-André Lureau
2026-07-10 17:43:52 +04:00
committed by Philippe Mathieu-Daudé
parent ecc569639a
commit a3cc0069e1
1 changed files with 17 additions and 11 deletions
Download Patch File Download Diff File Expand all files Collapse all files

28
hw/display/qxl-render.c
View File

@@ -217,7 +217,8 @@ void qxl_render_update_area_done(PCIQXLDevice *qxl, QXLCookie *cookie)
}
static void qxl_unpack_chunks(void *dest, size_t size, PCIQXLDevice *qxl,
QXLDataChunk *chunk, uint32_t group_id)
QXLDataChunk *chunk, uint32_t group_id,
uint32_t chunk_data_size)
{
uint32_t max_chunks = 32;
size_t offset = 0;
@@ -225,22 +226,21 @@ static void qxl_unpack_chunks(void *dest, size_t size, PCIQXLDevice *qxl,
QXLPHYSICAL next_chunk_phys = 0;
for (;;) {
bytes = MIN(size - offset, chunk->data_size);
bytes = MIN(size - offset, chunk_data_size);
memcpy(dest + offset, chunk->data, bytes);
offset += bytes;
if (offset == size) {
return;
}
next_chunk_phys = chunk->next_chunk;
/* fist time, only get the next chunk's data size */
chunk = qxl_phys2virt(qxl, next_chunk_phys, group_id,
sizeof(QXLDataChunk));
if (!chunk) {
return;
}
/* second time, check data size and get data */
chunk_data_size = chunk->data_size;
chunk = qxl_phys2virt(qxl, next_chunk_phys, group_id,
sizeof(QXLDataChunk) + chunk->data_size);
sizeof(QXLDataChunk) + chunk_data_size);
if (!chunk) {
return;
}
@@ -252,7 +252,7 @@ static void qxl_unpack_chunks(void *dest, size_t size, PCIQXLDevice *qxl,
}
static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor,
uint32_t group_id)
uint32_t group_id, uint32_t chunk_data_size)
{
QEMUCursor *c;
uint8_t *and_mask, *xor_mask;
@@ -272,11 +272,11 @@ static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor,
case SPICE_CURSOR_TYPE_MONO:
/* Assume that the full cursor is available in a single chunk. */
size = 2 * cursor_get_mono_bpl(c) * c->height;
if (size != cursor->data_size || cursor->chunk.data_size < size) {
if (size != cursor->data_size || chunk_data_size < size) {
qxl_set_guest_bug(qxl, "%s: bad monochrome cursor %ux%u"
" data_size %u chunk_size %u",
__func__, c->width, c->height,
cursor->data_size, cursor->chunk.data_size);
cursor->data_size, chunk_data_size);
goto fail;
}
and_mask = cursor->chunk.data;
@@ -288,7 +288,8 @@ static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor,
break;
case SPICE_CURSOR_TYPE_ALPHA:
size = sizeof(uint32_t) * c->width * c->height;
qxl_unpack_chunks(c->data, size, qxl, &cursor->chunk, group_id);
qxl_unpack_chunks(c->data, size, qxl, &cursor->chunk, group_id,
chunk_data_size);
if (qxl->debug > 2) {
cursor_print_ascii_art(c, "qxl/alpha");
}
@@ -325,19 +326,23 @@ int qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext)
}
switch (cmd->type) {
case QXL_CURSOR_SET:
{
uint32_t chunk_data_size;
/* First read the QXLCursor to get QXLDataChunk::data_size ... */
cursor = qxl_phys2virt(qxl, cmd->u.set.shape, ext->group_id,
sizeof(QXLCursor));
if (!cursor) {
return 1;
}
chunk_data_size = cursor->chunk.data_size;
/* Then read including the chunked data following QXLCursor. */
cursor = qxl_phys2virt(qxl, cmd->u.set.shape, ext->group_id,
sizeof(QXLCursor) + cursor->chunk.data_size);
sizeof(QXLCursor) + chunk_data_size);
if (!cursor) {
return 1;
}
c = qxl_cursor(qxl, cursor, ext->group_id);
c = qxl_cursor(qxl, cursor, ext->group_id, chunk_data_size);
if (c == NULL) {
c = cursor_builtin_left_ptr();
}
@@ -351,6 +356,7 @@ int qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext)
qemu_mutex_unlock(&qxl->ssd.lock);
qemu_bh_schedule(qxl->ssd.cursor_bh);
break;
}
case QXL_CURSOR_MOVE:
qemu_mutex_lock(&qxl->ssd.lock);
qxl->ssd.mouse_x = cmd->u.position.x;