[PATCH] qemu: Fix potential crash during driver cleanup

[Martin Kletzander]

On Tue, Apr 11, 2023 at 09:47:28AM -0600, Jim Fehlig wrote:
>During qemu driver shutdown, objects are freed in qemuStateCleanup that
>could still be used by active worker threads, resulting in crashes. E.g.
>a worker thread could be processing a monitor EOF event after the
>security manager is already disposed
>
>Program terminated with signal SIGSEGV, Segmentation fault.
>#0  0x00007fd9a9a1e1fe in virSecurityManagerMoveImageMetadata (mgr=0x7fd948012160, pid=-1, src=src at entry=0x7fd98c072c90, dst=dst at entry=0x0)
>    at ../../src/security/security_manager.c:468
>#1  0x00007fd9646ff0f0 in qemuSecurityMoveImageMetadata (driver=driver at entry=0x7fd948043830, vm=vm at entry=0x7fd98c066db0, src=src at entry=0x7fd98c072c90,
>    dst=dst at entry=0x0) at ../../src/qemu/qemu_security.c:182
>#2  0x00007fd96462c7b0 in qemuBlockRemoveImageMetadata (driver=driver at entry=0x7fd948043830, vm=vm at entry=0x7fd98c066db0, diskTarget=0x7fd98c072530 "vda",
>    src=<optimized out>) at ../../src/qemu/qemu_block.c:2628
>#3  0x00007fd9646929d6 in qemuProcessStop (driver=driver at entry=0x7fd948043830, vm=vm at entry=0x7fd98c066db0, reason=reason at entry=VIR_DOMAIN_SHUTOFF_SHUTDOWN,
>    asyncJob=asyncJob at entry=QEMU_ASYNC_JOB_NONE, flags=<optimized out>) at ../../src/qemu/qemu_process.c:7585
>#4  0x00007fd9646fc842 in processMonitorEOFEvent (vm=0x7fd98c066db0, driver=0x7fd948043830) at ../../src/qemu/qemu_driver.c:4794
>#5  qemuProcessEventHandler (data=0x561a93febb60, opaque=0x7fd948043830) at ../../src/qemu/qemu_driver.c:4900
>#6  0x00007fd9a9971a31 in virThreadPoolWorker (opaque=opaque at entry=0x561a93fb58e0) at ../../src/util/virthreadpool.c:163
>(gdb) p mgr->drv
>$2 = (virSecurityDriverPtr) 0x0
>
>Prior to commit 7cf76d4e3ab, the worker thread pool was freed before
>disposing any driver objects. Let's return to that pattern, but leave
>the other changes made by 7cf76d4e3ab.
>
>Signed-off-by: Jim Fehlig <jfehlig at suse.com>

Reviewed-by: Martin Kletzander <mkletzan at redhat.com>

>---
> src/qemu/qemu_driver.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
>diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
>index b4fb7ec1df..28e470e4a2 100644
>--- a/src/qemu/qemu_driver.c
>+++ b/src/qemu/qemu_driver.c
>@@ -1062,6 +1062,7 @@ qemuStateCleanup(void)
>     if (!qemu_driver)
>         return -1;
>
>+    virThreadPoolFree(qemu_driver->workerPool);
>     virObjectUnref(qemu_driver->migrationErrors);
>     virLockManagerPluginUnref(qemu_driver->lockManager);
>     virSysinfoDefFree(qemu_driver->hostsysinfo);
>@@ -1078,7 +1079,6 @@ qemuStateCleanup(void)
>     ebtablesContextFree(qemu_driver->ebtables);
>     VIR_FREE(qemu_driver->qemuImgBinary);
>     virObjectUnref(qemu_driver->domains);
>-    virThreadPoolFree(qemu_driver->workerPool);
>
>     if (qemu_driver->lockFD != -1)
>         virPidFileRelease(qemu_driver->config->stateDir, "driver", qemu_driver->lockFD);
>-- 
>2.40.0
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20230412/1c7520a4/attachment.sig>