[PATCH] qemu: Avoid crash in qemuStateShutdownPrepare() and qemuStateShutdownWait()

[Nikolay Shirokovskiy]

On Fri, Jan 22, 2021 at 12:45 PM Michal Privoznik <mprivozn at redhat.com>
wrote:

> If libvirtd is sent SIGTERM while it is still initializing, it
> may crash. The following scenario was observed (using 'stress' to
> slow down CPU so much that the window where the problem exists is
> bigger):
>
> 1) The main thread is already executing virNetDaemonRun() and is
>    in virEventRunDefaultImpl().
> 2) The thread that's supposed to run daemonRunStateInit() is
>    spawned already, but daemonRunStateInit() is in its very early
>    stage (in the stack trace I see it's executing
>    virIdentityGetSystem()).
>
> If SIGTERM (or any other signal that we don't override handler
> for) arrives at this point, the main thread jumps out from
> virEventRunDefaultImpl() and enters virStateShutdownPrepare()
> (via shutdownPrepareCb which was set earlier). This iterates
> through stateShutdownPrepare() callbacks of state drivers and
> reaching qemuStateShutdownPrepare() eventually only to
> dereference qemu_driver. But since thread 2) has not been
> scheduled/not proceeded yet, qemu_driver was not allocated yet.
>
> Solution is simple - just check if qemu_driver is not NULL. But
> doing so only in qemuStateShutdownPrepare() would push the
> problem down to virStateShutdownWait(), well
> qemuStateShutdownWait(). Therefore, duplicate the trick there
> too.
>

I guess this is a partial solution. Initialization may be in a state when
qemu_driver is initialized but qemu_driver->workerPool is still NULL
for example.

Maybe we'd better delay shutdown until initialization is finished?

Nikolay



>
> Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1895359#c14
> Signed-off-by
> <https://bugzilla.redhat.com/show_bug.cgi?id=1895359#c14Signed-off-by>:
> Michal Privoznik <mprivozn at redhat.com>
> ---
>  src/qemu/qemu_driver.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
> index 027617deef..ca4f366323 100644
> --- a/src/qemu/qemu_driver.c
> +++ b/src/qemu/qemu_driver.c
> @@ -1072,6 +1072,9 @@ qemuStateStop(void)
>  static int
>  qemuStateShutdownPrepare(void)
>  {
> +    if (!qemu_driver)
> +        return 0;
> +
>      virThreadPoolStop(qemu_driver->workerPool);
>      return 0;
>  }
> @@ -1091,6 +1094,9 @@ qemuDomainObjStopWorkerIter(virDomainObjPtr vm,
>  static int
>  qemuStateShutdownWait(void)
>  {
> +    if (!qemu_driver)
> +        return 0;
> +
>      virDomainObjListForEach(qemu_driver->domains, false,
>                              qemuDomainObjStopWorkerIter, NULL);
>      virThreadPoolDrain(qemu_driver->workerPool);
> --
> 2.26.2
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20210122/db3ecb64/attachment-0001.htm>