From 38ee14f4f33f8836fc0e209ca59c6ae8c6edf380 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Thu, 6 Mar 2014 13:54:28 +0100 Subject: [PATCH] vnc: fix use-after-free in vnc_update_client_sync Spotted by Coverity: 876 static int vnc_update_client_sync(VncState *vs, int has_dirty) 877 { (1) Event freed_arg: "vnc_update_client(VncState *, int)" frees "vs". [details] Also see events: [deref_arg] 878 int ret = vnc_update_client(vs, has_dirty); (2) Event deref_arg: Calling "vnc_jobs_join(VncState *)" dereferences freed pointer "vs". [details] Also see events: [freed_arg] 879 vnc_jobs_join(vs); 880 return ret; 881 } Remove vnc_update_client_sync wrapper, replace it with an additional argument to vnc_update_client, so we can so the sync properly in vnc_update_client (i.e. skip it in case of a client disconnect). Signed-off-by: Gerd Hoffmann Reviewed-by: Markus Armbruster --- ui/vnc.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/ui/vnc.c b/ui/vnc.c index 7dfc94a358..b053d0e261 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -417,8 +417,7 @@ out_error: 3) resolutions > 1024 */ -static int vnc_update_client(VncState *vs, int has_dirty); -static int vnc_update_client_sync(VncState *vs, int has_dirty); +static int vnc_update_client(VncState *vs, int has_dirty, bool sync); static void vnc_disconnect_start(VncState *vs); static void vnc_colordepth(VncState *vs); @@ -751,7 +750,7 @@ static void vnc_dpy_copy(DisplayChangeListener *dcl, QTAILQ_FOREACH_SAFE(vs, &vd->clients, next, vn) { if (vnc_has_feature(vs, VNC_FEATURE_COPYRECT)) { vs->force_update = 1; - vnc_update_client_sync(vs, 1); + vnc_update_client(vs, 1, true); /* vs might be free()ed here */ } } @@ -874,14 +873,7 @@ static int find_and_clear_dirty_height(struct VncState *vs, return h; } -static int vnc_update_client_sync(VncState *vs, int has_dirty) -{ - int ret = vnc_update_client(vs, has_dirty); - vnc_jobs_join(vs); - return ret; -} - -static int vnc_update_client(VncState *vs, int has_dirty) +static int vnc_update_client(VncState *vs, int has_dirty, bool sync) { if (vs->need_update && vs->csock != -1) { VncDisplay *vd = vs->vd; @@ -940,8 +932,11 @@ static int vnc_update_client(VncState *vs, int has_dirty) return n; } - if (vs->csock == -1) + if (vs->csock == -1) { vnc_disconnect_finish(vs); + } else if (sync) { + vnc_jobs_join(vs); + } return 0; } @@ -2734,7 +2729,7 @@ static void vnc_refresh(DisplayChangeListener *dcl) vnc_unlock_display(vd); QTAILQ_FOREACH_SAFE(vs, &vd->clients, next, vn) { - rects += vnc_update_client(vs, has_dirty); + rects += vnc_update_client(vs, has_dirty, false); /* vs might be free()ed here */ }