Re: [libvirt] [PATCH 3/5] libxl: Move virDomainXMLOptionNew into libxlCreateXMLConf

["Daniel P. Berrange" <berrange@redhat.com>]

On Fri, May 30, 2014 at 04:41:28PM -0600, Jim Fehlig wrote:
> Daniel P. Berrange wrote:
> > To allow the test suite to creat the XML option object,
> > move the virDomainXMLOptionNew call into a libxlCreateXMLConf
> > method.
> >
> > Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
> > ---
> >  src/libxl/libxl_conf.c   | 7 +++++++
> >  src/libxl/libxl_conf.h   | 2 ++
> >  src/libxl/libxl_domain.c | 4 ++--
> >  src/libxl/libxl_driver.c | 4 +---
> >  4 files changed, 12 insertions(+), 5 deletions(-)
> >

> > diff --git a/src/libxl/libxl_domain.c b/src/libxl/libxl_domain.c
> > index e00a3fb..00ff14f 100644
> > --- a/src/libxl/libxl_domain.c
> > +++ b/src/libxl/libxl_domain.c
> > @@ -1100,6 +1100,8 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> >  #endif
> >      virHostdevManagerPtr hostdev_mgr = driver->hostdevMgr;
> >  
> > +    libxl_domain_config_init(&d_config);
> > +
> >      if (libxlDomainObjPrivateInitCtx(vm) < 0)
> >          return ret;
> >  
> > @@ -1149,8 +1151,6 @@ libxlDomainStart(libxlDriverPrivatePtr driver, virDomainObjPtr vm,
> >          VIR_FREE(managed_save_path);
> >      }
> >  
> > -    libxl_domain_config_init(&d_config);
> > -
> >      if (libxlBuildDomainConfig(driver->reservedVNCPorts, vm->def,
> >                                 priv->ctx, &d_config) < 0)
> >          goto endjob;
> >   
> 
> Are these two hunks fixing a bug you found? :-)

Hmm, yes, I should have done those as a separate patch.


The 'd_config' variable is stack allocated so has undefined initial
state. 'libxl_domain_config_init' is basically doing a memset(0,...)
on it to give it predictable value. So if we call that late in the
function, there's a chance that a 'goto endjob' call will jump to
the place where we call libxl_domain_config_dispose(), which will
then access uninitialized memory, will unpredictably bad results.


Regards,
Daniel
-- 
|: http://berrange.com      -o-    http://www.flickr.com/photos/dberrange/ :|
|: http://libvirt.org              -o-             http://virt-manager.org :|
|: http://autobuild.org       -o-         http://search.cpan.org/~danberr/ :|
|: http://entangle-photo.org       -o-       http://live.gnome.org/gtk-vnc :|