Re: [PATCH] Fix regression initialising QXL interface

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Wed, Mar 09, 2016 at 05:50:45AM -0500, Frediano Ziglio wrote:
> > 
> > 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@xxxxxxxxxxxxxxx>
> > > ---
> > >  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.

Ah sorry, I read the _return_if_fail() backwards, this is checking that
the field is actually NULL, but the check is stricter than before and
breaks Xspice. Disregard my comments, still

Acked-by: Christophe Fergeau <cfergeau@xxxxxxxxxx> :)

Christophe

Attachment: signature.asc
Description: PGP signature

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel

[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]