@@ -100,12 +100,16 @@ static const char *qxl_v2n(const char *n[], size_t l, int v)
}
#define qxl_name(_list, _value) qxl_v2n(_list, ARRAY_SIZE(_list), _value)
-static void qxl_log_image(PCIQXLDevice *qxl, QXLPHYSICAL addr, int group_id)
+static void qxl_log_image(PCIQXLDevice *qxl, QXLPHYSICAL addr, int group_id,
+ int *error)
{
QXLImage *image;
QXLImageDescriptor *desc;
- image = qxl_phys2virt(qxl, addr, group_id);
+ image = qxl_phys2virt(qxl, addr, group_id, error);
+ if (*error) {
+ return;
+ }
desc = &image->descriptor;
fprintf(stderr, " (id %" PRIx64 " type %d flags %d width %d height %d",
desc->id, desc->type, desc->flags, desc->width, desc->height);
@@ -130,17 +134,19 @@ static void qxl_log_rect(QXLRect *rect)
rect->left, rect->top);
}
-static void qxl_log_cmd_draw_copy(PCIQXLDevice *qxl, QXLCopy *copy, int group_id)
+static void qxl_log_cmd_draw_copy(PCIQXLDevice *qxl, QXLCopy *copy,
+ int group_id, int *error)
{
fprintf(stderr, " src %" PRIx64,
copy->src_bitmap);
- qxl_log_image(qxl, copy->src_bitmap, group_id);
+ qxl_log_image(qxl, copy->src_bitmap, group_id, error);
fprintf(stderr, " area");
qxl_log_rect(©->src_area);
fprintf(stderr, " rop %d", copy->rop_descriptor);
}
-static void qxl_log_cmd_draw(PCIQXLDevice *qxl, QXLDrawable *draw, int group_id)
+static void qxl_log_cmd_draw(PCIQXLDevice *qxl, QXLDrawable *draw, int group_id,
+ int *error)
{
fprintf(stderr, ": surface_id %d type %s effect %s",
draw->surface_id,
@@ -148,13 +154,13 @@ static void qxl_log_cmd_draw(PCIQXLDevice *qxl, QXLDrawable *draw, int group_id)
qxl_name(qxl_draw_effect, draw->effect));
switch (draw->type) {
case QXL_DRAW_COPY:
- qxl_log_cmd_draw_copy(qxl, &draw->u.copy, group_id);
+ qxl_log_cmd_draw_copy(qxl, &draw->u.copy, group_id, error);
break;
}
}
static void qxl_log_cmd_draw_compat(PCIQXLDevice *qxl, QXLCompatDrawable *draw,
- int group_id)
+ int group_id, int *error)
{
fprintf(stderr, ": type %s effect %s",
qxl_name(qxl_draw_type, draw->type),
@@ -166,7 +172,7 @@ static void qxl_log_cmd_draw_compat(PCIQXLDevice *qxl, QXLCompatDrawable *draw,
}
switch (draw->type) {
case QXL_DRAW_COPY:
- qxl_log_cmd_draw_copy(qxl, &draw->u.copy, group_id);
+ qxl_log_cmd_draw_copy(qxl, &draw->u.copy, group_id, error);
break;
}
}
@@ -192,6 +198,7 @@ static void qxl_log_cmd_surface(PCIQXLDevice *qxl, QXLSurfaceCmd *cmd)
void qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id)
{
QXLCursor *cursor;
+ int error;
fprintf(stderr, ": %s",
qxl_name(qxl_cursor_cmd, cmd->type));
@@ -202,7 +209,10 @@ void 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, &error);
+ if (error) {
+ return;
+ }
fprintf(stderr, " type %s size %dx%d hot-spot +%d+%d"
" unique 0x%" PRIx64 " data-size %d",
qxl_name(spice_cursor_type, cursor->header.type),
@@ -220,6 +230,7 @@ void qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext)
{
bool compat = ext->flags & QXL_COMMAND_FLAG_COMPAT;
void *data;
+ int error;
if (!qxl->cmdlog) {
return;
@@ -230,13 +241,16 @@ void qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext)
qxl_name(qxl_type, ext->cmd.type),
compat ? "(compat)" : "");
- data = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
+ data = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id, &error);
+ if (error) {
+ return;
+ }
switch (ext->cmd.type) {
case QXL_CMD_DRAW:
if (!compat) {
- qxl_log_cmd_draw(qxl, data, ext->group_id);
+ qxl_log_cmd_draw(qxl, data, ext->group_id, &error);
} else {
- qxl_log_cmd_draw_compat(qxl, data, ext->group_id);
+ qxl_log_cmd_draw_compat(qxl, data, ext->group_id, &error);
}
break;
case QXL_CMD_SURFACE:
@@ -229,10 +229,16 @@ fail:
/* called from spice server thread context only */
void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext)
{
- QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
+ int error;
+ QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id,
+ &error);
QXLCursor *cursor;
QEMUCursor *c;
+ if (error) {
+ return;
+ }
+
if (!qxl->ssd.ds->mouse_set || !qxl->ssd.ds->cursor_define) {
return;
}
@@ -244,7 +250,11 @@ void 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);
+ cursor = qxl_phys2virt(qxl, cmd->u.set.shape, ext->group_id,
+ &error);
+ if (error) {
+ return;
+ }
if (cursor->chunk.data_size != cursor->data_size) {
fprintf(stderr, "%s: multiple chunks\n", __FUNCTION__);
return;
@@ -382,15 +382,32 @@ static void qxl_ring_set_dirty(PCIQXLDevice *qxl)
/*
* keep track of some command state, for savevm/loadvm.
* called from spice server thread context only
+ *
+ * Returns 0 on success, 1 on failure.
+ *
+ * Failure is caused by:
+ * non initialized slots (qxl_phys2virt failure)
+ * illegal command
+ * surface id larger then NUM_SURFACES
*/
-static void qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
+static int qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
{
+ int error;
+
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,
+ &error);
+ if (error) {
+ return 1;
+ }
uint32_t id = le32_to_cpu(cmd->surface_id);
- PANIC_ON(id >= NUM_SURFACES);
+
+ if (id >= NUM_SURFACES) {
+ qxl_guest_bug(qxl, "QXL_CMD_SURFACE id %d >= %d", id, NUM_SURFACES);
+ return 1;
+ }
qemu_mutex_lock(&qxl->track_lock);
if (cmd->type == QXL_SURFACE_CMD_CREATE) {
qxl->guest_surfaces.cmds[id] = ext->cmd.data;
@@ -407,7 +424,12 @@ static void qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
}
case QXL_CMD_CURSOR:
{
- QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
+ int error;
+ QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id,
+ &error);
+ if (error) {
+ return 1;
+ }
if (cmd->type == QXL_CURSOR_SET) {
qemu_mutex_lock(&qxl->track_lock);
qxl->guest_cursor = ext->cmd.data;
@@ -416,6 +438,7 @@ static void qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
break;
}
}
+ return 0;
}
/* spice display interface callbacks */
@@ -1087,25 +1110,45 @@ static void qxl_reset_surfaces(PCIQXLDevice *d)
}
/* can be also called from spice server thread context */
-void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL pqxl, int group_id)
+/* NULL is returned on error, but it can also be a legitimate return
+ * value, so this function needs to be changed to pass error by another
+ * argument */
+void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL pqxl, int group_id,
+ int *error)
{
uint64_t phys = le64_to_cpu(pqxl);
uint32_t slot = (phys >> (64 - 8)) & 0xff;
uint64_t offset = phys & 0xffffffffffff;
+ assert(error);
+ *error = 1;
switch (group_id) {
case MEMSLOT_GROUP_HOST:
return (void *)(intptr_t)offset;
case MEMSLOT_GROUP_GUEST:
- PANIC_ON(slot >= NUM_MEMSLOTS);
- PANIC_ON(!qxl->guest_slots[slot].active);
- PANIC_ON(offset < qxl->guest_slots[slot].delta);
+ if (slot >= NUM_MEMSLOTS) {
+ qxl_guest_bug(qxl, "slot too large %d >= %d", slot, NUM_MEMSLOTS);
+ return NULL;
+ }
+ if (!qxl->guest_slots[slot].active) {
+ qxl_guest_bug(qxl, "inactive slot %d\n", slot);
+ return NULL;
+ }
+ if (offset < qxl->guest_slots[slot].delta) {
+ qxl_guest_bug(qxl, "slot %d offset %"PRIu64" < delta %"PRIu64"\n",
+ slot, offset, qxl->guest_slots[slot].delta);
+ return NULL;
+ }
offset -= qxl->guest_slots[slot].delta;
- PANIC_ON(offset > qxl->guest_slots[slot].size)
+ if (offset > qxl->guest_slots[slot].size) {
+ qxl_guest_bug(qxl, "slot %d offset %"PRIu64" > size %"PRIu64"\n",
+ slot, offset, qxl->guest_slots[slot].size);
+ return NULL;
+ }
+ *error = 0;
return qxl->guest_slots[slot].ptr + offset;
- default:
- PANIC_ON(1);
}
+ return NULL;
}
static void qxl_create_guest_primary_complete(PCIQXLDevice *qxl)
@@ -1532,6 +1575,7 @@ static void qxl_dirty_surfaces(PCIQXLDevice *qxl)
{
intptr_t vram_start;
int i;
+ int error;
if (qxl->mode != QXL_MODE_NATIVE && qxl->mode != QXL_MODE_COMPAT) {
return;
@@ -1554,11 +1598,13 @@ static void qxl_dirty_surfaces(PCIQXLDevice *qxl)
}
cmd = qxl_phys2virt(qxl, qxl->guest_surfaces.cmds[i],
- MEMSLOT_GROUP_GUEST);
+ MEMSLOT_GROUP_GUEST, &error);
+ assert(!error);
assert(cmd->type == QXL_SURFACE_CMD_CREATE);
surface_offset = (intptr_t)qxl_phys2virt(qxl,
cmd->u.surface_create.data,
- MEMSLOT_GROUP_GUEST);
+ MEMSLOT_GROUP_GUEST, &error);
+ assert(!error);
surface_offset -= vram_start;
surface_size = cmd->u.surface_create.height *
abs(cmd->u.surface_create.stride);
@@ -126,7 +126,8 @@ typedef struct PCIQXLDevice {
#define QXL_DEFAULT_REVISION QXL_REVISION_STABLE_V10
/* qxl.c */
-void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id);
+void *qxl_phys2virt(PCIQXLDevice *qxl, QXLPHYSICAL phys, int group_id,
+ int *error);
void qxl_guest_bug(PCIQXLDevice *qxl, const char *msg, ...);
void qxl_spice_update_area(PCIQXLDevice *qxl, uint32_t surface_id,
Issues a qxl_guest_bug from qxl_phys2virt. Everywhere else will fail. qxl_phys2virt requires an additional argument because all it's possible return values are legit (well, I could use the fact it returns a pointer so it should be word aligned but I don't want to go there, this is totally internal). Signed-off-by: Alon Levy <alevy@redhat.com> --- hw/qxl-logger.c | 38 +++++++++++++++++++---------- hw/qxl-render.c | 14 +++++++++-- hw/qxl.c | 72 +++++++++++++++++++++++++++++++++++++++++++++---------- hw/qxl.h | 3 ++- 4 files changed, 99 insertions(+), 28 deletions(-)