[Qemu-devel] [PATCH 1/2] hw/qxl: extract verify_surface_cmd from qxl_track_command

[Qemu-devel] [PATCH 1/2] hw/qxl: extract verify_surface_cmd from qxl_track_command

[Alon Levy]

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/qxl.c | 48 +++++++++++++++++++++++++++++-------------------
 1 file changed, 29 insertions(+), 19 deletions(-)

diff --git a/hw/qxl.c b/hw/qxl.c
index 1482389..af5f68e 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -430,6 +430,34 @@ static void qxl_ring_set_dirty(PCIQXLDevice *qxl)
     qxl_set_dirty(&qxl->vga.vram, addr, end);
 }
 
+static int verify_surface_cmd(PCIQXLDevice *qxl, QXLSurfaceCmd *cmd)
+{
+    uint32_t id = le32_to_cpu(cmd->surface_id);
+
+    if (id >= qxl->ssd.num_surfaces) {
+        qxl_set_guest_bug(qxl, "QXL_CMD_SURFACE id %d >= %d", id,
+                          qxl->ssd.num_surfaces);
+        return 1;
+    }
+    if (cmd->type == QXL_SURFACE_CMD_CREATE &&
+        (cmd->u.surface_create.stride & 0x03) != 0) {
+        qxl_set_guest_bug(qxl, "QXL_CMD_SURFACE stride = %d %% 4 != 0\n",
+                          cmd->u.surface_create.stride);
+        return 1;
+    }
+    if (cmd->type == QXL_SURFACE_CMD_CREATE) {
+        intptr_t surface_offset = (intptr_t)qxl_phys2virt(qxl,
+                                                 cmd->u.surface_create.data,
+                                                 MEMSLOT_GROUP_GUEST);
+        if (!surface_offset) {
+            qxl_set_guest_bug(qxl, "QXL_CMD_SURFACE invalid data: %ld\n",
+                              cmd->u.surface_create.data);
+            return 1;
+        }
+    }
+    return 0;
+}
+
 /*
  * keep track of some command state, for savevm/loadvm.
  * called from spice server thread context only
@@ -446,27 +474,9 @@ static int qxl_track_command(PCIQXLDevice *qxl, struct QXLCommandExt *ext)
         }
         uint32_t id = le32_to_cpu(cmd->surface_id);
 
-        if (id >= qxl->ssd.num_surfaces) {
-            qxl_set_guest_bug(qxl, "QXL_CMD_SURFACE id %d >= %d", id,
-                              qxl->ssd.num_surfaces);
+        if (!verify_surface_cmd(qxl, cmd)) {
             return 1;
         }
-        if (cmd->type == QXL_SURFACE_CMD_CREATE &&
-            (cmd->u.surface_create.stride & 0x03) != 0) {
-            qxl_set_guest_bug(qxl, "QXL_CMD_SURFACE stride = %d %% 4 != 0\n",
-                              cmd->u.surface_create.stride);
-            return 1;
-        }
-        if (cmd->type == QXL_SURFACE_CMD_CREATE) {
-            intptr_t surface_offset = (intptr_t)qxl_phys2virt(qxl,
-                                                     cmd->u.surface_create.data,
-                                                     MEMSLOT_GROUP_GUEST);
-            if (!surface_offset) {
-                qxl_set_guest_bug(qxl, "QXL_CMD_SURFACE invalid data: %ld\n",
-                                  cmd->u.surface_create.data);
-                return 1;
-            }
-        }
         qemu_mutex_lock(&qxl->track_lock);
         if (cmd->type == QXL_SURFACE_CMD_CREATE) {
             qxl->guest_surfaces.cmds[id] = ext->cmd.data;
-- 
1.8.0

[Qemu-devel] [PATCH 2/2] qxl/verify_surface_cmd: check format != 0 (buggy drivers can do that)

[Alon Levy]

Signed-off-by: Alon Levy <alevy@redhat.com>
---
 hw/qxl.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/hw/qxl.c b/hw/qxl.c
index af5f68e..91e4fec 100644
--- a/hw/qxl.c
+++ b/hw/qxl.c
@@ -455,6 +455,12 @@ static int verify_surface_cmd(PCIQXLDevice *qxl, QXLSurfaceCmd *cmd)
             return 1;
         }
     }
+    if (cmd->type == QXL_SURFACE_CMD_CREATE &&
+        cmd->u.surface_create.format == 0) {
+        qxl_set_guest_bug(qxl, "QXL_CMD_SURFACE invalid format: %u\n",
+                          cmd->u.surface_create.format);
+        return 1;
+    }
     return 0;
 }
 
-- 
1.8.0

Re: [Qemu-devel] [PATCH 1/2] hw/qxl: extract verify_surface_cmd from qxl_track_command

[Gerd Hoffmann]

On 11/20/12 10:19, Alon Levy wrote:
> Signed-off-by: Alon Levy <alevy@redhat.com>

Doesn't apply to master.

cheers,
  Gerd

Re: [Qemu-devel] [PATCH 1/2] hw/qxl: extract verify_surface_cmd from qxl_track_command

[Alon Levy]

> On 11/20/12 10:19, Alon Levy wrote:
> > Signed-off-by: Alon Levy <alevy@redhat.com>
> 
> Doesn't apply to master.

Good. It didn't apply because it was missing warn on sync io-usage and validate surface data. The later we agreed should be dropped in favor of spice reporting an error to qxl.

> 
> cheers,
>   Gerd
> 
> 
>