Re: [Xen-devel] [PATCH for-4.10] xen/dom0: Fix latent dom0 construction bugs on all architectures

[Stefano Stabellini <sstabellini@xxxxxxxxxx>]

On Mon, 16 Oct 2017, Andrew Cooper wrote:
>  * x86 PV and ARM dom0's must not clear _VPF_down from v->pause_flags until
>    all state is actually set up.  As it currently stands, d0v0 is eligible for
>    scheduling before its registers have been set.  This is latent as we also
>    hold a systemcontroller pause reference at the time which prevents d0 from
>    being scheduled.
> 
>  * x86 PVH dom0's must set v->is_initialised on d0v0, to prevent another vcpu
>    being able to call VCPUOP_initialise and modify state under the feet of the
>    running vcpu.  This is latent as PVH dom0 construction don't yet function.
> 
> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>

ARM bits:

Reviewed-by: Stefano Stabellini <sstabellini@xxxxxxxxxx>


> ---
> CC: Jan Beulich <JBeulich@xxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Julien Grall <julien.grall@xxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> ---
>  xen/arch/arm/domain_build.c   |  6 +++---
>  xen/arch/x86/dom0_build.c     | 13 +++++++++++--
>  xen/arch/x86/hvm/dom0_build.c |  1 +
>  xen/arch/x86/pv/dom0_build.c  |  6 +++---
>  4 files changed, 18 insertions(+), 8 deletions(-)
> 
> diff --git a/xen/arch/arm/domain_build.c b/xen/arch/arm/domain_build.c
> index 4636b17..bf29299 100644
> --- a/xen/arch/arm/domain_build.c
> +++ b/xen/arch/arm/domain_build.c
> @@ -2197,9 +2197,6 @@ int construct_dom0(struct domain *d)
>  
>      discard_initial_modules();
>  
> -    v->is_initialised = 1;
> -    clear_bit(_VPF_down, &v->pause_flags);
> -
>      memset(regs, 0, sizeof(*regs));
>  
>      regs->pc = (register_t)kinfo.entry;
> @@ -2247,6 +2244,9 @@ int construct_dom0(struct domain *d)
>              vcpu_switch_to_aarch64_mode(d->vcpu[i]);
>      }
>  
> +    v->is_initialised = 1;
> +    clear_bit(_VPF_down, &v->pause_flags);
> +
>      return 0;
>  }
>  
> diff --git a/xen/arch/x86/dom0_build.c b/xen/arch/x86/dom0_build.c
> index e4bffd5..bf992fe 100644
> --- a/xen/arch/x86/dom0_build.c
> +++ b/xen/arch/x86/dom0_build.c
> @@ -466,6 +466,8 @@ int __init construct_dom0(struct domain *d, const 
> module_t *image,
>                            void *(*bootstrap_map)(const module_t *),
>                            char *cmdline)
>  {
> +    int rc;
> +
>      /* Sanity! */
>      BUG_ON(d->domain_id != 0);
>      BUG_ON(d->vcpu[0] == NULL);
> @@ -481,8 +483,15 @@ int __init construct_dom0(struct domain *d, const 
> module_t *image,
>      }
>  #endif
>  
> -    return (is_hvm_domain(d) ? dom0_construct_pvh : dom0_construct_pv)
> -           (d, image, image_headroom, initrd,bootstrap_map, cmdline);
> +    rc = (is_hvm_domain(d) ? dom0_construct_pvh : dom0_construct_pv)
> +         (d, image, image_headroom, initrd, bootstrap_map, cmdline);
> +    if ( rc )
> +        return rc;
> +
> +    /* Sanity! */
> +    BUG_ON(!d->vcpu[0]->is_initialised);
> +
> +    return 0;
>  }
>  
>  /*
> diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c
> index e8f746c..a67071c 100644
> --- a/xen/arch/x86/hvm/dom0_build.c
> +++ b/xen/arch/x86/hvm/dom0_build.c
> @@ -614,6 +614,7 @@ static int __init pvh_setup_cpus(struct domain *d, 
> paddr_t entry,
>  
>      update_domain_wallclock_time(d);
>  
> +    v->is_initialised = 1;
>      clear_bit(_VPF_down, &v->pause_flags);
>  
>      return 0;
> diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c
> index dcbee43..8ad7e3d 100644
> --- a/xen/arch/x86/pv/dom0_build.c
> +++ b/xen/arch/x86/pv/dom0_build.c
> @@ -847,9 +847,6 @@ int __init dom0_construct_pv(struct domain *d,
>  
>      update_domain_wallclock_time(d);
>  
> -    v->is_initialised = 1;
> -    clear_bit(_VPF_down, &v->pause_flags);
> -
>      /*
>       * Initial register values:
>       *  DS,ES,FS,GS = FLAT_KERNEL_DS
> @@ -883,6 +880,9 @@ int __init dom0_construct_pv(struct domain *d,
>      if ( d->domain_id == hardware_domid )
>          iommu_hwdom_init(d);
>  
> +    v->is_initialised = 1;
> +    clear_bit(_VPF_down, &v->pause_flags);
> +
>      return 0;
>  
>  out:
> -- 
> 2.1.4
>

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