From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from eggs.gnu.org ([2001:4830:134:3::10]:54117) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLYt1-00046D-MV for qemu-devel@nongnu.org; Thu, 06 Mar 2014 09:03:05 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1WLYsv-0001jz-Fg for qemu-devel@nongnu.org; Thu, 06 Mar 2014 09:02:59 -0500 Received: from mx1.redhat.com ([209.132.183.28]:6290) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1WLYsv-0001ji-7P for qemu-devel@nongnu.org; Thu, 06 Mar 2014 09:02:53 -0500 From: Gerd Hoffmann Date: Thu, 6 Mar 2014 15:02:33 +0100 Message-Id: <1394114553-11676-1-git-send-email-kraxel@redhat.com> Subject: [Qemu-devel] [PATCH] vnc: fix use-after-free in vnc_update_client_sync List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: qemu-devel@nongnu.org Cc: Gerd Hoffmann , Anthony Liguori , Markus Armbruster 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). Cc: Markus Armbruster Signed-off-by: Gerd Hoffmann --- ui/vnc.c | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/ui/vnc.c b/ui/vnc.c index 5601cc3..b0efb1f 100644 --- a/ui/vnc.c +++ b/ui/vnc.c @@ -416,8 +416,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); @@ -750,7 +749,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 */ } } @@ -873,14 +872,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; @@ -939,8 +931,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; } @@ -2749,7 +2744,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 */ } -- 1.8.3.1