From mboxrd@z Thu Jan  1 00:00:00 1970
Received: from [140.186.70.92] (port=33887 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1Puqv3-0003Ma-Iq
	for qemu-devel@nongnu.org; Wed, 02 Mar 2011 13:37:06 -0500
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71)
	(envelope-from <weil@mail.berlios.de>) id 1Puqv1-0001CI-Tp
	for qemu-devel@nongnu.org; Wed, 02 Mar 2011 13:37:05 -0500
Received: from moutng.kundenserver.de ([212.227.126.171]:63102)
	by eggs.gnu.org with esmtp (Exim 4.71)
	(envelope-from <weil@mail.berlios.de>) id 1Puqv1-0001Bb-IF
	for qemu-devel@nongnu.org; Wed, 02 Mar 2011 13:37:03 -0500
Message-ID: <4D6E8E3A.50106@mail.berlios.de>
Date: Wed, 02 Mar 2011 19:36:42 +0100
From: Stefan Weil <weil@mail.berlios.de>
MIME-Version: 1.0
References: <4D6DBDA4.3050909@cn.fujitsu.com>	<4D6DC06B.6070308@cn.fujitsu.com>
	<AANLkTi=sjn2vHo7B-1yii4ofX5cseLei0u-BbWugdwt6@mail.gmail.com>
In-Reply-To: <AANLkTi=sjn2vHo7B-1yii4ofX5cseLei0u-BbWugdwt6@mail.gmail.com>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 7bit
Subject: [Qemu-devel] Re: [PATCH RESEND 2/2] vnc: Fix heap corruption
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: Corentin Chary <corentin.chary@gmail.com>
Cc: Anthony Liguori <aliguori@us.ibm.com>, qemu-devel <qemu-devel@nongnu.org>

Am 02.03.2011 11:57, schrieb Corentin Chary:
> On Wed, Mar 2, 2011 at 3:58 AM, Wen Congyang <wency@cn.fujitsu.com> wrote:
>> This bug is reported by Stefan Weil:
>> ========
>> 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!
>> ========
>>
>> 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().
>>
>> Signed-off-by: Wen Congyang <wency@cn.fujitsu.com>
>> Reported-by: Stefan Weil <weil@mail.berlios.de>
>
> Acked-by: Corentin Chary <corentin.chary@gmail.com>
>

No. I dont't think that the third parameter of bitmap_clear is
ok like that. See my patch for the correct value.

My own patch is also incomplete, so I'll send an update.

Stefan


>> ---
>>   ui/vnc.c |    6 +++---
>>   1 files changed, 3 insertions(+), 3 deletions(-)
>>
>> diff --git a/ui/vnc.c b/ui/vnc.c
>> index e3761b0..e7d0b5b 100644
>> --- a/ui/vnc.c
>> +++ b/ui/vnc.c
>> @@ -2390,6 +2390,7 @@ static int vnc_refresh_server_surface(VncDisplay *vd)
>>      unsigned long width_mask[VNC_DIRTY_WORDS];
>>      VncState *vs;
>>      int has_dirty = 0;
>> +    const size_t width = ds_get_width(vd->ds) / 16;
>>
>>      struct timeval tv = { 0, 0 };
>>
>> @@ -2403,9 +2404,8 @@ 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);
>> +    bitmap_set(width_mask, 0, width);
>> +    bitmap_clear(width_mask, width, VNC_DIRTY_WORDS * BITS_PER_LONG - width);
>>      cmp_bytes = 16 * ds_get_bytes_per_pixel(vd->ds);
>>      guest_row  = vd->guest.ds->data;
>>      server_row = vd->server->data;
>> --
>> 1.7.1
>>
>>
>
>
>