[Libvirt-announce] [libvirt] libvirt-0.9.5 availability of rc2

[Daniel Veillard]

On Wed, Sep 21, 2011 at 10:55:37AM +0100, Daniel P. Berrange wrote:
> On Tue, Sep 20, 2011 at 12:03:00PM +0800, Daniel Veillard wrote:
> > On Mon, Sep 19, 2011 at 08:15:18AM -0500, Adam Litke wrote:
> > > On Mon, Sep 19, 2011 at 04:04:04PM +0800, Daniel Veillard wrote:
> > > > On Sun, Sep 18, 2011 at 09:37:22AM -0500, Adam Litke wrote:
> > > >   Hum, I wonder if remoteRelayDomainEventBlockJob shouldn't strdup the
> > > > path string instead of using it directly in the
> > > > remote_domain_event_block_job_msg block. As a result since we now
> > > > free the datapointed by the xdr message within
> > > > remoteDispatchDomainEventSend() , this errors wasn't shown before but
> > > > leads to a double free now.
> > > > 
> > > > BTW it seems we don't check all allocations in the xdr code (on purpose
> > > > ?) for example make_nonnull_domain() doesn't check a strdup.
> > > > 
> > > > Could you check the following patch ?
> > > 
> > > Yep, this seems to fix the problem (and an extra check with valgrind shows no
> > > memory leaks.  Although I haven't verified it, the functions:
> > > 
> > >    remoteRelayDomainEventIOError
> > >    remoteRelayDomainEventIOErrorReason
> > >    remoteRelayDomainEventGraphics
> > > 
> > > appear to have the same problem as well.
> > 
> >   Right though they do far more allocations. I ended up pushing the
> > following patch which tries to be a bit better on handling memory
> > allocation error, but it's not fully complete yet:
> > 
> > 
> > Fix crash on events due to allocation errors
> > 
> > remoteRelayDomainEventBlockJob, remoteRelayDomainEventIOError,
> > remoteRelayDomainEventIOErrorReason and remoteRelayDomainEventGraphics
> > were using const string directly in rpc structure, before calling
> > remoteDispatchDomainEventSend(). But that routine now frees up all
> > the pointed allocated memory from the rpc structure and we end up
> > with a double free.
> > This now strdup() all the strings passed and provide mem_error goto
> > labels to be used when an allocation error occurs.
> > Note that the cleanup isn't completely finished because all relaying
> > function also call make_nonnull_domain() which also allocate a string
> > and never handle the error case. This patches doesn't try to address
> > this as this is only error correctness a priori and touches far more
> > functions in this module:
> > 
> > * daemon/remote.c: fix string allocations and memory error handling
> >   for remoteRelayDomainEventBlockJob, remoteRelayDomainEventIOError,
> >   remoteRelayDomainEventIOErrorReason and remoteRelayDomainEventGraphics
> > 
> > diff --git a/daemon/remote.c b/daemon/remote.c
> > index 45244f8..74e759a 100644
> > --- a/daemon/remote.c
> > +++ b/daemon/remote.c
> > @@ -234,9 +234,13 @@ static int remoteRelayDomainEventIOError(virConnectPtr conn ATTRIBUTE_UNUSED,
> >  
> >      /* build return data */
> >      memset(&data, 0, sizeof data);
> > +    data.srcPath = strdup(srcPath);
> > +    if (data.srcPath == NULL)
> > +        goto mem_error;
> > +    data.devAlias = strdup(devAlias);
> > +    if (data.devAlias == NULL)
> > +        goto mem_error;
> >      make_nonnull_domain(&data.dom, dom);
> > -    data.srcPath = (char*)srcPath;
> > -    data.devAlias = (char*)devAlias;
> >      data.action = action;
> >  
> >      remoteDispatchDomainEventSend(client, remoteProgram,
> > @@ -244,6 +248,11 @@ static int remoteRelayDomainEventIOError(virConnectPtr conn ATTRIBUTE_UNUSED,
> >                                    (xdrproc_t)xdr_remote_domain_event_io_error_msg, &data);
> >  
> >      return 0;
> > +mem_error:
> > +    virReportOOMError();
> > +    virFree(data.srcPath);
> > +    virFree(data.devAlias);
> > +    return -1;
> 
> s/virFree/VIR_FREE/ throughout all this code.

  Yeah Eric pointed that out, I already commited his patch and the
associated make syntax-check test :-)

Daniel

-- 
Daniel Veillard      | libxml Gnome XML XSLT toolkit  http://xmlsoft.org/
daniel at veillard.com  | Rpmfind RPM search engine http://rpmfind.net/
http://veillard.com/ | virtualization library  http://libvirt.org/