From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=37456 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1PuEV9-0005kp-Fg
	for qemu-devel@nongnu.org; Mon, 28 Feb 2011 20:35:48 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <wency@cn.fujitsu.com>) id 1PuEV8-0006ph-3a
	for qemu-devel@nongnu.org; Mon, 28 Feb 2011 20:35:47 -0500
Received: from [222.73.24.84] (port=59896 helo=song.cn.fujitsu.com)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <wency@cn.fujitsu.com>) id 1PuEV7-0006or-Na
	for qemu-devel@nongnu.org; Mon, 28 Feb 2011 20:35:46 -0500
Message-ID: <4D6C4D21.9060204@cn.fujitsu.com>
Date: Tue, 01 Mar 2011 09:34:25 +0800
From: Wen Congyang <wency@cn.fujitsu.com>
MIME-Version: 1.0
Subject: Re: [Qemu-devel] [PATCH] vnc: Fix heap corruption
References: <1298928892-24039-1-git-send-email-weil@mail.berlios.de>
In-Reply-To: <1298928892-24039-1-git-send-email-weil@mail.berlios.de>
Content-Transfer-Encoding: 7bit
Content-Type: text/plain; charset=ISO-8859-1
List-Id: qemu-devel.nongnu.org
List-Unsubscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=unsubscribe>
List-Archive: <http://lists.nongnu.org/archive/html/qemu-devel>
List-Post: <mailto:qemu-devel@nongnu.org>
List-Help: <mailto:qemu-devel-request@nongnu.org?subject=help>
List-Subscribe: <http://lists.nongnu.org/mailman/listinfo/qemu-devel>,
	<mailto:qemu-devel-request@nongnu.org?subject=subscribe>
To: Stefan Weil <weil@mail.berlios.de>
Cc: Corentin Chary <corentincj@iksaif.net>, Anthony Liguori <aliguori@us.ibm.com>, qemu-devel@nongnu.org

At 03/01/2011 05:34 AM, Stefan Weil Write:
> Commit bc2429b9174ac2d3c56b7fd35884b0d89ec7fb02 introduced
> a severe bug (heap corruption).
> 
> bitmap_clear was called with a wrong argument
> which caused out-of-bound writes to 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!
> 
> Cc: Corentin Chary <corentincj@iksaif.net>
> Cc: Anthony Liguori <aliguori@us.ibm.com>
> Signed-off-by: Stefan Weil <weil@mail.berlios.de>
> ---
>  ui/vnc.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
> 
> diff --git a/ui/vnc.c b/ui/vnc.c
> index af55156..89f71da 100644
> --- a/ui/vnc.c
> +++ b/ui/vnc.c
> @@ -2401,7 +2401,7 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
>       */
>      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);
> +                 (VNC_MAX_WIDTH - ds_get_width(vd->ds)) / 16);

The third argument of bitmap_clear() is number of bits to be cleared, but we pass
the end bits to be cleared to bitmap_clear().

I think we can fix this bug like this(I can not reproduce this bug, so I do not
know whether it can fix this bug):

diff --git a/ui/vnc.c b/ui/vnc.c
index fff34af..6d54661 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -2404,7 +2404,7 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
      */
     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);
+                 VNC_DIRTY_WORDS * BITS_PER_LONG - (ds_get_width(vd->ds) / 16));
     cmp_bytes = 16 * ds_get_bytes_per_pixel(vd->ds);
     guest_row  = vd->guest.ds->data;
     server_row = vd->server->data;

Thanks
Wen Congyang

>      cmp_bytes = 16 * ds_get_bytes_per_pixel(vd->ds);
>      guest_row  = vd->guest.ds->data;
>      server_row = vd->server->data;