Re: [PATCH 1/2] daemon: Fix segfault by reloading daemon right after start

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

 



On Wed, Feb 18, 2015 at 03:29:35PM +0000, Daniel P. Berrange wrote:
> On Wed, Feb 18, 2015 at 04:24:31PM +0100, Pavel Hrdina wrote:
> > Libvirt could crash with segfault if user issue "service reload" right
> > after "service start". One possible way to crash libvirt is to run reload
> > during initialization of QEMU driver.
> > 
> > It could happen when qemu driver will initialize qemu_driver_lock but
> > don't have a time to set it's "config" and the SIGHUP arrives. The
> > reload handler tries to get qemu_drv->config during "virStorageAutostart"
> > and dereference it which ends with segfault.
> > 
> > Let's ignore all reload requests until all drivers are initialized.
> > 
> > Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1179981
> > 
> > Signed-off-by: Pavel Hrdina <phrdina@xxxxxxxxxx>
> > ---
> >  daemon/libvirtd.c | 5 +++++
> >  1 file changed, 5 insertions(+)
> > 
> > diff --git a/daemon/libvirtd.c b/daemon/libvirtd.c
> > index 86accaa..835e7dc 100644
> > --- a/daemon/libvirtd.c
> > +++ b/daemon/libvirtd.c
> > @@ -785,6 +785,11 @@ static void daemonReloadHandler(virNetServerPtr srv ATTRIBUTE_UNUSED,
> >                                  siginfo_t *sig ATTRIBUTE_UNUSED,
> >                                  void *opaque ATTRIBUTE_UNUSED)
> >  {
> > +    if (!driversInitialized) {
> > +        VIR_WARN("Drivers are not initialized, reload ignored");
> > +        return;
> > +    }
> > +
> >      VIR_INFO("Reloading configuration on SIGHUP");
> >      virHookCall(VIR_HOOK_DRIVER_DAEMON, "-",
> >                  VIR_HOOK_DAEMON_OP_RELOAD, SIGHUP, "SIGHUP", NULL, NULL);
> 
> Why don't we just not register the reload handler until we get to the
> point where it is safe. eg register it only after virStateInitialize
> is complete, and unregister it before we call virStateCleanup.

In that case we should register empty handler for SIGHUP otherwise reload could
exit libvirtd during driver initialization. But you have a point that my patch
still doesn't fix the issue that reload could be triggered during
virStateCleanup.

I'll send a v2 where at first we will set empty handler. After
virStateInitialize is complete we set the proper reload handler and before
virStateCleanup we set again empty handler.

Thanks for review.

Pavel

> 
> 
> 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 :|

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list




[Index of Archives]     [Virt Tools]     [Libvirt Users]     [Lib OS Info]     [Fedora Users]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]     [Fedora Tools]