Skip to content

Commit

Permalink
hw/display/qxl: Pass requested buffer size to qxl_phys2virt()
Browse files Browse the repository at this point in the history 
Currently qxl_phys2virt() doesn't check for buffer overrun.
In order to do so in the next commit, pass the buffer size
as argument.

For QXLCursor in qxl_render_cursor() -> qxl_cursor() we
verify the size of the chunked data ahead, checking we can
access 'sizeof(QXLCursor) + chunk->data_size' bytes.
Since in the SPICE_CURSOR_TYPE_MONO case the cursor is
assumed to fit in one chunk, no change are required.
In SPICE_CURSOR_TYPE_ALPHA the ahead read is handled in
qxl_unpack_chunks().

Signed-off-by: Philippe Mathieu-Daudé <philmd@linaro.org>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Stefan Hajnoczi <stefanha@redhat.com>
Message-Id: <20221128202741.4945-4-philmd@linaro.org>
  • Loading branch information
@philmd @stefanhaRH
philmd authored and stefanhaRH committed Nov 29, 2022
1 parent b1901de commit 8efec0e
Show file tree
Hide file tree
Showing 4 changed files with 36 additions and 13 deletions.
11 changes: 8 additions & 3 deletions hw/display/qxl-logger.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,7 @@ static int qxl_log_image(PCIQXLDevice *qxl, QXLPHYSICAL addr, int group_id)
QXLImage *image;
QXLImageDescriptor *desc;

image = qxl_phys2virt(qxl, addr, group_id);
image = qxl_phys2virt(qxl, addr, group_id, sizeof(QXLImage));
if (!image) {
return 1;
}
Expand Down Expand Up @@ -214,7 +214,8 @@ int qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id)
cmd->u.set.position.y,
cmd->u.set.visible ? "yes" : "no",
cmd->u.set.shape);
cursor = qxl_phys2virt(qxl, cmd->u.set.shape, group_id);
cursor = qxl_phys2virt(qxl, cmd->u.set.shape, group_id,
sizeof(QXLCursor));
if (!cursor) {
return 1;
}
Expand All @@ -236,6 +237,7 @@ int qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext)
{
bool compat = ext->flags & QXL_COMMAND_FLAG_COMPAT;
void *data;
size_t datasz;
int ret;

if (!qxl->cmdlog) {
Expand All @@ -249,15 +251,18 @@ int qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext)

switch (ext->cmd.type) {
case QXL_CMD_DRAW:
datasz = compat ? sizeof(QXLCompatDrawable) : sizeof(QXLDrawable);
break;
case QXL_CMD_SURFACE:
datasz = sizeof(QXLSurfaceCmd);
break;
case QXL_CMD_CURSOR:
datasz = sizeof(QXLCursorCmd);
break;
default:
goto out;
}
data = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
data = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id, datasz);
if (!data) {
return 1;
}
Expand Down
20 changes: 16 additions & 4 deletions hw/display/qxl-render.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,9 @@ static void qxl_render_update_area_unlocked(PCIQXLDevice *qxl)
qxl->guest_primary.resized = 0;
qxl->guest_primary.data = qxl_phys2virt(qxl,
qxl->guest_primary.surface.mem,
MEMSLOT_GROUP_GUEST);
MEMSLOT_GROUP_GUEST,
qxl->guest_primary.abs_stride
* height);
if (!qxl->guest_primary.data) {
goto end;
}
Expand Down Expand Up @@ -228,7 +230,8 @@ static void qxl_unpack_chunks(void *dest, size_t size, PCIQXLDevice *qxl,
if (offset == size) {
return;
}
chunk = qxl_phys2virt(qxl, chunk->next_chunk, group_id);
chunk = qxl_phys2virt(qxl, chunk->next_chunk, group_id,
sizeof(QXLDataChunk) + chunk->data_size);
if (!chunk) {
return;
}
Expand Down Expand Up @@ -295,7 +298,8 @@ static QEMUCursor *qxl_cursor(PCIQXLDevice *qxl, QXLCursor *cursor,
/* called from spice server thread context only */
int qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext)
{
QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id,
sizeof(QXLCursorCmd));
QXLCursor *cursor;
QEMUCursor *c;

Expand All @@ -314,7 +318,15 @@ int qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext)
}
switch (cmd->type) {
case QXL_CURSOR_SET:
cursor = qxl_phys2virt(qxl, cmd->u.set.shape, ext->group_id);
/* 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;
}
/* 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);
if (!cursor) {
return 1;
}
Expand Down
14 changes: 9 additions & 5 deletions hw/display/qxl.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -274,7 +274,8 @@ static void qxl_spice_monitors_config_async(PCIQXLDevice *qxl, int replay)
QXL_IO_MONITORS_CONFIG_ASYNC));
}

cfg = qxl_phys2virt(qxl, qxl->guest_monitors_config, MEMSLOT_GROUP_GUEST);
cfg = qxl_phys2virt(qxl, qxl->guest_monitors_config, MEMSLOT_GROUP_GUEST,
sizeof(QXLMonitorsConfig));
if (cfg != NULL && cfg->count == 1) {
qxl->guest_primary.resized = 1;
qxl->guest_head0_width = cfg->heads[0].width;
Expand Down Expand Up @@ -459,7 +460,8 @@ static int qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
switch (le32_to_cpu(ext->cmd.type)) {
case QXL_CMD_SURFACE:
{
QXLSurfaceCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
QXLSurfaceCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id,
sizeof(QXLSurfaceCmd));

if (!cmd) {
return 1;
Expand Down Expand Up @@ -494,7 +496,8 @@ static int qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
}
case QXL_CMD_CURSOR:
{
QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id,
sizeof(QXLCursorCmd));

if (!cmd) {
return 1;
Expand Down Expand Up @@ -1456,7 +1459,8 @@ static bool qxl_get_check_slot_offset(PCIQXLDevice *qxl, QXLPHYSICAL pqxl,
}

/* can be also called from spice server thread context */
void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL pqxl, int group_id)
void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL pqxl, int group_id,
size_t size)
{
uint64_t offset;
uint32_t slot;
Expand Down Expand Up @@ -1964,7 +1968,7 @@ static void qxl_dirty_surfaces(PCIQXLDevice *qxl)
}

cmd = qxl_phys2virt(qxl, qxl->guest_surfaces.cmds[i],
MEMSLOT_GROUP_GUEST);
MEMSLOT_GROUP_GUEST, sizeof(QXLSurfaceCmd));
assert(cmd);
assert(cmd->type == QXL_SURFACE_CMD_CREATE);
qxl_dirty_one_surface(qxl, cmd->u.surface_create.data,
Expand Down
4 changes: 3 additions & 1 deletion hw/display/qxl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,6 +153,7 @@ OBJECT_DECLARE_SIMPLE_TYPE(PCIQXLDevice, PCI_QXL)
* @qxl: QXL device
* @phys: physical offset of buffer within the VRAM
* @group_id: memory slot group
* @size: size of the buffer
*
* Returns a host pointer to a buffer placed at offset @phys within the
* active slot @group_id of the PCI VGA RAM memory region associated with
Expand All @@ -166,7 +167,8 @@ OBJECT_DECLARE_SIMPLE_TYPE(PCIQXLDevice, PCI_QXL)
* the incoming ram_addr_t.
*
*/
void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id);
void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id,
size_t size);
void qxl_set_guest_bug(PCIQXLDevice *qxl, const char *msg, ...)
G_GNUC_PRINTF(2, 3);

Expand Down

0 comments on commit 8efec0e

Please sign in to comment.