Re: [Qemu-devel] [PATCH 2/5] spapr_cpu_core: fix potential leak in spapr_cpu_core_realize()

[David Gibson]

On Thu, Jun 14, 2018 at 11:50:27PM +0200, Greg Kurz wrote:
> Commit 94ad93bd97684 (QEMU 2.12) switched to instantiate CPUs separately
> but it missed to adapt the error path accordingly. If something fails in
> the CPU creation loop, then the CPU object that was just created is leaked.
> 
> The error paths in this function are a bit obfuscated, and adding
> yet another label to free this CPU object makes it worse. We should
> move the block of the loop to a separate function, with a proper
> rollback path, but this is a bigger cleanup.
> 
> For now, let's just fix the bug by adding the missing calls to
> object_unref(). This will allow easier backport to older QEMU
> versions.
> 
> Signed-off-by: Greg Kurz <address@hidden>

Applied to ppc-for-3.0, thanks.

> ---
>  hw/ppc/spapr_cpu_core.c |    2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/hw/ppc/spapr_cpu_core.c b/hw/ppc/spapr_cpu_core.c
> index 27602245fd55..003c4c5a79d2 100644
> --- a/hw/ppc/spapr_cpu_core.c
> +++ b/hw/ppc/spapr_cpu_core.c
> @@ -201,6 +201,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, 
> Error **errp)
>          cs->cpu_index = cc->core_id + i;
>          spapr_set_vcpu_id(cpu, cs->cpu_index, &local_err);
>          if (local_err) {
> +            object_unref(obj);
>              goto err;
>          }
>  
> @@ -212,6 +213,7 @@ static void spapr_cpu_core_realize(DeviceState *dev, 
> Error **errp)
>          object_property_add_child(OBJECT(sc), id, obj, &local_err);
>          g_free(id);
>          if (local_err) {
> +            object_unref(obj);
>              goto err;
>          }
>          object_unref(obj);
> 

-- 
David Gibson                    | I'll have my music baroque, and my code
david AT gibson.dropbear.id.au  | minimalist, thank you.  NOT _the_ _other_
                                | _way_ _around_!
http://www.ozlabs.org/~dgibson

signature.asc
Description: PGP signature