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 <corentincj@iksaif.net> Cc: Wen Congyang <wency@cn.fujitsu.com> Cc: Gerhard Wiesinger <lists@wiesinger.com> Cc: Anthony Liguori <aliguori@us.ibm.com> Signed-off-by: Stefan Weil <weil@mail.berlios.de> Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>
This commit is contained in:
		
							parent
							
								
									2ea720dba5
								
							
						
					
					
						commit
						23bfe28fff
					
				
							
								
								
									
										6
									
								
								ui/vnc.c
									
									
									
									
									
								
							
							
						
						
									
										6
									
								
								ui/vnc.c
									
									
									
									
									
								
							@ -2383,7 +2383,6 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
 | 
				
			|||||||
    uint8_t *guest_row;
 | 
					    uint8_t *guest_row;
 | 
				
			||||||
    uint8_t *server_row;
 | 
					    uint8_t *server_row;
 | 
				
			||||||
    int cmp_bytes;
 | 
					    int cmp_bytes;
 | 
				
			||||||
    unsigned long width_mask[VNC_DIRTY_WORDS];
 | 
					 | 
				
			||||||
    VncState *vs;
 | 
					    VncState *vs;
 | 
				
			||||||
    int has_dirty = 0;
 | 
					    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.
 | 
					     * Check and copy modified bits from guest to server surface.
 | 
				
			||||||
     * Update server dirty map.
 | 
					     * 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);
 | 
					    cmp_bytes = 16 * ds_get_bytes_per_pixel(vd->ds);
 | 
				
			||||||
    guest_row  = vd->guest.ds->data;
 | 
					    guest_row  = vd->guest.ds->data;
 | 
				
			||||||
    server_row = vd->server->data;
 | 
					    server_row = vd->server->data;
 | 
				
			||||||
    for (y = 0; y < vd->guest.ds->height; y++) {
 | 
					    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;
 | 
					            int x;
 | 
				
			||||||
            uint8_t *guest_ptr;
 | 
					            uint8_t *guest_ptr;
 | 
				
			||||||
            uint8_t *server_ptr;
 | 
					            uint8_t *server_ptr;
 | 
				
			||||||
 | 
				
			|||||||
							
								
								
									
										9
									
								
								ui/vnc.h
									
									
									
									
									
								
							
							
						
						
									
										9
									
								
								ui/vnc.h
									
									
									
									
									
								
							@ -79,9 +79,12 @@ typedef void VncSendHextileTile(VncState *vs,
 | 
				
			|||||||
                                void *last_fg,
 | 
					                                void *last_fg,
 | 
				
			||||||
                                int *has_bg, int *has_fg);
 | 
					                                int *has_bg, int *has_fg);
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
					/* VNC_MAX_WIDTH must be a multiple of 16. */
 | 
				
			||||||
#define VNC_MAX_WIDTH 2560
 | 
					#define VNC_MAX_WIDTH 2560
 | 
				
			||||||
#define VNC_MAX_HEIGHT 2048
 | 
					#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_RECT  64
 | 
				
			||||||
#define VNC_STAT_COLS (VNC_MAX_WIDTH / VNC_STAT_RECT)
 | 
					#define VNC_STAT_COLS (VNC_MAX_WIDTH / VNC_STAT_RECT)
 | 
				
			||||||
@ -114,7 +117,7 @@ typedef struct VncRectStat VncRectStat;
 | 
				
			|||||||
struct VncSurface
 | 
					struct VncSurface
 | 
				
			||||||
{
 | 
					{
 | 
				
			||||||
    struct timeval last_freq_check;
 | 
					    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];
 | 
					    VncRectStat stats[VNC_STAT_ROWS][VNC_STAT_COLS];
 | 
				
			||||||
    DisplaySurface *ds;
 | 
					    DisplaySurface *ds;
 | 
				
			||||||
};
 | 
					};
 | 
				
			||||||
@ -234,7 +237,7 @@ struct VncState
 | 
				
			|||||||
    int csock;
 | 
					    int csock;
 | 
				
			||||||
 | 
					
 | 
				
			||||||
    DisplayState *ds;
 | 
					    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
 | 
					    uint8_t **lossy_rect; /* Not an Array to avoid costly memcpy in
 | 
				
			||||||
                           * vnc-jobs-async.c */
 | 
					                           * vnc-jobs-async.c */
 | 
				
			||||||
 | 
					
 | 
				
			||||||
 | 
				
			|||||||
		Loading…
	
	
			
			x
			
			
		
	
		Reference in New Issue
	
	Block a user