From 46c217bff07c9d7e439e2b0e37b7409fd2cf5bd3 Mon Sep 17 00:00:00 2001
From: Peter Lieven <pl@kamp.de>
Date: Mon, 30 Jun 2014 10:57:51 +0200
Subject: [PATCH] ui/vnc: fix potential memory corruption issues

this patch makes the VNC server work correctly if the
server surface and the guest surface have different sizes.

Basically the server surface is adjusted to not exceed VNC_MAX_WIDTH
x VNC_MAX_HEIGHT and additionally the width is rounded up to multiple of
VNC_DIRTY_PIXELS_PER_BIT.

If we have a resolution whose width is not dividable by VNC_DIRTY_PIXELS_PER_BIT
we now get a small black bar on the right of the screen.

If the surface is too big to fit the limits only the upper left area is shown.

On top of that this fixes 2 memory corruption issues:

The first was actually discovered during playing
around with a Windows 7 vServer. During resolution
change in Windows 7 it happens sometimes that Windows
changes to an intermediate resolution where
server_stride % cmp_bytes != 0 (in vnc_refresh_server_surface).
This happens only if width % VNC_DIRTY_PIXELS_PER_BIT != 0.

The second is a theoretical issue, but is maybe exploitable
by the guest. If for some reason the guest surface size is bigger
than VNC_MAX_WIDTH x VNC_MAX_HEIGHT we end up in severe corruption since
this limit is nowhere enforced.

Signed-off-by: Peter Lieven <pl@kamp.de>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
(cherry picked from commit bea60dd7679364493a0d7f5b54316c767cf894ef)
[LM: BNC#1074701]
Signed-off-by: Lin Ma <lma@suse.com>
---
 osdep.h |   4 ++
 vnc.c   | 151 +++++++++++++++++++++++++++++++---------------------------------
 vnc.h   |  14 +++---
 3 files changed, 86 insertions(+), 83 deletions(-)

diff --git a/osdep.h b/osdep.h
index eae5eee1eb..98d570007f 100644
--- a/osdep.h
+++ b/osdep.h
@@ -57,6 +57,10 @@
 #define MAX(a, b) (((a) > (b)) ? (a) : (b))
 #endif
 
+#ifndef ROUND_UP
+#define ROUND_UP(n,d) (((n) + (d) - 1) & -(d))
+#endif
+
 #ifndef DIV_ROUND_UP
 #define DIV_ROUND_UP(n,d) (((n) + (d) - 1) / (d))
 #endif
diff --git a/vnc.c b/vnc.c
index 138a9d9b2b..a79b5e416d 100644
--- a/vnc.c
+++ b/vnc.c
@@ -44,16 +44,6 @@ static VncDisplay *vnc_display; /* needed for info vnc */
 static DisplayChangeListener *dcl;
 static int allowed_connections = 0;
 
-/* this sets only the visible pixels of a dirty bitmap */
-#define VNC_SET_VISIBLE_PIXELS_DIRTY(bitmap, w, h) {\
-        int y;\
-        memset(bitmap, 0x00, sizeof(bitmap));\
-        for (y = 0; y < h; y++) {\
-            bitmap_set(bitmap[y], 0,\
-                       DIV_ROUND_UP(w, VNC_DIRTY_PIXELS_PER_BIT));\
-        } \
-    }
-
 static char *addr_to_string(const char *format,
                             struct sockaddr_storage *sa,
                             socklen_t salen) {
@@ -366,13 +356,10 @@ static void framebuffer_update_request(VncState *vs, int incremental,
 static void vnc_refresh(void *opaque);
 static int vnc_refresh_server_surface(VncDisplay *vd);
 
-static void vnc_dpy_update(DisplayState *ds, int x, int y, int w, int h)
-{
-    VncDisplay *vd = ds->opaque;
-    struct VncSurface *s = &vd->guest;
-    int width = ds_get_width(ds);
-    int height = ds_get_height(ds);
-
+static void vnc_set_area_dirty(DECLARE_BITMAP(dirty[VNC_MAX_HEIGHT],
+                               VNC_MAX_WIDTH / VNC_DIRTY_PIXELS_PER_BIT),
+                               int width, int height,
+                               int x, int y, int w, int h) {
     /* this is needed this to ensure we updated all affected
      * blocks if x % VNC_DIRTY_PIXELS_PER_BIT != 0 */
     w += (x % VNC_DIRTY_PIXELS_PER_BIT);
@@ -383,9 +370,20 @@ static void vnc_dpy_update(DisplayState *ds, int x, int y, int w, int h)
     w = MIN(x + w, width) - x;
     h = MIN(y + h, height);
 
-    for (; y < h; y++)
-        bitmap_set(s->dirty[y], x / VNC_DIRTY_PIXELS_PER_BIT,
+    for (; y < h; y++) {
+        bitmap_set(dirty[y], x / VNC_DIRTY_PIXELS_PER_BIT,
                    DIV_ROUND_UP(w, VNC_DIRTY_PIXELS_PER_BIT));
+    }
+}
+
+static void vnc_dpy_update(DisplayState *ds, int x, int y, int w, int h)
+{
+    VncDisplay *vd = ds->opaque;
+    struct VncSurface *s = &vd->guest;
+    int width = pixman_image_get_width(vd->server);
+    int height = pixman_image_get_height(vd->server);
+
+    vnc_set_area_dirty(s->dirty, width, height, x, y, w, h);
 }
 
 void vnc_framebuffer_update(VncState *vs, int x, int y, int w, int h,
@@ -442,17 +440,15 @@ void buffer_append(Buffer *buffer, const void *data, size_t len)
 
 static void vnc_desktop_resize(VncState *vs)
 {
-    DisplayState *ds = vs->ds;
-
     if (vs->csock == -1 || !vnc_has_feature(vs, VNC_FEATURE_RESIZE)) {
         return;
     }
-    if (vs->client_width == ds_get_width(ds) &&
-        vs->client_height == ds_get_height(ds)) {
+    if (vs->client_width == pixman_image_get_width(vs->vd->server) &&
+        vs->client_height == pixman_image_get_height(vs->vd->server)) {
         return;
     }
-    vs->client_width = ds_get_width(ds);
-    vs->client_height = ds_get_height(ds);
+    vs->client_width = pixman_image_get_width(vs->vd->server);
+    vs->client_height = pixman_image_get_height(vs->vd->server);
     vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
     vnc_write_u8(vs, 0);
     vnc_write_u16(vs, 1); /* number of rects */
@@ -480,13 +476,15 @@ static void vnc_dpy_resize(DisplayState *ds)
 {
     VncDisplay *vd = ds->opaque;
     VncState *vs = vd->clients;
+    int width, height;
 
     /* server surface */
     qemu_pixman_image_unref(vd->server);
+    width = MIN(VNC_MAX_WIDTH, ROUND_UP(ds_get_width(ds),
+                                        VNC_DIRTY_PIXELS_PER_BIT));
+    height = MIN(VNC_MAX_HEIGHT, ds_get_height(ds));
     vd->server = pixman_image_create_bits(VNC_SERVER_FB_FORMAT,
-                                          ds_get_width(ds),
-                                          ds_get_height(ds),
-                                          NULL, 0);
+                                          width, height, NULL, 0);
 
     /* guest surface */
 #if 0 /* FIXME */
@@ -496,16 +494,16 @@ static void vnc_dpy_resize(DisplayState *ds)
     qemu_pixman_image_unref(vd->guest.fb);
     vd->guest.fb = pixman_image_ref(ds->surface->image);
     vd->guest.format = ds->surface->format;
-    VNC_SET_VISIBLE_PIXELS_DIRTY(vd->guest.dirty,
-                                 ds_get_width(ds),
-                                 ds_get_height(ds));
+    memset(vd->guest.dirty, 0x00, sizeof(vd->guest.dirty));
+    vnc_set_area_dirty(vd->guest.dirty, width, height, 0, 0,
+                       width, height);
 
     while (vs != NULL) {
         vnc_colordepth(vs);
         vnc_desktop_resize(vs);
-        VNC_SET_VISIBLE_PIXELS_DIRTY(vs->dirty,
-                                     ds_get_width(ds),
-                                     ds_get_height(ds));
+        memset(vs->dirty, 0x00, sizeof(vs->dirty));
+        vnc_set_area_dirty(vs->dirty, width, height, 0, 0,
+                           width, height);
         vs = vs->next;
     }
 }
@@ -757,7 +755,7 @@ static int vnc_update_client(VncState *vs, int has_dirty)
         saved_offset = vs->output.offset;
         vnc_write_u16(vs, 0);
 
-        height = MIN(pixman_image_get_height(vd->server), vs->client_height);
+        height = pixman_image_get_height(vd->server);
 
         y = 0;
         for (;;) {
@@ -1227,7 +1225,8 @@ static void check_pointer_type_change(VncState *vs, int absolute)
         vnc_write_u8(vs, 0);
         vnc_write_u16(vs, 1);
         vnc_framebuffer_update(vs, absolute, 0,
-                               ds_get_width(vs->ds), ds_get_height(vs->ds),
+                               pixman_image_get_width(vs->vd->server),
+                               pixman_image_get_height(vs->vd->server),
                                VNC_ENCODING_POINTER_TYPE_CHANGE);
         vnc_flush(vs);
     }
@@ -1238,6 +1237,8 @@ static void pointer_event(VncState *vs, int button_mask, int x, int y)
 {
     int buttons = 0;
     int dz = 0;
+    int width = pixman_image_get_width(vs->vd->server);
+    int height = pixman_image_get_height(vs->vd->server);
 
     if (button_mask & 0x01)
         buttons |= MOUSE_EVENT_LBUTTON;
@@ -1251,10 +1252,8 @@ static void pointer_event(VncState *vs, int button_mask, int x, int y)
         dz = 1;
 
     if (vs->absolute) {
-        kbd_mouse_event(ds_get_width(vs->ds) > 1 ?
-                          x * 0x7FFF / (ds_get_width(vs->ds) - 1) : 0x4000,
-                        ds_get_height(vs->ds) > 1 ?
-                          y * 0x7FFF / (ds_get_height(vs->ds) - 1) : 0x4000,
+        kbd_mouse_event(width > 1 ? x * 0x7FFF / (width - 1) : 0x4000,
+                        height > 1 ? y * 0x7FFF / (height - 1) : 0x4000,
                         dz, buttons);
     } else if (vnc_has_feature(vs, VNC_FEATURE_POINTER_TYPE_CHANGE)) {
         x -= 0x7FFF;
@@ -1490,26 +1489,18 @@ static void ext_key_event(VncState *vs, int down,
 }
 
 static void framebuffer_update_request(VncState *vs, int incremental,
-                                       int x_position, int y_position,
-                                       int w, int h)
+                                       int x, int y, int w, int h)
 {
-    if (x_position > ds_get_width(vs->ds))
-        x_position = ds_get_width(vs->ds);
-    if (y_position > ds_get_height(vs->ds))
-        y_position = ds_get_height(vs->ds);
-    if (x_position + w >= ds_get_width(vs->ds))
-        w = ds_get_width(vs->ds)  - x_position;
-    if (y_position + h >= ds_get_height(vs->ds))
-        h = ds_get_height(vs->ds) - y_position;
+    int width = pixman_image_get_width(vs->vd->server);
+    int height = pixman_image_get_height(vs->vd->server);
 
-    int i;
     vs->need_update = 1;
-    if (!incremental) {
-        vs->force_update = 1;
-        for (i = 0; i < h; i++) {
-            bitmap_set(vs->dirty[y_position + i], x_position / 16, w / 16);
-        }
+
+    if (incremental) {
+        return;
     }
+
+    vnc_set_area_dirty(vs->dirty, width, height, x, y, w, h);
 }
 
 static void send_ext_key_event_ack(VncState *vs)
@@ -1517,7 +1508,8 @@ static void send_ext_key_event_ack(VncState *vs)
     vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
     vnc_write_u8(vs, 0);
     vnc_write_u16(vs, 1);
-    vnc_framebuffer_update(vs, 0, 0, ds_get_width(vs->ds), ds_get_height(vs->ds),
+    vnc_framebuffer_update(vs, 0, 0, pixman_image_get_width(vs->vd->server),
+                           pixman_image_get_height(vs->vd->server),
                            VNC_ENCODING_EXT_KEY_EVENT);
     vnc_flush(vs);
 }
@@ -1527,7 +1519,8 @@ static void send_ext_audio_ack(VncState *vs)
     vnc_write_u8(vs, VNC_MSG_SERVER_FRAMEBUFFER_UPDATE);
     vnc_write_u8(vs, 0);
     vnc_write_u16(vs, 1);
-    vnc_framebuffer_update(vs, 0, 0, ds_get_width(vs->ds), ds_get_height(vs->ds),
+    vnc_framebuffer_update(vs, 0, 0, pixman_image_get_width(vs->vd->server),
+                           pixman_image_get_height(vs->vd->server),
                            VNC_ENCODING_AUDIO);
     vnc_flush(vs);
 }
@@ -1674,7 +1667,8 @@ static void vnc_dpy_setdata(DisplayState *ds)
     qemu_pixman_image_unref(vd->guest.fb);
     vd->guest.fb = pixman_image_ref(ds->surface->image);
     vd->guest.format = ds->surface->format;
-    vnc_dpy_update(ds, 0, 0, ds_get_width(ds), ds_get_height(ds));
+    vnc_dpy_update(ds, 0, 0, pixman_image_get_width(vd->server),
+                   pixman_image_get_height(vd->server));
 }
 
 static void vnc_colordepth(VncState *vs)
@@ -1684,8 +1678,9 @@ static void vnc_colordepth(VncState *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, 0, 0, ds_get_width(vs->ds), 
-                               ds_get_height(vs->ds), VNC_ENCODING_WMVi);
+        vnc_framebuffer_update(vs, 0, 0, pixman_image_get_width(vs->vd->server),
+                               pixman_image_get_height(vs->vd->server),
+                               VNC_ENCODING_WMVi);
         pixel_format_message(vs);
         vnc_flush(vs);
     } else {
@@ -1848,8 +1843,8 @@ static int protocol_client_init(VncState *vs, uint8_t *data, size_t len)
     char buf[1024];
     int size;
 
-    vs->client_width = ds_get_width(vs->ds);
-    vs->client_height = ds_get_height(vs->ds);
+    vs->client_width = pixman_image_get_width(vs->vd->server);
+    vs->client_height = pixman_image_get_height(vs->vd->server);
     vnc_write_u16(vs, vs->client_width);
     vnc_write_u16(vs, vs->client_height);
 
@@ -2159,12 +2154,12 @@ static void vnc_rect_updated(VncDisplay *vd, int x, int y, struct timeval * tv)
 
 static int vnc_refresh_server_surface(VncDisplay *vd)
 {
-    int width = pixman_image_get_width(vd->guest.fb);
-    int height = pixman_image_get_height(vd->guest.fb);
-    int y;
+    int width = MIN(pixman_image_get_width(vd->guest.fb),
+                    pixman_image_get_width(vd->server));
+    int height = MIN(pixman_image_get_height(vd->guest.fb),
+                     pixman_image_get_height(vd->server));
+    int cmp_bytes, server_stride, min_stride, guest_stride, y = 0;
     uint8_t *guest_row0 = NULL, *server_row0;
-    int guest_stride = 0, server_stride;
-    int cmp_bytes;
     VncState *vs;
     int has_dirty = 0;
     pixman_image_t *tmpbuf = NULL;
@@ -2179,10 +2174,10 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
      * Check and copy modified bits from guest to server surface.
      * Update server dirty map.
      */
-    cmp_bytes = VNC_DIRTY_PIXELS_PER_BIT * VNC_SERVER_FB_BYTES;
-    if (cmp_bytes > vnc_server_fb_stride(vd)) {
-        cmp_bytes = vnc_server_fb_stride(vd);
-    }
+    server_row0 = (uint8_t *)pixman_image_get_data(vd->server);
+    server_stride = guest_stride = pixman_image_get_stride(vd->server);
+    cmp_bytes = MIN(VNC_DIRTY_PIXELS_PER_BIT * VNC_SERVER_FB_BYTES,
+                    server_stride);
     if (vd->guest.format != VNC_SERVER_FB_FORMAT) {
         int width = pixman_image_get_width(vd->server);
         tmpbuf = qemu_pixman_linebuf_create(VNC_SERVER_FB_FORMAT, width);
@@ -2190,10 +2185,8 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
         guest_row0 = (uint8_t *)pixman_image_get_data(vd->guest.fb);
         guest_stride = pixman_image_get_stride(vd->guest.fb);
     }
-    server_row0 = (uint8_t *)pixman_image_get_data(vd->server);
-    server_stride = pixman_image_get_stride(vd->server);
+    min_stride = MIN(server_stride, guest_stride);
 
-    y = 0;
     for (;;) {
         int x;
         uint8_t *guest_ptr, *server_ptr;
@@ -2219,13 +2212,17 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
 
         for (; x < DIV_ROUND_UP(width, VNC_DIRTY_PIXELS_PER_BIT);
              x++, guest_ptr += cmp_bytes, server_ptr += cmp_bytes) {
+            int _cmp_bytes = cmp_bytes;
             if (!test_and_clear_bit(x, vd->guest.dirty[y])) {
                 continue;
             }
-            if (memcmp(server_ptr, guest_ptr, cmp_bytes) == 0) {
+            if ((x + 1) * cmp_bytes > min_stride) {
+                _cmp_bytes = min_stride - x * cmp_bytes;
+            }
+            if (memcmp(server_ptr, guest_ptr, _cmp_bytes) == 0) {
                 continue;
             }
-            memcpy(server_ptr, guest_ptr, cmp_bytes);
+            memcpy(server_ptr, guest_ptr, _cmp_bytes);
             vnc_rect_updated(vd, x * VNC_DIRTY_PIXELS_PER_BIT, y, &tv);
             vs = vd->clients;
             while (vs != NULL) {
diff --git a/vnc.h b/vnc.h
index 25161fc41c..c10182beff 100644
--- a/vnc.h
+++ b/vnc.h
@@ -70,14 +70,15 @@ typedef void VncSendHextileTile(VncState *vs,
                                 void *last_fg,
                                 int *has_bg, int *has_fg);
 
-/* VNC_MAX_WIDTH must be a multiple of 16. */
-#define VNC_MAX_WIDTH 2560
-#define VNC_MAX_HEIGHT 2048
-
 /* VNC_DIRTY_PIXELS_PER_BIT is the number of dirty pixels represented
- * by one bit in the dirty bitmap */
+ * by one bit in the dirty bitmap, should be a power of 2 */
 #define VNC_DIRTY_PIXELS_PER_BIT 16
 
+/* VNC_MAX_WIDTH must be a multiple of VNC_DIRTY_PIXELS_PER_BIT. */
+
+#define VNC_MAX_WIDTH ROUND_UP(2560, VNC_DIRTY_PIXELS_PER_BIT)
+#define VNC_MAX_HEIGHT 2048
+
 /* VNC_DIRTY_BITS is the number of bits in the dirty bitmap. */
 #define VNC_DIRTY_BITS (VNC_MAX_WIDTH / VNC_DIRTY_PIXELS_PER_BIT)
 
@@ -116,7 +117,8 @@ typedef struct VncRectStat VncRectStat;
 struct VncSurface
 {
     struct timeval last_freq_check;
-    DECLARE_BITMAP(dirty[VNC_MAX_HEIGHT], VNC_MAX_WIDTH / 16);
+    DECLARE_BITMAP(dirty[VNC_MAX_HEIGHT],
+                   VNC_MAX_WIDTH / VNC_DIRTY_PIXELS_PER_BIT);
     VncRectStat stats[VNC_STAT_ROWS][VNC_STAT_COLS];
     pixman_image_t *fb;
     pixman_format_code_t format;
