[Qemu-devel] [PATCH 04/12] qxl: check for NULL return from qxl_phys2virt

[Gerd Hoffmann]

From: Alon Levy <address@hidden>

Signed-off-by: Alon Levy <address@hidden>
Signed-off-by: Gerd Hoffmann <address@hidden>
---
 hw/qxl-logger.c |   51 ++++++++++++++++++++++++++++++++++++++-------------
 hw/qxl-render.c |   14 +++++++++++---
 hw/qxl.c        |   13 ++++++++++++-
 hw/qxl.h        |    6 +++---
 4 files changed, 64 insertions(+), 20 deletions(-)

diff --git a/hw/qxl-logger.c b/hw/qxl-logger.c
index 367aad1..fe2878c 100644
--- a/hw/qxl-logger.c
+++ b/hw/qxl-logger.c
@@ -100,12 +100,15 @@ 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 int qxl_log_image(PCIQXLDevice *qxl, QXLPHYSICAL addr, int group_id)
 {
     QXLImage *image;
     QXLImageDescriptor *desc;
 
     image = qxl_phys2virt(qxl, addr, group_id);
+    if (!image) {
+        return 1;
+    }
     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);
@@ -120,6 +123,7 @@ static void qxl_log_image(PCIQXLDevice *qxl, QXLPHYSICAL 
addr, int group_id)
         break;
     }
     fprintf(stderr, ")");
+    return 0;
 }
 
 static void qxl_log_rect(QXLRect *rect)
@@ -130,17 +134,24 @@ 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 int qxl_log_cmd_draw_copy(PCIQXLDevice *qxl, QXLCopy *copy,
+                                 int group_id)
 {
+    int ret;
+
     fprintf(stderr, " src %" PRIx64,
             copy->src_bitmap);
-    qxl_log_image(qxl, copy->src_bitmap, group_id);
+    ret = qxl_log_image(qxl, copy->src_bitmap, group_id);
+    if (ret != 0) {
+        return ret;
+    }
     fprintf(stderr, " area");
     qxl_log_rect(&copy->src_area);
     fprintf(stderr, " rop %d", copy->rop_descriptor);
+    return 0;
 }
 
-static void qxl_log_cmd_draw(PCIQXLDevice *qxl, QXLDrawable *draw, int 
group_id)
+static int qxl_log_cmd_draw(PCIQXLDevice *qxl, QXLDrawable *draw, int group_id)
 {
     fprintf(stderr, ": surface_id %d type %s effect %s",
             draw->surface_id,
@@ -148,13 +159,14 @@ 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);
+        return qxl_log_cmd_draw_copy(qxl, &draw->u.copy, group_id);
         break;
     }
+    return 0;
 }
 
-static void qxl_log_cmd_draw_compat(PCIQXLDevice *qxl, QXLCompatDrawable *draw,
-                                    int group_id)
+static int qxl_log_cmd_draw_compat(PCIQXLDevice *qxl, QXLCompatDrawable *draw,
+                                   int group_id)
 {
     fprintf(stderr, ": type %s effect %s",
             qxl_name(qxl_draw_type, draw->type),
@@ -166,9 +178,10 @@ 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);
+        return qxl_log_cmd_draw_copy(qxl, &draw->u.copy, group_id);
         break;
     }
+    return 0;
 }
 
 static void qxl_log_cmd_surface(PCIQXLDevice *qxl, QXLSurfaceCmd *cmd)
@@ -189,7 +202,7 @@ static void qxl_log_cmd_surface(PCIQXLDevice *qxl, 
QXLSurfaceCmd *cmd)
     }
 }
 
-void qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id)
+int qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id)
 {
     QXLCursor *cursor;
 
@@ -203,6 +216,9 @@ void qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd 
*cmd, int group_id)
                 cmd->u.set.visible ? "yes" : "no",
                 cmd->u.set.shape);
         cursor = qxl_phys2virt(qxl, cmd->u.set.shape, group_id);
+        if (!cursor) {
+            return 1;
+        }
         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),
@@ -214,15 +230,17 @@ void qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd 
*cmd, int group_id)
         fprintf(stderr, " +%d+%d", cmd->u.position.x, cmd->u.position.y);
         break;
     }
+    return 0;
 }
 
-void qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext)
+int qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext)
 {
     bool compat = ext->flags & QXL_COMMAND_FLAG_COMPAT;
     void *data;
+    int ret;
 
     if (!qxl->cmdlog) {
-        return;
+        return 0;
     }
     fprintf(stderr, "%" PRId64 " qxl-%d/%s:", qemu_get_clock_ns(vm_clock),
             qxl->id, ring);
@@ -231,12 +249,18 @@ void qxl_log_command(PCIQXLDevice *qxl, const char *ring, 
QXLCommandExt *ext)
             compat ? "(compat)" : "");
 
     data = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
+    if (!data) {
+        return 1;
+    }
     switch (ext->cmd.type) {
     case QXL_CMD_DRAW:
         if (!compat) {
-            qxl_log_cmd_draw(qxl, data, ext->group_id);
+            ret = qxl_log_cmd_draw(qxl, data, ext->group_id);
         } else {
-            qxl_log_cmd_draw_compat(qxl, data, ext->group_id);
+            ret = qxl_log_cmd_draw_compat(qxl, data, ext->group_id);
+        }
+        if (ret) {
+            return ret;
         }
         break;
     case QXL_CMD_SURFACE:
@@ -247,4 +271,5 @@ void qxl_log_command(PCIQXLDevice *qxl, const char *ring, 
QXLCommandExt *ext)
         break;
     }
     fprintf(stderr, "\n");
+    return 0;
 }
diff --git a/hw/qxl-render.c b/hw/qxl-render.c
index f7f1bfd..e2e3fe2 100644
--- a/hw/qxl-render.c
+++ b/hw/qxl-render.c
@@ -228,14 +228,18 @@ fail:
 
 
 /* called from spice server thread context only */
-void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext)
+int qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext)
 {
     QXLCursorCmd *cmd = qxl_phys2virt(qxl, ext->cmd.data, ext->group_id);
     QXLCursor *cursor;
     QEMUCursor *c;
 
+    if (!cmd) {
+        return 1;
+    }
+
     if (!qxl->ssd.ds->mouse_set || !qxl->ssd.ds->cursor_define) {
-        return;
+        return 0;
     }
 
     if (qxl->debug > 1 && cmd->type != QXL_CURSOR_MOVE) {
@@ -246,9 +250,12 @@ 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);
+        if (!cursor) {
+            return 1;
+        }
         if (cursor->chunk.data_size != cursor->data_size) {
             fprintf(stderr, "%s: multiple chunks\n", __FUNCTION__);
-            return;
+            return 1;
         }
         c = qxl_cursor(qxl, cursor);
         if (c == NULL) {
@@ -270,4 +277,5 @@ void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt 
*ext)
         qemu_mutex_unlock(&qxl->ssd.lock);
         break;
     }
+    return 0;
 }
diff --git a/hw/qxl.c b/hw/qxl.c
index 9e8cdf3..b6a738e 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -383,12 +383,16 @@ static void qxl_ring_set_dirty(PCIQXLDevice *qxl)
  * keep track of some command state, for savevm/loadvm.
  * called from spice server thread context only
  */
-static void qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
+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);
+
+        if (!cmd) {
+            return 1;
+        }
         uint32_t id = le32_to_cpu(cmd->surface_id);
         PANIC_ON(id >= NUM_SURFACES);
         qemu_mutex_lock(&qxl->track_lock);
@@ -408,6 +412,10 @@ 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);
+
+        if (!cmd) {
+            return 1;
+        }
         if (cmd->type == QXL_CURSOR_SET) {
             qemu_mutex_lock(&qxl->track_lock);
             qxl->guest_cursor = ext->cmd.data;
@@ -416,6 +424,7 @@ static void qxl_track_command(PCIQXLDevice *qxl, struct 
QXLCommandExt *ext)
         break;
     }
     }
+    return 0;
 }
 
 /* spice display interface callbacks */
@@ -1568,10 +1577,12 @@ static void qxl_dirty_surfaces(PCIQXLDevice *qxl)
 
         cmd = qxl_phys2virt(qxl, qxl->guest_surfaces.cmds[i],
                             MEMSLOT_GROUP_GUEST);
+        assert(cmd);
         assert(cmd->type == QXL_SURFACE_CMD_CREATE);
         surface_offset = (intptr_t)qxl_phys2virt(qxl,
                                                  cmd->u.surface_create.data,
                                                  MEMSLOT_GROUP_GUEST);
+        assert(surface_offset);
         surface_offset -= vram_start;
         surface_size = cmd->u.surface_create.height *
                        abs(cmd->u.surface_create.stride);
diff --git a/hw/qxl.h b/hw/qxl.h
index cbb1e2d..3102950 100644
--- a/hw/qxl.h
+++ b/hw/qxl.h
@@ -142,12 +142,12 @@ void qxl_spice_reset_image_cache(PCIQXLDevice *qxl);
 void qxl_spice_reset_cursor(PCIQXLDevice *qxl);
 
 /* qxl-logger.c */
-void qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id);
-void qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext);
+int qxl_log_cmd_cursor(PCIQXLDevice *qxl, QXLCursorCmd *cmd, int group_id);
+int qxl_log_command(PCIQXLDevice *qxl, const char *ring, QXLCommandExt *ext);
 
 /* qxl-render.c */
 void qxl_render_resize(PCIQXLDevice *qxl);
 void qxl_render_update(PCIQXLDevice *qxl);
-void qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext);
+int qxl_render_cursor(PCIQXLDevice *qxl, QXLCommandExt *ext);
 void qxl_render_update_area_done(PCIQXLDevice *qxl, QXLCookie *cookie);
 void qxl_render_update_area_bh(void *opaque);
-- 
1.7.1