| Message ID | alpine.DEB.2.00.1001251210570.7512@kaball-desktop |
|---|---|
| State | New |
| Headers | show |
On 01/25/2010 06:54 AM, Stefano Stabellini wrote: > Hi all, > this patch fixes another bug in vnc_refresh: calling vnc_update_client > might cause vs to be free()ed, in this case we cannot access vs->next > right after to examine the next item on the list. > > Signed-off-by: Stefano Stabellini<stefano.stabellini@eu.citrix.com> > Applied. Thanks. Regards, Anthony Liguori > --- > > diff --git a/vnc.c b/vnc.c > index cc2a26e..92facde 100644 > --- a/vnc.c > +++ b/vnc.c > @@ -2345,7 +2345,7 @@ static int vnc_refresh_server_surface(VncDisplay *vd) > static void vnc_refresh(void *opaque) > { > VncDisplay *vd = opaque; > - VncState *vs = NULL; > + VncState *vs = NULL, *vn = NULL; > int has_dirty = 0, rects = 0; > > vga_hw_update(); > @@ -2354,8 +2354,10 @@ static void vnc_refresh(void *opaque) > > vs = vd->clients; > while (vs != NULL) { > + vn = vs->next; > rects += vnc_update_client(vs, has_dirty); > - vs = vs->next; > + /* vs might be free()ed here */ > + vs = vn; > } > /* vd->timer could be NULL now if the last client disconnected, > * in this case don't update the timer */ > > > >
On 01/27/10 01:07, Anthony Liguori wrote: > On 01/25/2010 06:54 AM, Stefano Stabellini wrote: >> Hi all, >> this patch fixes another bug in vnc_refresh: calling vnc_update_client >> might cause vs to be free()ed, in this case we cannot access vs->next >> right after to examine the next item on the list. >> >> Signed-off-by: Stefano Stabellini<stefano.stabellini@eu.citrix.com> > > Applied. Thanks. IMHO this should go to stable too. cheers, Gerd
On Wed, 27 Jan 2010, Gerd Hoffmann wrote: > On 01/27/10 01:07, Anthony Liguori wrote: > > On 01/25/2010 06:54 AM, Stefano Stabellini wrote: > >> Hi all, > >> this patch fixes another bug in vnc_refresh: calling vnc_update_client > >> might cause vs to be free()ed, in this case we cannot access vs->next > >> right after to examine the next item on the list. > >> > >> Signed-off-by: Stefano Stabellini<stefano.stabellini@eu.citrix.com> > > > > Applied. Thanks. > > IMHO this should go to stable too. > Yes, good idea.
On 01/27/2010 06:29 AM, Stefano Stabellini wrote: > On Wed, 27 Jan 2010, Gerd Hoffmann wrote: > >> On 01/27/10 01:07, Anthony Liguori wrote: >> >>> On 01/25/2010 06:54 AM, Stefano Stabellini wrote: >>> >>>> Hi all, >>>> this patch fixes another bug in vnc_refresh: calling vnc_update_client >>>> might cause vs to be free()ed, in this case we cannot access vs->next >>>> right after to examine the next item on the list. >>>> >>>> Signed-off-by: Stefano Stabellini<stefano.stabellini@eu.citrix.com> >>>> >>> Applied. Thanks. >>> >> IMHO this should go to stable too. >> >> > Yes, good idea. > Already did. Regards, Anthony Liguori
diff --git a/vnc.c b/vnc.c index cc2a26e..92facde 100644 --- a/vnc.c +++ b/vnc.c @@ -2345,7 +2345,7 @@ static int vnc_refresh_server_surface(VncDisplay *vd) static void vnc_refresh(void *opaque) { VncDisplay *vd = opaque; - VncState *vs = NULL; + VncState *vs = NULL, *vn = NULL; int has_dirty = 0, rects = 0; vga_hw_update(); @@ -2354,8 +2354,10 @@ static void vnc_refresh(void *opaque) vs = vd->clients; while (vs != NULL) { + vn = vs->next; rects += vnc_update_client(vs, has_dirty); - vs = vs->next; + /* vs might be free()ed here */ + vs = vn; } /* vd->timer could be NULL now if the last client disconnected, * in this case don't update the timer */
Hi all, this patch fixes another bug in vnc_refresh: calling vnc_update_client might cause vs to be free()ed, in this case we cannot access vs->next right after to examine the next item on the list. Signed-off-by: Stefano Stabellini <stefano.stabellini@eu.citrix.com> ---