From 9986ddec4ccdc1f6320c158be50274fe390b7bb2 Mon Sep 17 00:00:00 2001 From: Greg Kurz Date: Thu, 14 Jun 2018 23:50:42 +0200 Subject: [PATCH] spapr_cpu_core: add missing rollback on realization path The spapr_realize_vcpu() function doesn't rollback in case of error. This isn't a problem with coldplugged CPUs because the machine won't start and QEMU will exit. Hotplug is a different story though: the CPU thread is started under object_property_set_bool() and it assumes it can access the CPU object. If icp_create() fails, we return an error without unregistering the reset handler for this CPU, and we let the underlying QEMU thread for this CPU alive. Since spapr_cpu_core_realize() doesn't care to unrealize already realized CPUs either, but happily frees all of them anyway, the CPU thread crashes instantly: (qemu) device_add host-spapr-cpu-core,core-id=1,id=gku GKU: failing icp_create (cpu 0x11497fd0) ^^^^^^^^^^ Program received signal SIGSEGV, Segmentation fault. [Switching to Thread 0x7fffee3feaa0 (LWP 24725)] 0x00000000104c8374 in object_dynamic_cast_assert (obj=0x11497fd0, ^^^^^^^^^^^^^^ pointer to the CPU object 623 trace_object_dynamic_cast_assert(obj ? obj->class->type->name (gdb) p obj->class->type $1 = (Type) 0x0 (gdb) p * obj $2 = {class = 0x10ea9c10, free = 0x11244620, ^^^^^^^^^^ should be g_free (gdb) p g_free $3 = {} 0x7ffff282bef0 obj is a dangling pointer to the CPU that was just destroyed in spapr_cpu_core_realize(). This patch adds proper rollback to both spapr_realize_vcpu() and spapr_cpu_core_realize(). Signed-off-by: Greg Kurz [dwg: Fixed a conflict due to a change in my tree] Signed-off-by: David Gibson --- hw/ppc/spapr_cpu_core.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c index a9bb2d61e8..30a7cf9e03 100644 --- a/hw/ppc/spapr_cpu_core.c +++ b/hw/ppc/spapr_cpu_core.c @@ -145,11 +145,14 @@ static void spapr_realize_vcpu(PowerPCCPU *cpu, sPAPRMachineState *spapr, cpu->intc = icp_create(OBJECT(cpu), spapr->icp_type, XICS_FABRIC(spapr), &local_err); if (local_err) { - goto error; + goto error_unregister; } return; +error_unregister: + qemu_unregister_reset(spapr_cpu_reset, cpu); + cpu_remove_sync(CPU(cpu)); error: error_propagate(errp, local_err); } @@ -208,11 +211,15 @@ static void spapr_cpu_core_realize(DeviceState *dev, Error **errp) for (j = 0; j < cc->nr_threads; j++) { spapr_realize_vcpu(sc->threads[j], spapr, &local_err); if (local_err) { - goto err; + goto err_unrealize; } } return; +err_unrealize: + while (--j >= 0) { + spapr_unrealize_vcpu(sc->threads[j]); + } err: while (--i >= 0) { obj = OBJECT(sc->threads[i]);