On Wed, Feb 18, 2015 at 04:13:15PM +0000, Daniel P. Berrange wrote: > On Wed, Feb 18, 2015 at 05:08:28PM +0100, Pavel Hrdina wrote: > > 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. > > Oh that's a good point. I forgot about default SIGHUP behaviour. > > > 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. > > Maybe your current approach is actually simpler than having to create a > dummy sighup handler function. We'd just need to reset driversInitialized > to false again before virSTateCleanup > Agree, that's the missing peace. I'll sent v2. > > 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 -- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list