summaryrefslogtreecommitdiffstats
path: root/hw
diff options
context:
space:
mode:
authorGreg Kurz2018-06-14 23:50:42 +0200
committerDavid Gibson2018-06-16 08:32:33 +0200
commit9986ddec4ccdc1f6320c158be50274fe390b7bb2 (patch)
tree828ae80349c70d3ccf852bfd6704511293229809 /hw
parentspapr_cpu_core: fix potential leak in spapr_cpu_core_realize() (diff)
downloadqemu-9986ddec4ccdc1f6320c158be50274fe390b7bb2.tar.gz
qemu-9986ddec4ccdc1f6320c158be50274fe390b7bb2.tar.xz
qemu-9986ddec4ccdc1f6320c158be50274fe390b7bb2.zip
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 = {<text variable, no debug info>} 0x7ffff282bef0 <g_free> 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 <groug@kaod.org> [dwg: Fixed a conflict due to a change in my tree] Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
Diffstat (limited to 'hw')
-rw-r--r--hw/ppc/spapr_cpu_core.c11
1 files 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]);