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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 16/10/17 16:39, Jan Beulich wrote:
>>>> On 16.10.17 at 16:49, <andrew.cooper3@xxxxxxxxxx> 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

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