cf91b1d
From: Gerd Hoffmann <kraxel@redhat.com>
cf91b1d
Date: Mon, 30 May 2016 09:09:20 +0200
cf91b1d
Subject: [PATCH] vmsvga: shadow fifo registers
cf91b1d
cf91b1d
The fifo is normal ram.  So kvm vcpu threads and qemu iothread can
cf91b1d
access the fifo in parallel without syncronization.  Which in turn
cf91b1d
implies we can't use the fifo pointers in-place because the guest
cf91b1d
can try changing them underneath us.  So add shadows for them, to
cf91b1d
make sure the guest can't modify them after we've applied sanity
cf91b1d
checks.
cf91b1d
cf91b1d
Fixes: CVE-2016-4454
cf91b1d
Cc: qemu-stable@nongnu.org
cf91b1d
Cc: P J P <ppandit@redhat.com>
cf91b1d
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
cf91b1d
Message-id: 1464592161-18348-4-git-send-email-kraxel@redhat.com
cf91b1d
(cherry picked from commit 7e486f7577764a07aa35588e119903c80a5c30a2)
cf91b1d
---
cf91b1d
 hw/display/vmware_vga.c | 57 ++++++++++++++++++++++++-------------------------
cf91b1d
 1 file changed, 28 insertions(+), 29 deletions(-)
cf91b1d
cf91b1d
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
cf91b1d
index a26e62e..de2567b 100644
cf91b1d
--- a/hw/display/vmware_vga.c
cf91b1d
+++ b/hw/display/vmware_vga.c
cf91b1d
@@ -66,17 +66,11 @@ struct vmsvga_state_s {
cf91b1d
     uint8_t *fifo_ptr;
cf91b1d
     unsigned int fifo_size;
cf91b1d
 
cf91b1d
-    union {
cf91b1d
-        uint32_t *fifo;
cf91b1d
-        struct QEMU_PACKED {
cf91b1d
-            uint32_t min;
cf91b1d
-            uint32_t max;
cf91b1d
-            uint32_t next_cmd;
cf91b1d
-            uint32_t stop;
cf91b1d
-            /* Add registers here when adding capabilities.  */
cf91b1d
-            uint32_t fifo[0];
cf91b1d
-        } *cmd;
cf91b1d
-    };
cf91b1d
+    uint32_t *fifo;
cf91b1d
+    uint32_t fifo_min;
cf91b1d
+    uint32_t fifo_max;
cf91b1d
+    uint32_t fifo_next;
cf91b1d
+    uint32_t fifo_stop;
cf91b1d
 
cf91b1d
 #define REDRAW_FIFO_LEN  512
cf91b1d
     struct vmsvga_rect_s {
cf91b1d
@@ -198,7 +192,7 @@ enum {
cf91b1d
      */
cf91b1d
     SVGA_FIFO_MIN = 0,
cf91b1d
     SVGA_FIFO_MAX,      /* The distance from MIN to MAX must be at least 10K */
cf91b1d
-    SVGA_FIFO_NEXT_CMD,
cf91b1d
+    SVGA_FIFO_NEXT,
cf91b1d
     SVGA_FIFO_STOP,
cf91b1d
 
cf91b1d
     /*
cf91b1d
@@ -546,8 +540,6 @@ static inline void vmsvga_cursor_define(struct vmsvga_state_s *s,
cf91b1d
 }
cf91b1d
 #endif
cf91b1d
 
cf91b1d
-#define CMD(f)  le32_to_cpu(s->cmd->f)
cf91b1d
-
cf91b1d
 static inline int vmsvga_fifo_length(struct vmsvga_state_s *s)
cf91b1d
 {
cf91b1d
     int num;
cf91b1d
@@ -556,38 +548,44 @@ static inline int vmsvga_fifo_length(struct vmsvga_state_s *s)
cf91b1d
         return 0;
cf91b1d
     }
cf91b1d
 
cf91b1d
+    s->fifo_min  = le32_to_cpu(s->fifo[SVGA_FIFO_MIN]);
cf91b1d
+    s->fifo_max  = le32_to_cpu(s->fifo[SVGA_FIFO_MAX]);
cf91b1d
+    s->fifo_next = le32_to_cpu(s->fifo[SVGA_FIFO_NEXT]);
cf91b1d
+    s->fifo_stop = le32_to_cpu(s->fifo[SVGA_FIFO_STOP]);
cf91b1d
+
cf91b1d
     /* Check range and alignment.  */
cf91b1d
-    if ((CMD(min) | CMD(max) | CMD(next_cmd) | CMD(stop)) & 3) {
cf91b1d
+    if ((s->fifo_min | s->fifo_max | s->fifo_next | s->fifo_stop) & 3) {
cf91b1d
         return 0;
cf91b1d
     }
cf91b1d
-    if (CMD(min) < (uint8_t *) s->cmd->fifo - (uint8_t *) s->fifo) {
cf91b1d
+    if (s->fifo_min < sizeof(uint32_t) * 4) {
cf91b1d
         return 0;
cf91b1d
     }
cf91b1d
-    if (CMD(max) > SVGA_FIFO_SIZE ||
cf91b1d
-        CMD(min) >= SVGA_FIFO_SIZE ||
cf91b1d
-        CMD(stop) >= SVGA_FIFO_SIZE ||
cf91b1d
-        CMD(next_cmd) >= SVGA_FIFO_SIZE) {
cf91b1d
+    if (s->fifo_max > SVGA_FIFO_SIZE ||
cf91b1d
+        s->fifo_min >= SVGA_FIFO_SIZE ||
cf91b1d
+        s->fifo_stop >= SVGA_FIFO_SIZE ||
cf91b1d
+        s->fifo_next >= SVGA_FIFO_SIZE) {
cf91b1d
         return 0;
cf91b1d
     }
cf91b1d
-    if (CMD(max) < CMD(min) + 10 * 1024) {
cf91b1d
+    if (s->fifo_max < s->fifo_min + 10 * 1024) {
cf91b1d
         return 0;
cf91b1d
     }
cf91b1d
 
cf91b1d
-    num = CMD(next_cmd) - CMD(stop);
cf91b1d
+    num = s->fifo_next - s->fifo_stop;
cf91b1d
     if (num < 0) {
cf91b1d
-        num += CMD(max) - CMD(min);
cf91b1d
+        num += s->fifo_max - s->fifo_min;
cf91b1d
     }
cf91b1d
     return num >> 2;
cf91b1d
 }
cf91b1d
 
cf91b1d
 static inline uint32_t vmsvga_fifo_read_raw(struct vmsvga_state_s *s)
cf91b1d
 {
cf91b1d
-    uint32_t cmd = s->fifo[CMD(stop) >> 2];
cf91b1d
+    uint32_t cmd = s->fifo[s->fifo_stop >> 2];
cf91b1d
 
cf91b1d
-    s->cmd->stop = cpu_to_le32(CMD(stop) + 4);
cf91b1d
-    if (CMD(stop) >= CMD(max)) {
cf91b1d
-        s->cmd->stop = s->cmd->min;
cf91b1d
+    s->fifo_stop += 4;
cf91b1d
+    if (s->fifo_stop >= s->fifo_max) {
cf91b1d
+        s->fifo_stop = s->fifo_min;
cf91b1d
     }
cf91b1d
+    s->fifo[SVGA_FIFO_STOP] = cpu_to_le32(s->fifo_stop);
cf91b1d
     return cmd;
cf91b1d
 }
cf91b1d
 
cf91b1d
@@ -607,7 +605,7 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
cf91b1d
     len = vmsvga_fifo_length(s);
cf91b1d
     while (len > 0) {
cf91b1d
         /* May need to go back to the start of the command if incomplete */
cf91b1d
-        cmd_start = s->cmd->stop;
cf91b1d
+        cmd_start = s->fifo_stop;
cf91b1d
 
cf91b1d
         switch (cmd = vmsvga_fifo_read(s)) {
cf91b1d
         case SVGA_CMD_UPDATE:
cf91b1d
@@ -766,7 +764,8 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
cf91b1d
             break;
cf91b1d
 
cf91b1d
         rewind:
cf91b1d
-            s->cmd->stop = cmd_start;
cf91b1d
+            s->fifo_stop = cmd_start;
cf91b1d
+            s->fifo[SVGA_FIFO_STOP] = cpu_to_le32(s->fifo_stop);
cf91b1d
             break;
cf91b1d
         }
cf91b1d
     }