Re: [Qemu-devel] Discussion: vnc: memory leak in zrle_compress_data

[Li Qiang]

fangying <address@hidden> 于2019年9月1日周日 下午8:29写道：

> Nice work, your patch does fix this issue in my test.
>
> I think we should make VncState.zlib to be a pointer type as well.
>
> Since we are going to use pointers instead of copy, we must make sure that
> there’s no race condition of pointer members between the local vs (vnc
> worker thread) and origin vs (main thread).
>
>

Yes, there is a race between the main thread and vnc thread. Maybe we
should add a lock just like the 'vs->output_mutex'.
Let's wait for Gerd's (or others) comments whether there is a better way
than using pointer before sending the revision.

Thanks,
Li Qiang


>
> Thanks.
> Ying Fang
> *发件人： *Li Qiang<address@hidden>
> *收件人： *fangying<address@hidden>
> *抄送： *Gerd Hoffmann<address@hidden>;qemu-devel<address@hidden>;Daniel
> P. Berrange<address@hidden>;zhouyibo<address@hidden>
> *主题： *Re: [Qemu-devel] Discussion: vnc: memory leak in zrle_compress_data
> *时间： *2019-08-31 23:48:10
>
>
>
> fangying <address@hidden> 于2019年8月31日周六 上午8:45写道：
>
>> Hi Gerd,
>>
>> Memory leak is observed in zrle_compress_data when we are doing some
>> AddressSanitizer tests. The leak stack is as bellow:
>>
>> =================================================================
>> ==47887==ERROR: LeakSanitizer: detected memory leaks
>>
>> Direct leak of 29760 byte(s) in 5 object(s) allocated from:
>>      #0 0xffffa67ef3c3 in __interceptor_calloc
>> (/lib64/libasan.so.4+0xd33c3)
>>      #1 0xffffa65071cb in g_malloc0 (/lib64/libglib-2.0.so.0+0x571cb)
>>      #2 0xffffa5e968f7 in deflateInit2_ (/lib64/libz.so.1+0x78f7)
>>      #3 0xaaaacec58613 in zrle_compress_data ui/vnc-enc-zrle.c:87
>>      #4 0xaaaacec58613 in zrle_send_framebuffer_update
>> ui/vnc-enc-zrle.c:344
>>      #5 0xaaaacec34e77 in vnc_send_framebuffer_update ui/vnc.c:919
>>      #6 0xaaaacec5e023 in vnc_worker_thread_loop ui/vnc-jobs.c:271
>>      #7 0xaaaacec5e5e7 in vnc_worker_thread ui/vnc-jobs.c:340
>>      #8 0xaaaacee4d3c3 in qemu_thread_start util/qemu-thread-posix.c:502
>>      #9 0xffffa544e8bb in start_thread (/lib64/libpthread.so.0+0x78bb)
>>      #10 0xffffa53965cb in thread_start (/lib64/libc.so.6+0xd55cb)
>>
>> This leak stack can be easily reproduced in the case that a client
>> repeatedly
>> does vnc connect/disconnect .
>>
>> To get the leak stack, we can compile qemu with
>> --extra-ldflags=-Wl,--build-id -Wl,-z,relro -Wl,-z,now -lasan'
>> '--extra-cflags=-O0 -g -fno-omit-frame-pointer -fno-stack-protector
>> -fsanitize=address -fsanitize=leak' using gcc that supports asan.
>>
>> It is obvious that there may be memory leak in the zlib/zrle compress
>> stuff.
>> IIUC, *deflateInit2* is called with the local VncState vs->zrle.stream
>> when a
>> client is connected the vnc server. And then *deflate* is called to do
>> the
>> encoding. Finally *deflateEnd* is called in vnc_zrle_clear when a
>> connection is
>> closed.
>>
>> I had not think it out why this memory leak could happen here.
>> It is noticed that deflateInit2 is called with the local vs,
>> however deflateEnd is called with the origin vs.
>> The local vs is copied to the origin vs in vnc_async_encoding_start and
>> vnc_async_encoding_end on the contrary.
>>
>> Have you got any idea on this issue ?
>>
>>
> Hello Ying,
>
> I have posted a patch for this issue:
> --> https://lists.gnu.org/archive/html/qemu-devel/2019-08/msg06675.html
>
> Please check whether the patch can address this issue.
>
> Thanks,
> Li Qiang
>
>
>
>
>
>> Thanks.
>> Ying Fang
>>
>>
>>