From 6bf21f3d83e95bcc4ba35a7a07cc6655e8b010b0 Mon Sep 17 00:00:00 2001 From: Li Qiang Date: Sat, 31 Aug 2019 08:39:22 -0700 Subject: vnc: fix memory leak when vnc disconnect Currently when qemu receives a vnc connect, it creates a 'VncState' to represent this connection. In 'vnc_worker_thread_loop' it creates a local 'VncState'. The connection 'VcnState' and local 'VncState' exchange data in 'vnc_async_encoding_start' and 'vnc_async_encoding_end'. In 'zrle_compress_data' it calls 'deflateInit2' to allocate the libz library opaque data. The 'VncState' used in 'zrle_compress_data' is the local 'VncState'. In 'vnc_zrle_clear' it calls 'deflateEnd' to free the libz library opaque data. The 'VncState' used in 'vnc_zrle_clear' is the connection 'VncState'. In currently implementation there will be a memory leak when the vnc disconnect. Following is the asan output backtrack: 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 is because the opaque allocated in 'deflateInit2' is not freed in 'deflateEnd'. The reason is that the 'deflateEnd' calls 'deflateStateCheck' and in the latter will check whether 's->strm != strm'(libz's data structure). This check will be true so in 'deflateEnd' it just return 'Z_STREAM_ERROR' and not free the data allocated in 'deflateInit2'. The reason this happens is that the 'VncState' contains the whole 'VncZrle', so when calling 'deflateInit2', the 's->strm' will be the local address. So 's->strm != strm' will be true. To fix this issue, we need to make 'zrle' of 'VncState' to be a pointer. Then the connection 'VncState' and local 'VncState' exchange mechanism will work as expection. The 'tight' of 'VncState' has the same issue, let's also turn it to a pointer. Reported-by: Ying Fang Signed-off-by: Li Qiang Message-id: 20190831153922.121308-1-liq3ea@163.com Signed-off-by: Gerd Hoffmann --- ui/vnc-enc-zrle.c | 68 +++++++++++++++++++++++++++---------------------------- 1 file changed, 34 insertions(+), 34 deletions(-) (limited to 'ui/vnc-enc-zrle.c') diff --git a/ui/vnc-enc-zrle.c b/ui/vnc-enc-zrle.c index 7493a84723..17fd28a2e2 100644 --- a/ui/vnc-enc-zrle.c +++ b/ui/vnc-enc-zrle.c @@ -37,18 +37,18 @@ static const int bits_per_packed_pixel[] = { static void vnc_zrle_start(VncState *vs) { - buffer_reset(&vs->zrle.zrle); + buffer_reset(&vs->zrle->zrle); /* make the output buffer be the zlib buffer, so we can compress it later */ - vs->zrle.tmp = vs->output; - vs->output = vs->zrle.zrle; + vs->zrle->tmp = vs->output; + vs->output = vs->zrle->zrle; } static void vnc_zrle_stop(VncState *vs) { /* switch back to normal output/zlib buffers */ - vs->zrle.zrle = vs->output; - vs->output = vs->zrle.tmp; + vs->zrle->zrle = vs->output; + vs->output = vs->zrle->tmp; } static void *zrle_convert_fb(VncState *vs, int x, int y, int w, int h, @@ -56,24 +56,24 @@ static void *zrle_convert_fb(VncState *vs, int x, int y, int w, int h, { Buffer tmp; - buffer_reset(&vs->zrle.fb); - buffer_reserve(&vs->zrle.fb, w * h * bpp + bpp); + buffer_reset(&vs->zrle->fb); + buffer_reserve(&vs->zrle->fb, w * h * bpp + bpp); tmp = vs->output; - vs->output = vs->zrle.fb; + vs->output = vs->zrle->fb; vnc_raw_send_framebuffer_update(vs, x, y, w, h); - vs->zrle.fb = vs->output; + vs->zrle->fb = vs->output; vs->output = tmp; - return vs->zrle.fb.buffer; + return vs->zrle->fb.buffer; } static int zrle_compress_data(VncState *vs, int level) { - z_streamp zstream = &vs->zrle.stream; + z_streamp zstream = &vs->zrle->stream; - buffer_reset(&vs->zrle.zlib); + buffer_reset(&vs->zrle->zlib); if (zstream->opaque != vs) { int err; @@ -93,13 +93,13 @@ static int zrle_compress_data(VncState *vs, int level) } /* reserve memory in output buffer */ - buffer_reserve(&vs->zrle.zlib, vs->zrle.zrle.offset + 64); + buffer_reserve(&vs->zrle->zlib, vs->zrle->zrle.offset + 64); /* set pointers */ - zstream->next_in = vs->zrle.zrle.buffer; - zstream->avail_in = vs->zrle.zrle.offset; - zstream->next_out = vs->zrle.zlib.buffer + vs->zrle.zlib.offset; - zstream->avail_out = vs->zrle.zlib.capacity - vs->zrle.zlib.offset; + zstream->next_in = vs->zrle->zrle.buffer; + zstream->avail_in = vs->zrle->zrle.offset; + zstream->next_out = vs->zrle->zlib.buffer + vs->zrle->zlib.offset; + zstream->avail_out = vs->zrle->zlib.capacity - vs->zrle->zlib.offset; zstream->data_type = Z_BINARY; /* start encoding */ @@ -108,8 +108,8 @@ static int zrle_compress_data(VncState *vs, int level) return -1; } - vs->zrle.zlib.offset = vs->zrle.zlib.capacity - zstream->avail_out; - return vs->zrle.zlib.offset; + vs->zrle->zlib.offset = vs->zrle->zlib.capacity - zstream->avail_out; + return vs->zrle->zlib.offset; } /* Try to work out whether to use RLE and/or a palette. We do this by @@ -259,14 +259,14 @@ static int zrle_send_framebuffer_update(VncState *vs, int x, int y, size_t bytes; int zywrle_level; - if (vs->zrle.type == VNC_ENCODING_ZYWRLE) { - if (!vs->vd->lossy || vs->tight.quality == (uint8_t)-1 - || vs->tight.quality == 9) { + if (vs->zrle->type == VNC_ENCODING_ZYWRLE) { + if (!vs->vd->lossy || vs->tight->quality == (uint8_t)-1 + || vs->tight->quality == 9) { zywrle_level = 0; - vs->zrle.type = VNC_ENCODING_ZRLE; - } else if (vs->tight.quality < 3) { + vs->zrle->type = VNC_ENCODING_ZRLE; + } else if (vs->tight->quality < 3) { zywrle_level = 3; - } else if (vs->tight.quality < 6) { + } else if (vs->tight->quality < 6) { zywrle_level = 2; } else { zywrle_level = 1; @@ -337,30 +337,30 @@ static int zrle_send_framebuffer_update(VncState *vs, int x, int y, vnc_zrle_stop(vs); bytes = zrle_compress_data(vs, Z_DEFAULT_COMPRESSION); - vnc_framebuffer_update(vs, x, y, w, h, vs->zrle.type); + vnc_framebuffer_update(vs, x, y, w, h, vs->zrle->type); vnc_write_u32(vs, bytes); - vnc_write(vs, vs->zrle.zlib.buffer, vs->zrle.zlib.offset); + vnc_write(vs, vs->zrle->zlib.buffer, vs->zrle->zlib.offset); return 1; } int vnc_zrle_send_framebuffer_update(VncState *vs, int x, int y, int w, int h) { - vs->zrle.type = VNC_ENCODING_ZRLE; + vs->zrle->type = VNC_ENCODING_ZRLE; return zrle_send_framebuffer_update(vs, x, y, w, h); } int vnc_zywrle_send_framebuffer_update(VncState *vs, int x, int y, int w, int h) { - vs->zrle.type = VNC_ENCODING_ZYWRLE; + vs->zrle->type = VNC_ENCODING_ZYWRLE; return zrle_send_framebuffer_update(vs, x, y, w, h); } void vnc_zrle_clear(VncState *vs) { - if (vs->zrle.stream.opaque) { - deflateEnd(&vs->zrle.stream); + if (vs->zrle->stream.opaque) { + deflateEnd(&vs->zrle->stream); } - buffer_free(&vs->zrle.zrle); - buffer_free(&vs->zrle.fb); - buffer_free(&vs->zrle.zlib); + buffer_free(&vs->zrle->zrle); + buffer_free(&vs->zrle->fb); + buffer_free(&vs->zrle->zlib); } -- cgit 1.4.1