[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Xen-devel] [PATCH for-next 5/8] x86/domain: factor out pv_domain_destroy



On 10/04/17 14:27, Wei Liu wrote:
> No functional change.
>
> Signed-off-by: Wei Liu <wei.liu2@xxxxxxxxxx>

Throughout this series, please make sure you add in proper NULL'ing of
freed data.

While this patch is no functional change at the moment, you have
introduced a latent double-free bug for if (/when) pv_domain_destroy()
gets used on a failed create path.

Please make all of these functions idempotent when breaking them out.

~Andrew

> ---
>  xen/arch/x86/domain.c | 17 +++++++++--------
>  1 file changed, 9 insertions(+), 8 deletions(-)
>
> diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c
> index af060d8239..05885a103d 100644
> --- a/xen/arch/x86/domain.c
> +++ b/xen/arch/x86/domain.c
> @@ -527,6 +527,12 @@ static bool emulation_flags_ok(const struct domain *d, 
> uint32_t emflags)
>      return true;
>  }
>  
> +static void pv_domain_destroy(struct domain *d)
> +{
> +    xfree(d->arch.pv_domain.cpuidmasks);
> +    free_xenheap_page(d->arch.pv_domain.gdt_ldt_l1tab);
> +}
> +
>  int arch_domain_create(struct domain *d, unsigned int domcr_flags,
>                         struct xen_arch_domainconfig *config)
>  {
> @@ -704,10 +710,8 @@ int arch_domain_create(struct domain *d, unsigned int 
> domcr_flags,
>          paging_final_teardown(d);
>      free_perdomain_mappings(d);
>      if ( is_pv_domain(d) )
> -    {
> -        xfree(d->arch.pv_domain.cpuidmasks);
> -        free_xenheap_page(d->arch.pv_domain.gdt_ldt_l1tab);
> -    }
> +        pv_domain_destroy(d);
> +
>      return rc;
>  }
>  
> @@ -727,10 +731,7 @@ void arch_domain_destroy(struct domain *d)
>  
>      free_perdomain_mappings(d);
>      if ( is_pv_domain(d) )
> -    {
> -        free_xenheap_page(d->arch.pv_domain.gdt_ldt_l1tab);
> -        xfree(d->arch.pv_domain.cpuidmasks);
> -    }
> +        pv_domain_destroy(d);
>  
>      free_xenheap_page(d->shared_info);
>      cleanup_domain_irq_mapping(d);


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.