[Spice-devel] [PATCH] Fix regression initialising QXL interface

Frediano Ziglio fziglio at redhat.com
Wed Mar 9 10:50:45 UTC 2016


> 
> On Wed, Mar 09, 2016 at 10:25:59AM +0000, Frediano Ziglio wrote:
> > st field in QXLInstance was not checked for NULL before
> > initialisation.
> > The additional check make Xspice crash during startup.
> > 
> > Reported-by: Francois Gouget <fgouget at codeweavers.com>
> > ---
> >  server/red-qxl.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/server/red-qxl.c b/server/red-qxl.c
> > index 1d29496..491046f 100644
> > --- a/server/red-qxl.c
> > +++ b/server/red-qxl.c
> > @@ -941,7 +941,6 @@ void red_qxl_init(RedsState *reds, QXLInstance *qxl)
> >      ClientCbs client_cbs = { NULL, };
> >  
> >      spice_return_if_fail(qxl != NULL);
> > -    spice_return_if_fail(qxl->st == NULL);
> >  
> >      static gsize initialized = FALSE;
> >      if (g_once_init_enter(&initialized)) {
> 
> ACK. Is this only causing issues with Xspice? If qxl->st is not NULL
> when we get into red_qxl_init(), it seems we are going to leak memory
> (or that it was working by chance because we did not memset
> QXLInstance to 0).
> 
> Christophe
> 

Actually only Xspice was affected. Could be that some application uses
static variable or they memset structure containing QXLInstance.
In any case this was a regression as previous one just set without any
check. Initializing twice leads to leak (as before) however I think
that in this case the bug is in the caller, not in spice-server, it's
like

  FILE *f = fopen(...);
  f = fopen(...);

the second assignment leak the first file.

Frediano


More information about the Spice-devel mailing list