summary refs log tree commit diff stats
path: root/ui/vnc.c
diff options
context:
space:
mode:
authorLi Qiang <liq3ea@163.com>2019-08-31 08:39:22 -0700
committerGerd Hoffmann <kraxel@redhat.com>2019-09-17 13:45:10 +0200
commit6bf21f3d83e95bcc4ba35a7a07cc6655e8b010b0 (patch)
tree659fa371214fe7d4b736d3940004ac4aeff1a5c7 /ui/vnc.c
parent6105683da35babad9ae168a72d1e89e63e9d6974 (diff)
downloadfocaccia-qemu-6bf21f3d83e95bcc4ba35a7a07cc6655e8b010b0.tar.gz
focaccia-qemu-6bf21f3d83e95bcc4ba35a7a07cc6655e8b010b0.zip
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 <fangying1@huawei.com>
Signed-off-by: Li Qiang <liq3ea@163.com>
Message-id: 20190831153922.121308-1-liq3ea@163.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Diffstat (limited to 'ui/vnc.c')
-rw-r--r--ui/vnc.c28
1 files changed, 16 insertions, 12 deletions
diff --git a/ui/vnc.c b/ui/vnc.c
index bc43c4ca16..87b8045afe 100644
--- a/ui/vnc.c
+++ b/ui/vnc.c
@@ -1307,6 +1307,8 @@ void vnc_disconnect_finish(VncState *vs)
     object_unref(OBJECT(vs->sioc));
     vs->sioc = NULL;
     vs->magic = 0;
+    g_free(vs->zrle);
+    g_free(vs->tight);
     g_free(vs);
 }
 
@@ -2058,8 +2060,8 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
 
     vs->features = 0;
     vs->vnc_encoding = 0;
-    vs->tight.compression = 9;
-    vs->tight.quality = -1; /* Lossless by default */
+    vs->tight->compression = 9;
+    vs->tight->quality = -1; /* Lossless by default */
     vs->absolute = -1;
 
     /*
@@ -2127,11 +2129,11 @@ static void set_encodings(VncState *vs, int32_t *encodings, size_t n_encodings)
             vs->features |= VNC_FEATURE_LED_STATE_MASK;
             break;
         case VNC_ENCODING_COMPRESSLEVEL0 ... VNC_ENCODING_COMPRESSLEVEL0 + 9:
-            vs->tight.compression = (enc & 0x0F);
+            vs->tight->compression = (enc & 0x0F);
             break;
         case VNC_ENCODING_QUALITYLEVEL0 ... VNC_ENCODING_QUALITYLEVEL0 + 9:
             if (vs->vd->lossy) {
-                vs->tight.quality = (enc & 0x0F);
+                vs->tight->quality = (enc & 0x0F);
             }
             break;
         default:
@@ -3034,6 +3036,8 @@ static void vnc_connect(VncDisplay *vd, QIOChannelSocket *sioc,
     int i;
 
     trace_vnc_client_connect(vs, sioc);
+    vs->zrle = g_new0(VncZrle, 1);
+    vs->tight = g_new0(VncTight, 1);
     vs->magic = VNC_MAGIC;
     vs->sioc = sioc;
     object_ref(OBJECT(vs->sioc));
@@ -3045,19 +3049,19 @@ static void vnc_connect(VncDisplay *vd, QIOChannelSocket *sioc,
     buffer_init(&vs->output,         "vnc-output/%p", sioc);
     buffer_init(&vs->jobs_buffer,    "vnc-jobs_buffer/%p", sioc);
 
-    buffer_init(&vs->tight.tight,    "vnc-tight/%p", sioc);
-    buffer_init(&vs->tight.zlib,     "vnc-tight-zlib/%p", sioc);
-    buffer_init(&vs->tight.gradient, "vnc-tight-gradient/%p", sioc);
+    buffer_init(&vs->tight->tight,    "vnc-tight/%p", sioc);
+    buffer_init(&vs->tight->zlib,     "vnc-tight-zlib/%p", sioc);
+    buffer_init(&vs->tight->gradient, "vnc-tight-gradient/%p", sioc);
 #ifdef CONFIG_VNC_JPEG
-    buffer_init(&vs->tight.jpeg,     "vnc-tight-jpeg/%p", sioc);
+    buffer_init(&vs->tight->jpeg,     "vnc-tight-jpeg/%p", sioc);
 #endif
 #ifdef CONFIG_VNC_PNG
-    buffer_init(&vs->tight.png,      "vnc-tight-png/%p", sioc);
+    buffer_init(&vs->tight->png,      "vnc-tight-png/%p", sioc);
 #endif
     buffer_init(&vs->zlib.zlib,      "vnc-zlib/%p", sioc);
-    buffer_init(&vs->zrle.zrle,      "vnc-zrle/%p", sioc);
-    buffer_init(&vs->zrle.fb,        "vnc-zrle-fb/%p", sioc);
-    buffer_init(&vs->zrle.zlib,      "vnc-zrle-zlib/%p", sioc);
+    buffer_init(&vs->zrle->zrle,      "vnc-zrle/%p", sioc);
+    buffer_init(&vs->zrle->fb,        "vnc-zrle-fb/%p", sioc);
+    buffer_init(&vs->zrle->zlib,      "vnc-zrle-zlib/%p", sioc);
 
     if (skipauth) {
         vs->auth = VNC_AUTH_NONE;