619821
From 9bd81fb917c9ac22055e0dc7b3a89a22d5cfbfc1 Mon Sep 17 00:00:00 2001
976a86
From: Gerd Hoffmann <kraxel@redhat.com>
619821
Date: Mon, 27 Mar 2017 10:01:17 +0200
976a86
Subject: [PATCH 2/7] cirrus/vnc: zap bitblit support from console code.
976a86
976a86
RH-Author: Gerd Hoffmann <kraxel@redhat.com>
619821
Message-id: <1490608882-10242-3-git-send-email-kraxel@redhat.com>
619821
Patchwork-id: 74554
619821
O-Subject: [RHEL-7.4 qemu-kvm PATCH v2 2/7] cirrus/vnc: zap bitblit support from console code.
619821
Bugzilla: 1430060
976a86
RH-Acked-by: Dr. David Alan Gilbert <dgilbert@redhat.com>
976a86
RH-Acked-by: Laurent Vivier <lvivier@redhat.com>
619821
RH-Acked-by: Paolo Bonzini <pbonzini@redhat.com>
976a86
976a86
There is a special code path (dpy_gfx_copy) to allow graphic emulation
976a86
notify user interface code about bitblit operations carryed out by
976a86
guests.  It is supported by cirrus and vnc server.  The intended purpose
976a86
is to optimize display scrolls and just send over the scroll op instead
976a86
of a full display update.
976a86
976a86
This is rarely used these days though because modern guests simply don't
976a86
use the cirrus blitter any more.  Any linux guest using the cirrus drm
976a86
driver doesn't.  Any windows guest newer than winxp doesn't ship with a
976a86
cirrus driver any more and thus uses the cirrus as simple framebuffer.
976a86
976a86
So this code tends to bitrot and bugs can go unnoticed for a long time.
976a86
See for example commit "3e10c3e vnc: fix qemu crash because of SIGSEGV"
976a86
which fixes a bug lingering in the code for almost a year, added by
976a86
commit "c7628bf vnc: only alloc server surface with clients connected".
976a86
976a86
Also the vnc server will throttle the frame rate in case it figures the
976a86
network can't keep up (send buffers are full).  This doesn't work with
976a86
dpy_gfx_copy, for any copy operation sent to the vnc client we have to
976a86
send all outstanding updates beforehand, otherwise the vnc client might
976a86
run the client side blit on outdated data and thereby corrupt the
976a86
display.  So this dpy_gfx_copy "optimization" might even make things
976a86
worse on slow network links.
976a86
976a86
Lets kill it once for all.
976a86
976a86
Oh, and one more reason: Turns out (after writing the patch) we have a
976a86
security bug in that code path ...
976a86
976a86
Fixes: CVE-2016-9603
976a86
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
976a86
Message-id: 1489494419-14340-1-git-send-email-kraxel@redhat.com
976a86
(cherry picked from commit 50628d3479e4f9aa97e323506856e394fe7ad7a6)
976a86
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
976a86
976a86
Conflicts:
976a86
	include/ui/console.h
976a86
	ui/vnc.c
976a86
---
619821
 hw/display/cirrus_vga.c | 12 ++----
976a86
 include/ui/console.h    |  7 ----
619821
 ui/console.c            | 28 --------------
619821
 ui/vnc.c                | 99 -------------------------------------------------
619821
 4 files changed, 3 insertions(+), 143 deletions(-)
976a86
976a86
diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
976a86
index 1b972db..83cef70 100644
976a86
--- a/hw/display/cirrus_vga.c
976a86
+++ b/hw/display/cirrus_vga.c
976a86
@@ -790,21 +790,15 @@ static int cirrus_do_copy(CirrusVGAState *s, int dst, int src, int w, int h)
976a86
         }
976a86
     }
976a86
 
976a86
-    /* we have to flush all pending changes so that the copy
976a86
-       is generated at the appropriate moment in time */
976a86
-    if (notify)
976a86
-        graphic_hw_update(s->vga.con);
976a86
-
976a86
     (*s->cirrus_rop) (s, s->vga.vram_ptr + s->cirrus_blt_dstaddr,
976a86
                       s->vga.vram_ptr + s->cirrus_blt_srcaddr,
976a86
 		      s->cirrus_blt_dstpitch, s->cirrus_blt_srcpitch,
976a86
 		      s->cirrus_blt_width, s->cirrus_blt_height);
976a86
 
976a86
     if (notify) {
976a86
-        qemu_console_copy(s->vga.con,
976a86
-			  sx, sy, dx, dy,
976a86
-			  s->cirrus_blt_width / depth,
976a86
-			  s->cirrus_blt_height);
976a86
+        dpy_gfx_update(s->vga.con, dx, dy,
976a86
+                       s->cirrus_blt_width / depth,
976a86
+                       s->cirrus_blt_height);
976a86
     }
976a86
 
976a86
     /* we don't have to notify the display that this portion has
976a86
diff --git a/include/ui/console.h b/include/ui/console.h
976a86
index 4307b5f..7f5fa66 100644
976a86
--- a/include/ui/console.h
976a86
+++ b/include/ui/console.h
976a86
@@ -159,9 +159,6 @@ typedef struct DisplayChangeListenerOps {
976a86
                            int x, int y, int w, int h);
976a86
     void (*dpy_gfx_switch)(DisplayChangeListener *dcl,
976a86
                            struct DisplaySurface *new_surface);
976a86
-    void (*dpy_gfx_copy)(DisplayChangeListener *dcl,
976a86
-                         int src_x, int src_y,
976a86
-                         int dst_x, int dst_y, int w, int h);
976a86
 
976a86
     void (*dpy_text_cursor)(DisplayChangeListener *dcl,
976a86
                             int x, int y);
976a86
@@ -216,8 +213,6 @@ void unregister_displaychangelistener(DisplayChangeListener *dcl);
976a86
 void dpy_gfx_update(QemuConsole *con, int x, int y, int w, int h);
976a86
 void dpy_gfx_replace_surface(QemuConsole *con,
976a86
                              DisplaySurface *surface);
976a86
-void dpy_gfx_copy(QemuConsole *con, int src_x, int src_y,
976a86
-                  int dst_x, int dst_y, int w, int h);
976a86
 void dpy_text_cursor(QemuConsole *con, int x, int y);
976a86
 void dpy_text_update(QemuConsole *con, int x, int y, int w, int h);
976a86
 void dpy_text_resize(QemuConsole *con, int w, int h);
976a86
@@ -295,8 +290,6 @@ void text_consoles_set_display(DisplayState *ds);
976a86
 void console_select(unsigned int index);
976a86
 void console_color_init(DisplayState *ds);
976a86
 void qemu_console_resize(QemuConsole *con, int width, int height);
976a86
-void qemu_console_copy(QemuConsole *con, int src_x, int src_y,
976a86
-                       int dst_x, int dst_y, int w, int h);
976a86
 DisplaySurface *qemu_console_surface(QemuConsole *con);
976a86
 DisplayState *qemu_console_displaystate(QemuConsole *console);
976a86
 
976a86
diff --git a/ui/console.c b/ui/console.c
976a86
index d422083..fb08ec0 100644
976a86
--- a/ui/console.c
976a86
+++ b/ui/console.c
976a86
@@ -1461,27 +1461,6 @@ void dpy_refresh(DisplayState *s)
976a86
     }
976a86
 }
976a86
 
976a86
-void dpy_gfx_copy(QemuConsole *con, int src_x, int src_y,
976a86
-                  int dst_x, int dst_y, int w, int h)
976a86
-{
976a86
-    DisplayState *s = con->ds;
976a86
-    DisplayChangeListener *dcl;
976a86
-
976a86
-    if (!qemu_console_is_visible(con)) {
976a86
-        return;
976a86
-    }
976a86
-    QLIST_FOREACH(dcl, &s->listeners, next) {
976a86
-        if (con != (dcl->con ? dcl->con : active_console)) {
976a86
-            continue;
976a86
-        }
976a86
-        if (dcl->ops->dpy_gfx_copy) {
976a86
-            dcl->ops->dpy_gfx_copy(dcl, src_x, src_y, dst_x, dst_y, w, h);
976a86
-        } else { /* TODO */
976a86
-            dcl->ops->dpy_gfx_update(dcl, dst_x, dst_y, w, h);
976a86
-        }
976a86
-    }
976a86
-}
976a86
-
976a86
 void dpy_text_cursor(QemuConsole *con, int x, int y)
976a86
 {
976a86
     DisplayState *s = con->ds;
976a86
@@ -1843,13 +1822,6 @@ void qemu_console_resize(QemuConsole *s, int width, int height)
976a86
     dpy_gfx_replace_surface(s, surface);
976a86
 }
976a86
 
976a86
-void qemu_console_copy(QemuConsole *con, int src_x, int src_y,
976a86
-                       int dst_x, int dst_y, int w, int h)
976a86
-{
976a86
-    assert(con->console_type == GRAPHIC_CONSOLE);
976a86
-    dpy_gfx_copy(con, src_x, src_y, dst_x, dst_y, w, h);
976a86
-}
976a86
-
976a86
 DisplaySurface *qemu_console_surface(QemuConsole *console)
976a86
 {
976a86
     return console->surface;
976a86
diff --git a/ui/vnc.c b/ui/vnc.c
619821
index b68918e..1834db0 100644
976a86
--- a/ui/vnc.c
976a86
+++ b/ui/vnc.c
976a86
@@ -417,7 +417,6 @@ out_error:
976a86
 */
976a86
 
976a86
 static int vnc_update_client(VncState *vs, int has_dirty);
976a86
-static int vnc_update_client_sync(VncState *vs, int has_dirty);
976a86
 static void vnc_disconnect_start(VncState *vs);
976a86
 
976a86
 static void vnc_colordepth(VncState *vs);
619821
@@ -728,96 +727,6 @@ int vnc_send_framebuffer_update(VncState *vs, int x, int y, int w, int h)
976a86
     return n;
976a86
 }
976a86
 
976a86
-static void vnc_copy(VncState *vs, int src_x, int src_y, int dst_x, int dst_y, int w, int h)
976a86
-{
976a86
-    /* send bitblit op to the vnc client */
976a86
-    vnc_lock_output(vs);
976a86
-    vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
976a86
-    vnc_write_u8(vs, 0);
976a86
-    vnc_write_u16(vs, 1); /* number of rects */
976a86
-    vnc_framebuffer_update(vs, dst_x, dst_y, w, h, VNC_ENCODING_COPYRECT);
976a86
-    vnc_write_u16(vs, src_x);
976a86
-    vnc_write_u16(vs, src_y);
976a86
-    vnc_unlock_output(vs);
976a86
-    vnc_flush(vs);
976a86
-}
976a86
-
976a86
-static void vnc_dpy_copy(DisplayChangeListener *dcl,
976a86
-                         int src_x, int src_y,
976a86
-                         int dst_x, int dst_y, int w, int h)
976a86
-{
976a86
-    VncDisplay *vd = container_of(dcl, VncDisplay, dcl);
976a86
-    VncState *vs, *vn;
976a86
-    uint8_t *src_row;
976a86
-    uint8_t *dst_row;
976a86
-    int i, x, y, pitch, inc, w_lim, s;
976a86
-    int cmp_bytes;
976a86
-
976a86
-    vnc_refresh_server_surface(vd);
976a86
-    QTAILQ_FOREACH_SAFE(vs, &vd->clients, next, vn) {
976a86
-        if (vnc_has_feature(vs, VNC_FEATURE_COPYRECT)) {
976a86
-            vs->force_update = 1;
976a86
-            vnc_update_client_sync(vs, 1);
976a86
-            /* vs might be free()ed here */
976a86
-        }
976a86
-    }
976a86
-
976a86
-    /* do bitblit op on the local surface too */
976a86
-    pitch = vnc_server_fb_stride(vd);
976a86
-    src_row = vnc_server_fb_ptr(vd, src_x, src_y);
976a86
-    dst_row = vnc_server_fb_ptr(vd, dst_x, dst_y);
976a86
-    y = dst_y;
976a86
-    inc = 1;
976a86
-    if (dst_y > src_y) {
976a86
-        /* copy backwards */
976a86
-        src_row += pitch * (h-1);
976a86
-        dst_row += pitch * (h-1);
976a86
-        pitch = -pitch;
976a86
-        y = dst_y + h - 1;
976a86
-        inc = -1;
976a86
-    }
619821
-    w_lim = w - (VNC_DIRTY_PIXELS_PER_BIT - (dst_x % VNC_DIRTY_PIXELS_PER_BIT));
619821
-    if (w_lim < 0) {
976a86
-        w_lim = w;
619821
-    } else {
619821
-        w_lim = w - (w_lim % VNC_DIRTY_PIXELS_PER_BIT);
619821
-    }
976a86
-    for (i = 0; i < h; i++) {
976a86
-        for (x = 0; x <= w_lim;
976a86
-                x += s, src_row += cmp_bytes, dst_row += cmp_bytes) {
976a86
-            if (x == w_lim) {
976a86
-                if ((s = w - w_lim) == 0)
976a86
-                    break;
976a86
-            } else if (!x) {
619821
-                s = (VNC_DIRTY_PIXELS_PER_BIT -
619821
-                    (dst_x % VNC_DIRTY_PIXELS_PER_BIT));
976a86
-                s = MIN(s, w_lim);
976a86
-            } else {
619821
-                s = VNC_DIRTY_PIXELS_PER_BIT;
976a86
-            }
976a86
-            cmp_bytes = s * VNC_SERVER_FB_BYTES;
976a86
-            if (memcmp(src_row, dst_row, cmp_bytes) == 0)
976a86
-                continue;
976a86
-            memmove(dst_row, src_row, cmp_bytes);
976a86
-            QTAILQ_FOREACH(vs, &vd->clients, next) {
976a86
-                if (!vnc_has_feature(vs, VNC_FEATURE_COPYRECT)) {
619821
-                    set_bit(((x + dst_x) / VNC_DIRTY_PIXELS_PER_BIT),
619821
-                            vs->dirty[y]);
976a86
-                }
976a86
-            }
976a86
-        }
976a86
-        src_row += pitch - w * VNC_SERVER_FB_BYTES;
976a86
-        dst_row += pitch - w * VNC_SERVER_FB_BYTES;
976a86
-        y += inc;
976a86
-    }
976a86
-
976a86
-    QTAILQ_FOREACH(vs, &vd->clients, next) {
976a86
-        if (vnc_has_feature(vs, VNC_FEATURE_COPYRECT)) {
976a86
-            vnc_copy(vs, src_x, src_y, dst_x, dst_y, w, h);
976a86
-        }
976a86
-    }
976a86
-}
976a86
-
976a86
 static void vnc_mouse_set(DisplayChangeListener *dcl,
976a86
                           int x, int y, int visible)
976a86
 {
619821
@@ -883,13 +792,6 @@ static int find_and_clear_dirty_height(struct VncState *vs,
976a86
     return h;
976a86
 }
976a86
 
976a86
-static int vnc_update_client_sync(VncState *vs, int has_dirty)
976a86
-{
976a86
-    int ret = vnc_update_client(vs, has_dirty);
976a86
-    vnc_jobs_join(vs);
976a86
-    return ret;
976a86
-}
976a86
-
976a86
 static int vnc_update_client(VncState *vs, int has_dirty)
976a86
 {
976a86
     if (vs->need_update && vs->csock != -1) {
619821
@@ -2936,7 +2838,6 @@ static void vnc_listen_websocket_read(void *opaque)
976a86
 static const DisplayChangeListenerOps dcl_ops = {
976a86
     .dpy_name          = "vnc",
976a86
     .dpy_refresh       = vnc_refresh,
976a86
-    .dpy_gfx_copy      = vnc_dpy_copy,
976a86
     .dpy_gfx_update    = vnc_dpy_update,
976a86
     .dpy_gfx_switch    = vnc_dpy_switch,
976a86
     .dpy_mouse_set     = vnc_mouse_set,
976a86
-- 
976a86
1.8.3.1
976a86