From 7afafb7ecc968cb829e41d7b12f44b43ddda1f75 Mon Sep 17 00:00:00 2001
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Tue, 14 Mar 2017 13:26:59 +0100
Subject: [PATCH] cirrus/vnc: zap bitblit support from console code.

There is a special code path (dpy_gfx_copy) to allow graphic emulation
notify user interface code about bitblit operations carryed out by
guests.  It is supported by cirrus and vnc server.  The intended purpose
is to optimize display scrolls and just send over the scroll op instead
of a full display update.

This is rarely used these days though because modern guests simply don't
use the cirrus blitter any more.  Any linux guest using the cirrus drm
driver doesn't.  Any windows guest newer than winxp doesn't ship with a
cirrus driver any more and thus uses the cirrus as simple framebuffer.

So this code tends to bitrot and bugs can go unnoticed for a long time.
See for example commit "3e10c3e vnc: fix qemu crash because of SIGSEGV"
which fixes a bug lingering in the code for almost a year, added by
commit "c7628bf vnc: only alloc server surface with clients connected".

Also the vnc server will throttle the frame rate in case it figures the
network can't keep up (send buffers are full).  This doesn't work with
dpy_gfx_copy, for any copy operation sent to the vnc client we have to
send all outstanding updates beforehand, otherwise the vnc client might
run the client side blit on outdated data and thereby corrupt the
display.  So this dpy_gfx_copy "optimization" might even make things
worse on slow network links.

Lets kill it once for all.

Oh, and one more reason: Turns out (after writing the patch) we have a
security bug in that code path ...

Fixes: CVE-2016-9603
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Message-id: 1489494419-14340-1-git-send-email-kraxel@redhat.com
(cherry picked from commit 50628d3479e4f9aa97e323506856e394fe7ad7a6)
[BR: BSC#1028656]
Signed-off-by: Bruce Rogers <brogers@suse.com>
---
 hw/cirrus_vga.c      | 10 ++----
 include/ui/console.h | 18 ----------
 ui/console.c         |  8 -----
 ui/vnc.c             | 94 ----------------------------------------------------
 4 files changed, 2 insertions(+), 128 deletions(-)

diff --git a/hw/cirrus_vga.c b/hw/cirrus_vga.c
index 2edfeb0fe1..b08ff7d07d 100644
--- a/hw/cirrus_vga.c
+++ b/hw/cirrus_vga.c
@@ -786,24 +786,18 @@ static int cirrus_do_copy(CirrusVGAState *s, int dst, int src, int w, int h)
         }
     }
 
-    /* we have to flush all pending changes so that the copy
-       is generated at the appropriate moment in time */
-    if (notify)
-	vga_hw_update();
-
     (*s->cirrus_rop) (s, s->vga.vram_ptr + s->cirrus_blt_dstaddr,
                       s->vga.vram_ptr + s->cirrus_blt_srcaddr,
 		      s->cirrus_blt_dstpitch, s->cirrus_blt_srcpitch,
 		      s->cirrus_blt_width, s->cirrus_blt_height);
 
     if (notify)
-	qemu_console_copy(s->vga.ds,
-			  sx, sy, dx, dy,
+	dpy_gfx_update(s->vga.ds, dx, dy,
 			  s->cirrus_blt_width / depth,
 			  s->cirrus_blt_height);
 
     /* we don't have to notify the display that this portion has
-       changed since qemu_console_copy implies this */
+       changed since dpy_gfx_update implies this */
 
     cirrus_invalidate_region(s, s->cirrus_blt_dstaddr,
 				s->cirrus_blt_dstpitch, s->cirrus_blt_width,
diff --git a/include/ui/console.h b/include/ui/console.h
index d8db7946bc..d78b098199 100644
--- a/include/ui/console.h
+++ b/include/ui/console.h
@@ -156,9 +156,6 @@ struct DisplayChangeListener {
     void (*dpy_gfx_update)(struct DisplayState *s, int x, int y, int w, int h);
     void (*dpy_gfx_resize)(struct DisplayState *s);
     void (*dpy_gfx_setdata)(struct DisplayState *s);
-    void (*dpy_gfx_copy)(struct DisplayState *s, int src_x, int src_y,
-                         int dst_x, int dst_y, int w, int h);
-
     void (*dpy_text_cursor)(struct DisplayState *s, int x, int y);
     void (*dpy_text_resize)(struct DisplayState *s, int w, int h);
     void (*dpy_text_update)(struct DisplayState *s, int x, int y, int w, int h);
@@ -276,19 +273,6 @@ static inline void dpy_refresh(DisplayState *s)
     }
 }
 
-static inline void dpy_gfx_copy(struct DisplayState *s, int src_x, int src_y,
-                             int dst_x, int dst_y, int w, int h)
-{
-    struct DisplayChangeListener *dcl;
-    QLIST_FOREACH(dcl, &s->listeners, next) {
-        if (dcl->dpy_gfx_copy) {
-            dcl->dpy_gfx_copy(s, src_x, src_y, dst_x, dst_y, w, h);
-        } else { /* TODO */
-            dcl->dpy_gfx_update(s, dst_x, dst_y, w, h);
-        }
-    }
-}
-
 static inline void dpy_text_cursor(struct DisplayState *s, int x, int y)
 {
     struct DisplayChangeListener *dcl;
@@ -448,8 +432,6 @@ void text_consoles_set_display(DisplayState *ds);
 void console_select(unsigned int index);
 void console_color_init(DisplayState *ds);
 void qemu_console_resize(DisplayState *ds, int width, int height);
-void qemu_console_copy(DisplayState *ds, int src_x, int src_y,
-                       int dst_x, int dst_y, int w, int h);
 
 /* sdl.c */
 void sdl_display_init(DisplayState *ds, int full_screen, int no_frame);
diff --git a/ui/console.c b/ui/console.c
index d5e519f10d..fb148a488a 100644
--- a/ui/console.c
+++ b/ui/console.c
@@ -1597,14 +1597,6 @@ void qemu_console_resize(DisplayState *ds, int width, int height)
     }
 }
 
-void qemu_console_copy(DisplayState *ds, int src_x, int src_y,
-                       int dst_x, int dst_y, int w, int h)
-{
-    if (is_graphic_console()) {
-        dpy_gfx_copy(ds, src_x, src_y, dst_x, dst_y, w, h);
-    }
-}
-
 PixelFormat qemu_different_endianness_pixelformat(int bpp)
 {
     PixelFormat pf;
diff --git a/ui/vnc.c b/ui/vnc.c
index 65ed9e1c63..f26bc6d9d2 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -419,7 +419,6 @@ out_error:
 */
 
 static int vnc_update_client(VncState *vs, int has_dirty);
-static int vnc_update_client_sync(VncState *vs, int has_dirty);
 static void vnc_disconnect_start(VncState *vs);
 static void vnc_init_timer(VncDisplay *vd);
 static void vnc_remove_timer(VncDisplay *vd);
@@ -722,91 +721,6 @@ int vnc_send_framebuffer_update(VncState *vs, int x, int y, int w, int h)
     return n;
 }
 
-static void vnc_copy(VncState *vs, int src_x, int src_y, int dst_x, int dst_y, int w, int h)
-{
-    /* send bitblit op to the vnc client */
-    vnc_lock_output(vs);
-    vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
-    vnc_write_u8(vs, 0);
-    vnc_write_u16(vs, 1); /* number of rects */
-    vnc_framebuffer_update(vs, dst_x, dst_y, w, h, VNC_ENCODING_COPYRECT);
-    vnc_write_u16(vs, src_x);
-    vnc_write_u16(vs, src_y);
-    vnc_unlock_output(vs);
-    vnc_flush(vs);
-}
-
-static void vnc_dpy_copy(DisplayState *ds, int src_x, int src_y, int dst_x, int dst_y, int w, int h)
-{
-    VncDisplay *vd = ds->opaque;
-    VncState *vs, *vn;
-    uint8_t *src_row;
-    uint8_t *dst_row;
-    int i, x, y, pitch, inc, w_lim, s;
-    int cmp_bytes;
-
-    vnc_refresh_server_surface(vd);
-    QTAILQ_FOREACH_SAFE(vs, &vd->clients, next, vn) {
-        if (vnc_has_feature(vs, VNC_FEATURE_COPYRECT)) {
-            vs->force_update = 1;
-            vnc_update_client_sync(vs, 1);
-            /* vs might be free()ed here */
-        }
-    }
-
-    /* do bitblit op on the local surface too */
-    pitch = vnc_server_fb_stride(vd);
-    src_row = vnc_server_fb_ptr(vd, src_x, src_y);
-    dst_row = vnc_server_fb_ptr(vd, dst_x, dst_y);
-    y = dst_y;
-    inc = 1;
-    if (dst_y > src_y) {
-        /* copy backwards */
-        src_row += pitch * (h-1);
-        dst_row += pitch * (h-1);
-        pitch = -pitch;
-        y = dst_y + h - 1;
-        inc = -1;
-    }
-    w_lim = w - (16 - (dst_x % 16));
-    if (w_lim < 0)
-        w_lim = w;
-    else
-        w_lim = w - (w_lim % 16);
-    for (i = 0; i < h; i++) {
-        for (x = 0; x <= w_lim;
-                x += s, src_row += cmp_bytes, dst_row += cmp_bytes) {
-            if (x == w_lim) {
-                if ((s = w - w_lim) == 0)
-                    break;
-            } else if (!x) {
-                s = (16 - (dst_x % 16));
-                s = MIN(s, w_lim);
-            } else {
-                s = 16;
-            }
-            cmp_bytes = s * VNC_SERVER_FB_BYTES;
-            if (memcmp(src_row, dst_row, cmp_bytes) == 0)
-                continue;
-            memmove(dst_row, src_row, cmp_bytes);
-            QTAILQ_FOREACH(vs, &vd->clients, next) {
-                if (!vnc_has_feature(vs, VNC_FEATURE_COPYRECT)) {
-                    set_bit(((x + dst_x) / 16), vs->dirty[y]);
-                }
-            }
-        }
-        src_row += pitch - w * VNC_SERVER_FB_BYTES;
-        dst_row += pitch - w * VNC_SERVER_FB_BYTES;
-        y += inc;
-    }
-
-    QTAILQ_FOREACH(vs, &vd->clients, next) {
-        if (vnc_has_feature(vs, VNC_FEATURE_COPYRECT)) {
-            vnc_copy(vs, src_x, src_y, dst_x, dst_y, w, h);
-        }
-    }
-}
-
 static void vnc_mouse_set(DisplayState *ds, int x, int y, int visible)
 {
     /* can we ask the client(s) to move the pointer ??? */
@@ -870,13 +784,6 @@ static int find_and_clear_dirty_height(struct VncState *vs,
     return h;
 }
 
-static int vnc_update_client_sync(VncState *vs, int has_dirty)
-{
-    int ret = vnc_update_client(vs, has_dirty);
-    vnc_jobs_join(vs);
-    return ret;
-}
-
 static int vnc_update_client(VncState *vs, int has_dirty)
 {
     if (vs->need_update && vs->csock != -1) {
@@ -2955,7 +2862,6 @@ void vnc_display_init(DisplayState *ds)
     qemu_mutex_init(&vs->mutex);
     vnc_start_worker_thread();
 
-    dcl->dpy_gfx_copy = vnc_dpy_copy;
     dcl->dpy_gfx_update = vnc_dpy_update;
     dcl->dpy_gfx_resize = vnc_dpy_resize;
     dcl->dpy_gfx_setdata = vnc_dpy_setdata;
