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