Re: [Xen-devel] [PATCH] xen/arm: vcpu: Correctly release resource when the VCPU failed to initialized

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 30/04/14 19:05, Julien Grall wrote:
> While I was adding new failing code at the end of the function, I've noticed
> that the vtimers are not freed which mess all the timers and will crash Xen
> quickly when the page will be reused.
>
> Currently none of vcpu_vgic_init and vcpu_vtimer_init doesn't failed, so we

This sentence is confusing.  Do you mean "Currently neither
vcpu_vgic_init nor vcpu_vtimer_init fail, so we..." ?

~Andrew

> are safe for now. With the new GICv3 code, the former function will be able
> to fail. This will result to a memory leak.
>
> Call vcpu_destroy if the initialization has failed. We also need to add a
> boolean to know if the vtimers are correctly setup as the timer common code
> doesn't have safe guard against removing non-initialized timer.
>
> Signed-off-by: Julien Grall <julien.grall@xxxxxxxxxx>
> ---
>  xen/arch/arm/domain.c        |    8 ++++++--
>  xen/arch/arm/vtimer.c        |    5 +++++
>  xen/include/asm-arm/domain.h |    1 +
>  3 files changed, 12 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/arm/domain.c b/xen/arch/arm/domain.c
> index ccccb77..c47db4a 100644
> --- a/xen/arch/arm/domain.c
> +++ b/xen/arch/arm/domain.c
> @@ -468,12 +468,16 @@ int vcpu_initialise(struct vcpu *v)
>      processor_vcpu_initialise(v);
>  
>      if ( (rc = vcpu_vgic_init(v)) != 0 )
> -        return rc;
> +        goto fail;
>  
>      if ( (rc = vcpu_vtimer_init(v)) != 0 )
> -        return rc;
> +        goto fail;
>  
>      return rc;
> +
> +fail:
> +    vcpu_destroy(v);
> +    return rc;
>  }
>  
>  void vcpu_destroy(struct vcpu *v)
> diff --git a/xen/arch/arm/vtimer.c b/xen/arch/arm/vtimer.c
> index cb690bb..c515e7e 100644
> --- a/xen/arch/arm/vtimer.c
> +++ b/xen/arch/arm/vtimer.c
> @@ -77,11 +77,16 @@ int vcpu_vtimer_init(struct vcpu *v)
>          : GUEST_TIMER_VIRT_PPI;
>      t->v = v;
>  
> +    v->arch.vtimer_initialized = 1;
> +
>      return 0;
>  }
>  
>  void vcpu_timer_destroy(struct vcpu *v)
>  {
> +    if ( !v->arch.vtimer_initialized )
> +        return;
> +
>      kill_timer(&v->arch.virt_timer.timer);
>      kill_timer(&v->arch.phys_timer.timer);
>  }
> diff --git a/xen/include/asm-arm/domain.h b/xen/include/asm-arm/domain.h
> index ec66a4e..1be3da2 100644
> --- a/xen/include/asm-arm/domain.h
> +++ b/xen/include/asm-arm/domain.h
> @@ -285,6 +285,7 @@ struct arch_vcpu
>  
>      struct vtimer phys_timer;
>      struct vtimer virt_timer;
> +    bool_t vtimer_initialized;
>  }  __cacheline_aligned;
>  
>  void vcpu_show_execution_state(struct vcpu *);


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel