Re: [Xen-devel] [PATCH] x86/shadow: Drop incorrect diagnostic when shadowing TSS.RSP0

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> On 08.04.19 at 13:37, <andrew.cooper3@xxxxxxxxxx> wrote:
> On 08/04/2019 11:14, Jan Beulich wrote:
>>>>> On 05.04.19 at 21:09, <andrew.cooper3@xxxxxxxxxx> wrote:
>>> --- a/xen/arch/x86/mm/shadow/multi.c
>>> +++ b/xen/arch/x86/mm/shadow/multi.c
>>> @@ -3305,8 +3305,9 @@ static int sh_page_fault(struct vcpu *v,
>>>      {
>>>          /*
>>>           * If we are in the middle of injecting an exception or interrupt 
>>> then
>>> -         * we should not emulate: it is not the instruction at %eip that 
>>> caused
>>> -         * the fault. Furthermore it is almost certainly the case the 
>>> handler
>>> +         * we should not emulate: the fault is a side effect of the 
>>> processor
>>> +         * trying to push an exception frame onto a stack which has yet to 
>>> be
>>> +         * shadowed.  Furthermore it is almost certainly the case the 
>>> handler
>>>           * stack is currently considered to be a page table, so we should
>>>           * unshadow the faulting page before exiting.
>>>           */
>> Your addition to me looks to contradict the part of the comment you
>> leave in place: You say "which has yet to be shadowed", while the
>> pre-existing text says "it is almost certainly the case the handler
>> stack is currently considered to be a page table", which to me means
>> it _is_ already shadowed (and in fact should not be).
>>
>> In your addition, do you perhaps mean the page tables covering the
>> stack which have yet to be shadowed?
> 
> This clause is inside an hvm_event_pending() which looks at VMCS/VMCB
> pending injection.
> 
> This only becomes true via VT-x's
> 
>     __vmread(IDT_VECTORING_INFO, &idtv_info);
>     if ( exit_reason != EXIT_REASON_TASK_SWITCH )
>         vmx_idtv_reinject(idtv_info);
> 
> path, and the equivalent case on SVM which leaves the EVENTINJ field
> valid after vmexit.  (This is assuming that we have no bugs whereby we
> enter sh_page_fault() late, after some emulation has occurred.)
> 
> What this means is that the processor is trying to deliver an exception,
> and the #PF intercept has been hit (which occurs before escalation to
> #DF).  i.e. it is the memory reads/writes made by microcode which suffer
> a fault due to the linear addresses not being present in the shadows.
> 
> Beyond that, there is a second aspect to getting here, which is when the
> linear address hit something which the shadow code thinks is protected,
> which AFAICT, starts off as everything which doesn't have an L1 shadow
> pointing writeably at it.
> 
> In the XTF case where I encountered this first, it so happens that the
> processor delivering an exception from userspace is the first thing to
> ever touch the linear address at RSP0, so the stack always becomes
> shadowed during IDT vectoring.

I'm (at least) mildly confused: I follow what you write (I think), but
you again say "the stack always becomes shadowed". My original
question was whether you really mean that, as stacks, if at all,
should get shadowed only unintentionally (and hence get un-shadowed
immediately when that condition is detected). That is, my (slightly
rephrased) question stands: Do you perhaps mean the page tables
mapping the stack to become shadowed, rather than the stack itself?

Jan



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel