[Qemu-devel] Re: [PATCH RESEND 2/2] vnc: Fix heap corruption

[Stefan Weil <weil@mail.berlios.de>]

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
>>
>>
>
>
>