Patchwork [for-4.10] xen/dom0: Fix latent dom0 construction bugs on all architectures

login
register
mail settings
Submitter Andrew Cooper
Date Oct. 16, 2017, 2:38 p.m.
Message ID <1508164683-25818-1-git-send-email-andrew.cooper3@citrix.com>
Download mbox | patch
Permalink /patch/363379/
State New
Headers show

Comments

Andrew Cooper - Oct. 16, 2017, 2:38 p.m.
* 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@citrix.com>
---
CC: Jan Beulich <JBeulich@suse.com>
CC: Stefano Stabellini <sstabellini@kernel.org>
CC: Julien Grall <julien.grall@arm.com>
CC: Wei Liu <wei.liu2@citrix.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
---
 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(-)
Wei Liu - Oct. 16, 2017, 2:44 p.m.
On Mon, Oct 16, 2017 at 03:38:03PM +0100, 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.
> 

While I think this patch is a good idea, the above paragraph confuses
me: I did boot PVH Dom0 at one point so it did function; I also never
triggered a bug like the one described here.
Wei Liu - Oct. 16, 2017, 3 p.m.
On Mon, Oct 16, 2017 at 03:49:54PM +0100, Andrew Cooper wrote:
> On 16/10/17 15:44, Wei Liu wrote:
> > On Mon, Oct 16, 2017 at 03:38:03PM +0100, 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.
> >>
> > While I think this patch is a good idea, the above paragraph confuses
> > me: I did boot PVH Dom0 at one point so it did function; I also never
> > triggered a bug like the one described here.
> 
> Strictly speaking, this is the PVH v2 dom0 path, not the legacy PVH dom0
> path.
> 
> The bottom of dom0_construct_pvh() currently has:
> 
> ...
>     panic("Building a PVHv2 Dom0 is not yet supported.");
>     return 0;
> }
> 

Oh yes, I was using a development branch.

> As for the v->is_initialised, a well behaved dom0 wouldn't hit the
> issue, because it wouldn't call VCPUOP_initialise against a running
> vcpu.  Nevertheless, it is relevant to Xen's security that such an
> attempt doesn't get to the point of actually trying to edit the VMC{S,B}
> under a running vcpu.
> 

Right.
Jan Beulich - Oct. 16, 2017, 3:39 p.m.
>>> On 16.10.17 at 16:49, <andrew.cooper3@citrix.com> wrote:
> On 16/10/17 15:44, Wei Liu wrote:
>> On Mon, Oct 16, 2017 at 03:38:03PM +0100, 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.
>>>
>> While I think this patch is a good idea, the above paragraph confuses
>> me: I did boot PVH Dom0 at one point so it did function; I also never
>> triggered a bug like the one described here.
> 
> Strictly speaking, this is the PVH v2 dom0 path, not the legacy PVH dom0
> path.
> 
> The bottom of dom0_construct_pvh() currently has:
> 
> ...
>     panic("Building a PVHv2 Dom0 is not yet supported.");
>     return 0;
> }
> 
> As for the v->is_initialised, a well behaved dom0 wouldn't hit the
> issue, because it wouldn't call VCPUOP_initialise against a running
> vcpu.  Nevertheless, it is relevant to Xen's security that such an
> attempt doesn't get to the point of actually trying to edit the VMC{S,B}
> under a running vcpu.

I don't understand this reply of yours: The changes you make
are for vCPU 0 only, and even there only for its initial state.
Even if Dom0 decided to bring down and back up that vCPU, it
would go through a different path.

Jan
Jan Beulich - Oct. 16, 2017, 3:41 p.m.
>>> On 16.10.17 at 16:38, <andrew.cooper3@citrix.com> wrote:
> --- 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);

How come this has no counterpart of code being deleted?

Jan
Roger Pau Monne - Oct. 16, 2017, 3:51 p.m.
On Mon, Oct 16, 2017 at 03:38:03PM +0100, 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@citrix.com>

LGTM, just one question.

> 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);

Don't you want to move this to the end of dom0_construct_pvh? In any
case, at this point the vCPU state is already setup correctly.

Thanks, Roger.
Andrew Cooper - Oct. 16, 2017, 4:07 p.m.
On 16/10/17 16:41, Jan Beulich wrote:
>  >>> On 16.10.17 at 16:38, <andrew.cooper3@citrix.com> wrote:
>> --- 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);
> How come this has no counterpart of code being deleted?

Because the bug is that it was never being set before.

~Andrew
Andrew Cooper - Oct. 16, 2017, 4:14 p.m.
On 16/10/17 16:39, Jan Beulich wrote:
>>>> On 16.10.17 at 16:49, <andrew.cooper3@citrix.com> wrote:
>> On 16/10/17 15:44, Wei Liu wrote:
>>> On Mon, Oct 16, 2017 at 03:38:03PM +0100, 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.
>>> While I think this patch is a good idea, the above paragraph confuses
>>> me: I did boot PVH Dom0 at one point so it did function; I also never
>>> triggered a bug like the one described here.
>> Strictly speaking, this is the PVH v2 dom0 path, not the legacy PVH dom0
>> path.
>>
>> The bottom of dom0_construct_pvh() currently has:
>>
>> ...
>>     panic("Building a PVHv2 Dom0 is not yet supported.");
>>     return 0;
>> }
>>
>> As for the v->is_initialised, a well behaved dom0 wouldn't hit the
>> issue, because it wouldn't call VCPUOP_initialise against a running
>> vcpu.  Nevertheless, it is relevant to Xen's security that such an
>> attempt doesn't get to the point of actually trying to edit the VMC{S,B}
>> under a running vcpu.
> I don't understand this reply of yours: The changes you make
> are for vCPU 0 only, and even there only for its initial state.

Correct.

> Even if Dom0 decided to bring down and back up that vCPU, it
> would go through a different path.

Correct as well, but unrelated to the bug.

The bug is that, at the moment, d0v1 can make a blind VCPUOP_initialise
call targeting d0v0, while d0v0 is running, and it will go and rewrite
state.  The problem is that d0v0 starts running in a way that
VCPUOP_initialise believes it to be eligible for initialisation.

All other mechanisms of bringing a vcpu down and up cause there to be
proper isolation between playing with a vcpus state, and it being
unscheduled.

~Andrew
Jan Beulich - Oct. 16, 2017, 4:21 p.m.
>>> On 16.10.17 at 18:07, <andrew.cooper3@citrix.com> wrote:
> On 16/10/17 16:41, Jan Beulich wrote:
>>  >>> On 16.10.17 at 16:38, <andrew.cooper3@citrix.com> wrote:
>>> --- 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);
>> How come this has no counterpart of code being deleted?
> 
> Because the bug is that it was never being set before.

Oh, I see - I had read that part of the commit message in slightly
a wrong way.

Reviewed-by: Jan Beulich <jbeulich@suse.com>

Jan
Andrew Cooper - Oct. 16, 2017, 4:25 p.m.
On 16/10/17 16:51, Roger Pau Monné wrote:
> On Mon, Oct 16, 2017 at 03:38:03PM +0100, 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@citrix.com>
> LGTM, just one question.
>
>> 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);
> Don't you want to move this to the end of dom0_construct_pvh? In any
> case, at this point the vCPU state is already setup correctly.

I had considered that, but a) As you say, it only matters when the vcpu
state is set up, and b) it would look odd being anywhere later.

~Andrew
Stefano Stabellini - Oct. 16, 2017, 5:26 p.m.
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@citrix.com>

ARM bits:

Reviewed-by: Stefano Stabellini <sstabellini@kernel.org>


> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien.grall@arm.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> ---
>  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
>
Andrew Cooper - Oct. 17, 2017, 10:38 a.m.
On 16/10/17 17:21, Jan Beulich wrote:
>>>> On 16.10.17 at 18:07, <andrew.cooper3@citrix.com> wrote:
>> On 16/10/17 16:41, Jan Beulich wrote:
>>>  >>> On 16.10.17 at 16:38, <andrew.cooper3@citrix.com> wrote:
>>>> --- 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);
>>> How come this has no counterpart of code being deleted?
>> Because the bug is that it was never being set before.
> Oh, I see - I had read that part of the commit message in slightly
> a wrong way.
>
> Reviewed-by: Jan Beulich <jbeulich@suse.com>

How about this for an adjusted commit message?

 * x86 PVH previously was not setting v->is_initialised for d0v0, despite
   setting the vcpu running eventually.  Therefore, a later VCPUOP_initialise
   hypercall will modify state under the feet of the running vcpu.  This is
   latent as PVH dom0 construction don't yet function.
Jan Beulich - Oct. 17, 2017, 10:47 a.m.
>>> On 17.10.17 at 12:38, <andrew.cooper3@citrix.com> wrote:
> On 16/10/17 17:21, Jan Beulich wrote:
>>>>> On 16.10.17 at 18:07, <andrew.cooper3@citrix.com> wrote:
>>> On 16/10/17 16:41, Jan Beulich wrote:
>>>>  >>> On 16.10.17 at 16:38, <andrew.cooper3@citrix.com> wrote:
>>>>> --- 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);
>>>> How come this has no counterpart of code being deleted?
>>> Because the bug is that it was never being set before.
>> Oh, I see - I had read that part of the commit message in slightly
>> a wrong way.
>>
>> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> 
> How about this for an adjusted commit message?
> 
>  * x86 PVH previously was not setting v->is_initialised for d0v0, despite
>    setting the vcpu running eventually.  Therefore, a later VCPUOP_initialise
>    hypercall will modify state under the feet of the running vcpu.  This is
>    latent as PVH dom0 construction don't yet function.

Yes, thanks.

Jan
Roger Pau Monne - Oct. 17, 2017, 10:50 a.m.
On Mon, Oct 16, 2017 at 03:38:03PM +0100, 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@citrix.com>

Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Julien Grall - Oct. 17, 2017, 1:11 p.m.
Hi Andrew,

On 16/10/17 15:38, 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@citrix.com>

Release-acked-by: Julien Grall <julien.grall@linaro.org>

Cheers,

> ---
> CC: Jan Beulich <JBeulich@suse.com>
> CC: Stefano Stabellini <sstabellini@kernel.org>
> CC: Julien Grall <julien.grall@arm.com>
> CC: Wei Liu <wei.liu2@citrix.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> ---
>   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:
>

Patch

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: