From d72c58c1de8c6733f159d6e56c26226e53c2d4ea Mon Sep 17 00:00:00 2001 From: Stefan Weil Date: Thu, 3 Mar 2011 21:37:55 +0100 Subject: [PATCH] vnc: Fix stack corruption and other bitmap related bugs Commit bc2429b9174ac2d3c56b7fd35884b0d89ec7fb02 introduced a severe bug (stack corruption). bitmap_clear was called with a wrong argument which caused out-of-bound writes to the local variable width_mask. This bug was detected with QEMU running on windows. It also occurs with wine: *** stack smashing detected ***: terminated wine: Unhandled illegal instruction at address 0x6115c7 (thread 0009), starting debugger... The bug is not windows specific! Instead of fixing the wrong parameter value, bitmap_clear(), bitmap_set and width_mask were removed, and bitmap_intersect() was replaced by !bitmap_empty(). The new operation is much shorter and equivalent to the old operations. The declarations of the dirty bitmaps in vnc.h were also wrong for 64 bit hosts because of a rounding effect: for these hosts, VNC_MAX_WIDTH is no longer a multiple of (16 * BITS_PER_LONG), so the rounded value of VNC_DIRTY_WORDS was too small. Fix both declarations by using the macro which is designed for this purpose. Cc: Corentin Chary Cc: Wen Congyang Cc: Gerhard Wiesinger Cc: Anthony Liguori Signed-off-by: Stefan Weil Signed-off-by: Anthony Liguori --- ui/vnc.c | 6 +----- ui/vnc.h | 9 ++++++--- 2 files changed, 7 insertions(+), 8 deletions(-) diff --git a/ui/vnc.c b/ui/vnc.c index 610f884..34dc0cd 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -2383,7 +2383,6 @@ static int vnc_refresh_server_surface(VncDisplay *vd) uint8_t *guest_row; uint8_t *server_row; int cmp_bytes; - unsigned long width_mask[VNC_DIRTY_WORDS]; VncState *vs; int has_dirty = 0; @@ -2399,14 +2398,11 @@ static int vnc_refresh_server_surface(VncDisplay *vd) * Check and copy modified bits from guest to server surface. * Update server dirty map. */ - bitmap_set(width_mask, 0, (ds_get_width(vd->ds) / 16)); - bitmap_clear(width_mask, (ds_get_width(vd->ds) / 16), - VNC_DIRTY_WORDS * BITS_PER_LONG); cmp_bytes = 16 * ds_get_bytes_per_pixel(vd->ds); guest_row = vd->guest.ds->data; server_row = vd->server->data; for (y = 0; y < vd->guest.ds->height; y++) { - if (bitmap_intersects(vd->guest.dirty[y], width_mask, VNC_DIRTY_WORDS)) { + if (!bitmap_empty(vd->guest.dirty[y], VNC_DIRTY_BITS)) { int x; uint8_t *guest_ptr; uint8_t *server_ptr; diff --git a/ui/vnc.h b/ui/vnc.h index 8a1e7b9..f10c5dc 100644 --- a/ui/vnc.h +++ b/ui/vnc.h @@ -79,9 +79,12 @@ 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 -#define VNC_DIRTY_WORDS (VNC_MAX_WIDTH / (16 * BITS_PER_LONG)) + +/* VNC_DIRTY_BITS is the number of bits in the dirty bitmap. */ +#define VNC_DIRTY_BITS (VNC_MAX_WIDTH / 16) #define VNC_STAT_RECT 64 #define VNC_STAT_COLS (VNC_MAX_WIDTH / VNC_STAT_RECT) @@ -114,7 +117,7 @@ typedef struct VncRectStat VncRectStat; struct VncSurface { struct timeval last_freq_check; - unsigned long dirty[VNC_MAX_HEIGHT][VNC_DIRTY_WORDS]; + DECLARE_BITMAP(dirty[VNC_MAX_HEIGHT], VNC_MAX_WIDTH / 16); VncRectStat stats[VNC_STAT_ROWS][VNC_STAT_COLS]; DisplaySurface *ds; }; @@ -234,7 +237,7 @@ struct VncState int csock; DisplayState *ds; - unsigned long dirty[VNC_MAX_HEIGHT][VNC_DIRTY_WORDS]; + DECLARE_BITMAP(dirty[VNC_MAX_HEIGHT], VNC_DIRTY_BITS); uint8_t **lossy_rect; /* Not an Array to avoid costly memcpy in * vnc-jobs-async.c */ -- 2.7.4