Commit 38ee14f4 authored by Gerd Hoffmann's avatar Gerd Hoffmann
Browse files

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: default avatarGerd Hoffmann <kraxel@redhat.com>
Reviewed-by: default avatarMarkus Armbruster <armbru@redhat.com>
parent e3c1adf1

ui/vnc.c

+8 −13
Original line number Diff line number Diff line
@@ -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 */

    }