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

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(a)redhat.com&gt; > > --- > > 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.