Re: [Qemu-devel] [FIX PATCH] spapr: prevent QEMU crash when CPU realization fails

[Greg Kurz]

On Thu, 15 Jun 2017 08:22:44 +0530
Bharata B Rao <address@hidden> wrote:

> ICPState objects were being allocated before CPU thread realization.
> However commit 9ed656631d73 (xics: setup cpu at realize time) reversed it
> by allocating ICPState objects after CPU thread is realized. But it
> didn't take care to fix the error path because of which we observe
> a SIGSEGV when CPU thread realization fails during cold/hotplug.
> 
> Fix this by ensuring that we do object_unparent() of ICPState object
> only in case when is was created earlier.
> 

Oops, my bad... my initial intent was to conditionally call object_unparent()
and I simply forgot to put the "if (obj) { }". But your patch is valid as well
of course. Maybe you can drop the initialization of obj to NULL on the way,
since it really doesn't make sense anymore.

Thanks!

Reviewed-by: Greg Kurz <address@hidden>

> Signed-off-by: Bharata B Rao <address@hidden>
> ---
> NOTE: There is another SIGSEGV failure that I am investigating which happens
> when CPU realization fails. It appears that that the CPU object isn't getting
> fully cleaned up.
> 



>  hw/ppc/spapr_cpu_core.c | 5 +++--
>  1 file changed, 3 insertions(+), 2 deletions(-)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index d6719d5..0d0e959 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -198,13 +198,14 @@ static void spapr_cpu_core_realize_child(Object *child, 
> Error **errp)
>      object_property_add_const_link(obj, ICP_PROP_CPU, child, &error_abort);
>      object_property_set_bool(obj, true, "realized", &local_err);
>      if (local_err) {
> -        goto error;
> +        goto free_icp;
>      }
>  
>      return;
>  
> -error:
> +free_icp:
>      object_unparent(obj);
> +error:
>      error_propagate(errp, local_err);
>  }
>

pgph9qcWTWHRt.pgp
Description: OpenPGP digital signature