Re: [Xen-devel] xc_hvm_inject_trap() races

[Razvan Cojocaru <rcojocaru@xxxxxxxxxxxxxxx>]

On 11/02/2016 12:55 AM, Andrew Cooper wrote:
> On 01/11/2016 22:17, Andrei Vlad LUTAS wrote:
>>>> First of all, to answer your original question: the injection decision
>>>> is made when the introspection logic needs to inspect a page that is
>>>> not present in the physical memory. We don't really care if the current
>>>> instruction triggers multiple faults or not (and here I'm not sure what
>>>> you mean by that - multiple exceptions, or multiple EPT violations -
>>>> but the answer is still the same), and removing the page restrictions
>>>> after the #PF injection is introspection specific logic - the address
>>>> for which we inject the #PF doesn't have to be related in any way to the 
>>>> current instruction.
>>> Ah, that's this no-architectural behavior again.
>> I don't think the HVI #PF injection internals or how the #PF is handled by 
>> the OS are relevant here. We are using an existing API that seems to not 
>> work quite correct under certain circumstances and we were curious if any of 
>> you can shed some light in this regard, and maybe point us to the right 
>> direction for cooking up a fix.
> 
> Just because there is an API like this, doesn't necessarily mean it ever
> worked.  This one clearly doesn't, and it got introduced before we as a
> community took a rather harder stance towards code review.
> 
> Architecturally speaking, faults are always raised as a direct
> consequence of the current state.  Therefore, having in introspection
> agent interposing on the instruction stream and causing faults as a side
> effect of EPT permissions/etc, is quite natural and in line with
> architectural expectation.
> 
> You also have a second usecase for this API, which is to trick Windows
> into paging in a frame you care about looking at.  Overall, using this
> #PF method to get Windows to page content back in clearly the rational
> way of making that happen, but it is very definitely a non-architectural
> usecase; if windows were to double check the instruction stream on
> getting this pagefault, it would get very confused, as the pagefault it
> received has nothing to do with the code pointed at in the exception frame.
> 
> It is quite likely that these different usecases might have different
> solutions.  IMO, the former should probably be controlled by responses
> in the vm_event ring, but this latter issue probably shouldn't.
> 
> When it comes to injecting exceptions, there are some restrictions which
> limit what can legitimately be done.  We can only inject a single thing
> at once.  Stacking a #PF on top of a plain interrupt could be resolved
> by leaving the interrupt pending in the vLAPIC and injecting the #PF
> instead.  Stacking a #PF on top of a different fault is going to cause
> hvm_combine_exceptions() to turn it into something more nasty.  OTOH,
> deferring the #PF by even a single instruction could result in it being
> sent in an unsafe context, which should also be avoided.

I wonder if it would be acceptable to simply add "&&
v->arch.hvm_vcpu.inject_trap.vector != -1" to the early return condition
for vmx_intr_assist()?

Event injection is already blocked there if v->arch.hvm_vcpu.single_step
is set, so at least in that case it is acceptable behaviour. We could
also block it if there's a pending user-requested injection.


Thanks,
Razvan

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