619821
From 3d842d39e26560dfb7679d88746c314a3545ca18 Mon Sep 17 00:00:00 2001
619821
From: Gerd Hoffmann <kraxel@redhat.com>
619821
Date: Wed, 22 Feb 2017 12:36:24 +0100
619821
Subject: [PATCH 06/24] ui/vnc: fix potential memory corruption issues
619821
MIME-Version: 1.0
619821
Content-Type: text/plain; charset=UTF-8
619821
Content-Transfer-Encoding: 8bit
619821
619821
RH-Author: Gerd Hoffmann <kraxel@redhat.com>
619821
Message-id: <1487766986-6329-7-git-send-email-kraxel@redhat.com>
619821
Patchwork-id: 73977
619821
O-Subject: [RHEL-7.4 qemu-kvm PATCH 6/8] ui/vnc: fix potential memory corruption issues
619821
Bugzilla: 1377977
619821
RH-Acked-by: Thomas Huth <thuth@redhat.com>
619821
RH-Acked-by: Marc-André Lureau <mlureau@redhat.com>
619821
RH-Acked-by: Laurent Vivier <lvivier@redhat.com>
619821
619821
From: Peter Lieven <pl@kamp.de>
619821
619821
this patch makes the VNC server work correctly if the
619821
server surface and the guest surface have different sizes.
619821
619821
Basically the server surface is adjusted to not exceed VNC_MAX_WIDTH
619821
x VNC_MAX_HEIGHT and additionally the width is rounded up to multiple of
619821
VNC_DIRTY_PIXELS_PER_BIT.
619821
619821
If we have a resolution whose width is not dividable by VNC_DIRTY_PIXELS_PER_BIT
619821
we now get a small black bar on the right of the screen.
619821
619821
If the surface is too big to fit the limits only the upper left area is shown.
619821
619821
On top of that this fixes 2 memory corruption issues:
619821
619821
The first was actually discovered during playing
619821
around with a Windows 7 vServer. During resolution
619821
change in Windows 7 it happens sometimes that Windows
619821
changes to an intermediate resolution where
619821
server_stride % cmp_bytes != 0 (in vnc_refresh_server_surface).
619821
This happens only if width % VNC_DIRTY_PIXELS_PER_BIT != 0.
619821
619821
The second is a theoretical issue, but is maybe exploitable
619821
by the guest. If for some reason the guest surface size is bigger
619821
than VNC_MAX_WIDTH x VNC_MAX_HEIGHT we end up in severe corruption since
619821
this limit is nowhere enforced.
619821
619821
Signed-off-by: Peter Lieven <pl@kamp.de>
619821
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
619821
(cherry picked from commit bea60dd7679364493a0d7f5b54316c767cf894ef)
619821
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
619821
619821
Conflicts:
619821
	ui/vnc.c  [ in pointer_event, input subsystem differences ]
619821
---
619821
 ui/vnc.c | 149 +++++++++++++++++++++++++++++----------------------------------
619821
 ui/vnc.h |  14 +++---
619821
 2 files changed, 77 insertions(+), 86 deletions(-)
619821
619821
diff --git a/ui/vnc.c b/ui/vnc.c
619821
index 51f95be..80b7792 100644
619821
--- a/ui/vnc.c
619821
+++ b/ui/vnc.c
619821
@@ -427,14 +427,10 @@ static void framebuffer_update_request(VncState *vs, int incremental,
619821
 static void vnc_refresh(DisplayChangeListener *dcl);
619821
 static int vnc_refresh_server_surface(VncDisplay *vd);
619821
 
619821
-static void vnc_dpy_update(DisplayChangeListener *dcl,
619821
-                           int x, int y, int w, int h)
619821
-{
619821
-    VncDisplay *vd = container_of(dcl, VncDisplay, dcl);
619821
-    struct VncSurface *s = &vd->guest;
619821
-    int width = surface_width(vd->ds);
619821
-    int height = surface_height(vd->ds);
619821
-
619821
+static void vnc_set_area_dirty(DECLARE_BITMAP(dirty[VNC_MAX_HEIGHT],
619821
+                               VNC_MAX_WIDTH / VNC_DIRTY_PIXELS_PER_BIT),
619821
+                               int width, int height,
619821
+                               int x, int y, int w, int h) {
619821
     /* this is needed this to ensure we updated all affected
619821
      * blocks if x % VNC_DIRTY_PIXELS_PER_BIT != 0 */
619821
     w += (x % VNC_DIRTY_PIXELS_PER_BIT);
619821
@@ -446,11 +442,22 @@ static void vnc_dpy_update(DisplayChangeListener *dcl,
619821
     h = MIN(y + h, height);
619821
 
619821
     for (; y < h; y++) {
619821
-        bitmap_set(s->dirty[y], x / VNC_DIRTY_PIXELS_PER_BIT,
619821
+        bitmap_set(dirty[y], x / VNC_DIRTY_PIXELS_PER_BIT,
619821
                    DIV_ROUND_UP(w, VNC_DIRTY_PIXELS_PER_BIT));
619821
     }
619821
 }
619821
 
619821
+static void vnc_dpy_update(DisplayChangeListener *dcl,
619821
+                           int x, int y, int w, int h)
619821
+{
619821
+    VncDisplay *vd = container_of(dcl, VncDisplay, dcl);
619821
+    struct VncSurface *s = &vd->guest;
619821
+    int width = pixman_image_get_width(vd->server);
619821
+    int height = pixman_image_get_height(vd->server);
619821
+
619821
+    vnc_set_area_dirty(s->dirty, width, height, x, y, w, h);
619821
+}
619821
+
619821
 void vnc_framebuffer_update(VncState *vs, int x, int y, int w, int h,
619821
                             int32_t encoding)
619821
 {
619821
@@ -512,17 +519,15 @@ void buffer_advance(Buffer *buf, size_t len)
619821
 
619821
 static void vnc_desktop_resize(VncState *vs)
619821
 {
619821
-    DisplaySurface *ds = vs->vd->ds;
619821
-
619821
     if (vs->csock == -1 || !vnc_has_feature(vs, VNC_FEATURE_RESIZE)) {
619821
         return;
619821
     }
619821
-    if (vs->client_width == surface_width(ds) &&
619821
-        vs->client_height == surface_height(ds)) {
619821
+    if (vs->client_width == pixman_image_get_width(vs->vd->server) &&
619821
+        vs->client_height == pixman_image_get_height(vs->vd->server)) {
619821
         return;
619821
     }
619821
-    vs->client_width = surface_width(ds);
619821
-    vs->client_height = surface_height(ds);
619821
+    vs->client_width = pixman_image_get_width(vs->vd->server);
619821
+    vs->client_height = pixman_image_get_height(vs->vd->server);
619821
     vnc_lock_output(vs);
619821
     vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
619821
     vnc_write_u8(vs, 0);
619821
@@ -566,31 +571,24 @@ void *vnc_server_fb_ptr(VncDisplay *vd, int x, int y)
619821
     ptr += x * VNC_SERVER_FB_BYTES;
619821
     return ptr;
619821
 }
619821
-/* this sets only the visible pixels of a dirty bitmap */
619821
-#define VNC_SET_VISIBLE_PIXELS_DIRTY(bitmap, w, h) {\
619821
-        int y;\
619821
-        memset(bitmap, 0x00, sizeof(bitmap));\
619821
-        for (y = 0; y < h; y++) {\
619821
-            bitmap_set(bitmap[y], 0,\
619821
-                       DIV_ROUND_UP(w, VNC_DIRTY_PIXELS_PER_BIT));\
619821
-        } \
619821
-    }
619821
 
619821
 static void vnc_dpy_switch(DisplayChangeListener *dcl,
619821
                            DisplaySurface *surface)
619821
 {
619821
     VncDisplay *vd = container_of(dcl, VncDisplay, dcl);
619821
     VncState *vs;
619821
+    int width, height;
619821
 
619821
     vnc_abort_display_jobs(vd);
619821
 
619821
     /* server surface */
619821
     qemu_pixman_image_unref(vd->server);
619821
     vd->ds = surface;
619821
+    width = MIN(VNC_MAX_WIDTH, ROUND_UP(surface_width(vd->ds),
619821
+                                        VNC_DIRTY_PIXELS_PER_BIT));
619821
+    height = MIN(VNC_MAX_HEIGHT, surface_height(vd->ds));
619821
     vd->server = pixman_image_create_bits(VNC_SERVER_FB_FORMAT,
619821
-                                          surface_width(vd->ds),
619821
-                                          surface_height(vd->ds),
619821
-                                          NULL, 0);
619821
+                                          width, height, NULL, 0);
619821
 
619821
     /* guest surface */
619821
 #if 0 /* FIXME */
619821
@@ -600,9 +598,9 @@ static void vnc_dpy_switch(DisplayChangeListener *dcl,
619821
     qemu_pixman_image_unref(vd->guest.fb);
619821
     vd->guest.fb = pixman_image_ref(surface->image);
619821
     vd->guest.format = surface->format;
619821
-    VNC_SET_VISIBLE_PIXELS_DIRTY(vd->guest.dirty,
619821
-                                 surface_width(vd->ds),
619821
-                                 surface_height(vd->ds));
619821
+    memset(vd->guest.dirty, 0x00, sizeof(vd->guest.dirty));
619821
+    vnc_set_area_dirty(vd->guest.dirty, width, height, 0, 0,
619821
+                       width, height);
619821
 
619821
     QTAILQ_FOREACH(vs, &vd->clients, next) {
619821
         vnc_colordepth(vs);
619821
@@ -610,9 +608,9 @@ static void vnc_dpy_switch(DisplayChangeListener *dcl,
619821
         if (vs->vd->cursor) {
619821
             vnc_cursor_define(vs);
619821
         }
619821
-        VNC_SET_VISIBLE_PIXELS_DIRTY(vs->dirty,
619821
-                                     surface_width(vd->ds),
619821
-                                     surface_height(vd->ds));
619821
+        memset(vs->dirty, 0x00, sizeof(vs->dirty));
619821
+        vnc_set_area_dirty(vs->dirty, width, height, 0, 0,
619821
+                           width, height);
619821
     }
619821
 }
619821
 
619821
@@ -916,8 +914,8 @@ static int vnc_update_client(VncState *vs, int has_dirty)
619821
          */
619821
         job = vnc_job_new(vs);
619821
 
619821
-        height = MIN(pixman_image_get_height(vd->server), vs->client_height);
619821
-        width = MIN(pixman_image_get_width(vd->server), vs->client_width);
619821
+        height = pixman_image_get_height(vd->server);
619821
+        width = pixman_image_get_width(vd->server);
619821
 
619821
         y = 0;
619821
         for (;;) {
619821
@@ -1500,8 +1498,8 @@ static void check_pointer_type_change(Notifier *notifier, void *data)
619821
         vnc_write_u8(vs, 0);
619821
         vnc_write_u16(vs, 1);
619821
         vnc_framebuffer_update(vs, absolute, 0,
619821
-                               surface_width(vs->vd->ds),
619821
-                               surface_height(vs->vd->ds),
619821
+                               pixman_image_get_width(vs->vd->server),
619821
+                               pixman_image_get_height(vs->vd->server),
619821
                                VNC_ENCODING_POINTER_TYPE_CHANGE);
619821
         vnc_unlock_output(vs);
619821
         vnc_flush(vs);
619821
@@ -1513,8 +1511,8 @@ static void pointer_event(VncState *vs, int button_mask, int x, int y)
619821
 {
619821
     int buttons = 0;
619821
     int dz = 0;
619821
-    int width = surface_width(vs->vd->ds);
619821
-    int height = surface_height(vs->vd->ds);
619821
+    int width = pixman_image_get_width(vs->vd->server);
619821
+    int height = pixman_image_get_height(vs->vd->server);
619821
 
619821
     if (button_mask & 0x01)
619821
         buttons |= MOUSE_EVENT_LBUTTON;
619821
@@ -1866,29 +1864,18 @@ static void ext_key_event(VncState *vs, int down,
619821
 }
619821
 
619821
 static void framebuffer_update_request(VncState *vs, int incremental,
619821
-                                       int x_position, int y_position,
619821
-                                       int w, int h)
619821
+                                       int x, int y, int w, int h)
619821
 {
619821
-    int i;
619821
-    const size_t width = surface_width(vs->vd->ds) / VNC_DIRTY_PIXELS_PER_BIT;
619821
-    const size_t height = surface_height(vs->vd->ds);
619821
-
619821
-    if (y_position > height) {
619821
-        y_position = height;
619821
-    }
619821
-    if (y_position + h >= height) {
619821
-        h = height - y_position;
619821
-    }
619821
+    int width = pixman_image_get_width(vs->vd->server);
619821
+    int height = pixman_image_get_height(vs->vd->server);
619821
 
619821
     vs->need_update = 1;
619821
-    if (!incremental) {
619821
-        vs->force_update = 1;
619821
-        for (i = 0; i < h; i++) {
619821
-            bitmap_set(vs->dirty[y_position + i], 0, width);
619821
-            bitmap_clear(vs->dirty[y_position + i], width,
619821
-                         VNC_DIRTY_BITS - width);
619821
-        }
619821
+
619821
+    if (incremental) {
619821
+        return;
619821
     }
619821
+
619821
+    vnc_set_area_dirty(vs->dirty, width, height, x, y, w, h);
619821
 }
619821
 
619821
 static void send_ext_key_event_ack(VncState *vs)
619821
@@ -1898,8 +1885,8 @@ static void send_ext_key_event_ack(VncState *vs)
619821
     vnc_write_u8(vs, 0);
619821
     vnc_write_u16(vs, 1);
619821
     vnc_framebuffer_update(vs, 0, 0,
619821
-                           surface_width(vs->vd->ds),
619821
-                           surface_height(vs->vd->ds),
619821
+                           pixman_image_get_width(vs->vd->server),
619821
+                           pixman_image_get_height(vs->vd->server),
619821
                            VNC_ENCODING_EXT_KEY_EVENT);
619821
     vnc_unlock_output(vs);
619821
     vnc_flush(vs);
619821
@@ -1912,8 +1899,8 @@ static void send_ext_audio_ack(VncState *vs)
619821
     vnc_write_u8(vs, 0);
619821
     vnc_write_u16(vs, 1);
619821
     vnc_framebuffer_update(vs, 0, 0,
619821
-                           surface_width(vs->vd->ds),
619821
-                           surface_height(vs->vd->ds),
619821
+                           pixman_image_get_width(vs->vd->server),
619821
+                           pixman_image_get_height(vs->vd->server),
619821
                            VNC_ENCODING_AUDIO);
619821
     vnc_unlock_output(vs);
619821
     vnc_flush(vs);
619821
@@ -2101,8 +2088,8 @@ static void vnc_colordepth(VncState *vs)
619821
         vnc_write_u8(vs, 0);
619821
         vnc_write_u16(vs, 1); /* number of rects */
619821
         vnc_framebuffer_update(vs, 0, 0,
619821
-                               surface_width(vs->vd->ds),
619821
-                               surface_height(vs->vd->ds),
619821
+                               pixman_image_get_width(vs->vd->server),
619821
+                               pixman_image_get_height(vs->vd->server),
619821
                                VNC_ENCODING_WMVi);
619821
         pixel_format_message(vs);
619821
         vnc_unlock_output(vs);
619821
@@ -2317,8 +2304,8 @@ static int protocol_client_init(VncState *vs, uint8_t *data, size_t len)
619821
     }
619821
     vnc_set_share_mode(vs, mode);
619821
 
619821
-    vs->client_width = surface_width(vs->vd->ds);
619821
-    vs->client_height = surface_height(vs->vd->ds);
619821
+    vs->client_width = pixman_image_get_width(vs->vd->server);
619821
+    vs->client_height = pixman_image_get_height(vs->vd->server);
619821
     vnc_write_u16(vs, vs->client_width);
619821
     vnc_write_u16(vs, vs->client_height);
619821
 
619821
@@ -2685,12 +2672,12 @@ static void vnc_rect_updated(VncDisplay *vd, int x, int y, struct timeval * tv)
619821
 
619821
 static int vnc_refresh_server_surface(VncDisplay *vd)
619821
 {
619821
-    int width = pixman_image_get_width(vd->guest.fb);
619821
-    int height = pixman_image_get_height(vd->guest.fb);
619821
-    int y;
619821
+    int width = MIN(pixman_image_get_width(vd->guest.fb),
619821
+                    pixman_image_get_width(vd->server));
619821
+    int height = MIN(pixman_image_get_height(vd->guest.fb),
619821
+                     pixman_image_get_height(vd->server));
619821
+    int cmp_bytes, server_stride, min_stride, guest_stride, y = 0;
619821
     uint8_t *guest_row0 = NULL, *server_row0;
619821
-    int guest_stride = 0, server_stride;
619821
-    int cmp_bytes;
619821
     VncState *vs;
619821
     int has_dirty = 0;
619821
     pixman_image_t *tmpbuf = NULL;
619821
@@ -2707,10 +2694,10 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
619821
      * Check and copy modified bits from guest to server surface.
619821
      * Update server dirty map.
619821
      */
619821
-    cmp_bytes = VNC_DIRTY_PIXELS_PER_BIT * VNC_SERVER_FB_BYTES;
619821
-    if (cmp_bytes > vnc_server_fb_stride(vd)) {
619821
-        cmp_bytes = vnc_server_fb_stride(vd);
619821
-    }
619821
+    server_row0 = (uint8_t *)pixman_image_get_data(vd->server);
619821
+    server_stride = guest_stride = pixman_image_get_stride(vd->server);
619821
+    cmp_bytes = MIN(VNC_DIRTY_PIXELS_PER_BIT * VNC_SERVER_FB_BYTES,
619821
+                    server_stride);
619821
     if (vd->guest.format != VNC_SERVER_FB_FORMAT) {
619821
         int width = pixman_image_get_width(vd->server);
619821
         tmpbuf = qemu_pixman_linebuf_create(VNC_SERVER_FB_FORMAT, width);
619821
@@ -2718,10 +2705,8 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
619821
         guest_row0 = (uint8_t *)pixman_image_get_data(vd->guest.fb);
619821
         guest_stride = pixman_image_get_stride(vd->guest.fb);
619821
     }
619821
-    server_row0 = (uint8_t *)pixman_image_get_data(vd->server);
619821
-    server_stride = pixman_image_get_stride(vd->server);
619821
+    min_stride = MIN(server_stride, guest_stride);
619821
 
619821
-    y = 0;
619821
     for (;;) {
619821
         int x;
619821
         uint8_t *guest_ptr, *server_ptr;
619821
@@ -2747,13 +2732,17 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
619821
 
619821
         for (; x < DIV_ROUND_UP(width, VNC_DIRTY_PIXELS_PER_BIT);
619821
              x++, guest_ptr += cmp_bytes, server_ptr += cmp_bytes) {
619821
+            int _cmp_bytes = cmp_bytes;
619821
             if (!test_and_clear_bit(x, vd->guest.dirty[y])) {
619821
                 continue;
619821
             }
619821
-            if (memcmp(server_ptr, guest_ptr, cmp_bytes) == 0) {
619821
+            if ((x + 1) * cmp_bytes > min_stride) {
619821
+                _cmp_bytes = min_stride - x * cmp_bytes;
619821
+            }
619821
+            if (memcmp(server_ptr, guest_ptr, _cmp_bytes) == 0) {
619821
                 continue;
619821
             }
619821
-            memcpy(server_ptr, guest_ptr, cmp_bytes);
619821
+            memcpy(server_ptr, guest_ptr, _cmp_bytes);
619821
             if (!vd->non_adaptive) {
619821
                 vnc_rect_updated(vd, x * VNC_DIRTY_PIXELS_PER_BIT,
619821
                                  y, &tv);
619821
diff --git a/ui/vnc.h b/ui/vnc.h
619821
index ebf4bdd..8d534b6 100644
619821
--- a/ui/vnc.h
619821
+++ b/ui/vnc.h
619821
@@ -77,14 +77,15 @@ typedef void VncSendHextileTile(VncState *vs,
619821
                                 void *last_fg,
619821
                                 int *has_bg, int *has_fg);
619821
 
619821
-/* VNC_MAX_WIDTH must be a multiple of 16. */
619821
-#define VNC_MAX_WIDTH 2560
619821
-#define VNC_MAX_HEIGHT 2048
619821
-
619821
 /* VNC_DIRTY_PIXELS_PER_BIT is the number of dirty pixels represented
619821
- * by one bit in the dirty bitmap */
619821
+ * by one bit in the dirty bitmap, should be a power of 2 */
619821
 #define VNC_DIRTY_PIXELS_PER_BIT 16
619821
 
619821
+/* VNC_MAX_WIDTH must be a multiple of VNC_DIRTY_PIXELS_PER_BIT. */
619821
+
619821
+#define VNC_MAX_WIDTH ROUND_UP(2560, VNC_DIRTY_PIXELS_PER_BIT)
619821
+#define VNC_MAX_HEIGHT 2048
619821
+
619821
 /* VNC_DIRTY_BITS is the number of bits in the dirty bitmap. */
619821
 #define VNC_DIRTY_BITS (VNC_MAX_WIDTH / VNC_DIRTY_PIXELS_PER_BIT)
619821
 
619821
@@ -126,7 +127,8 @@ typedef struct VncRectStat VncRectStat;
619821
 struct VncSurface
619821
 {
619821
     struct timeval last_freq_check;
619821
-    DECLARE_BITMAP(dirty[VNC_MAX_HEIGHT], VNC_MAX_WIDTH / 16);
619821
+    DECLARE_BITMAP(dirty[VNC_MAX_HEIGHT],
619821
+                   VNC_MAX_WIDTH / VNC_DIRTY_PIXELS_PER_BIT);
619821
     VncRectStat stats[VNC_STAT_ROWS][VNC_STAT_COLS];
619821
     pixman_image_t *fb;
619821
     pixman_format_code_t format;
619821
-- 
619821
1.8.3.1
619821