Re: [Xen-devel] [PATCH v11 6/9] xen: Add ring 3 vmware_port support

[George Dunlap <george.dunlap@xxxxxxxxxxxxx>]

On 06/04/2015 01:37 PM, Don Slutz wrote:
> On 06/03/15 12:58, George Dunlap wrote:
>> On 06/03/2015 05:41 PM, Don Slutz wrote:
>>> On 06/03/15 12:23, George Dunlap wrote:
>>>> On 06/03/2015 04:58 PM, Andrew Cooper wrote:
>>>>> On 03/06/15 16:26, George Dunlap wrote:
>>>>>> On 05/22/2015 04:50 PM, Don Slutz wrote:
>>>>>>> Summary is that VMware treats "in (%dx),%eax" (or "out %eax,(%dx)")
>>>>>>> to port 0x5658 specially.  Note: since many operations return data
>>>>>>> in EAX, "in (%dx),%eax" is the one to use.  The other lengths like
>>>>>>> "in (%dx),%al" will still do things, only AL part of EAX will be
>>>>>>> changed.  For "out %eax,(%dx)" of all lengths, EAX will remain
>>>>>>> unchanged.
>>>>>>>
>>>>>>> This instruction is allowed to be used from ring 3.  To
>>>>>>> support this the vmexit for GP needs to be enabled.  I have not
>>>>>>> fully tested that nested HVM is doing the right thing for this.
>>>>>>>
>>>>>>> Enable no-fault of pio in x86_emulate for VMware port
>>>>>>>
>>>>>>> Also adjust the emulation registers after doing a VMware
>>>>>>> backdoor operation.
>>>>>>>
>>>>>>> Add new routine hvm_emulate_one_gp() to be used by the #GP fault
>>>>>>> handler.
>>>>>>>
>>>>>>> Some of the best info is at:
>>>>>>>
>>>>>>> https://sites.google.com/site/chitchatvmback/backdoor
>>>>>>>
>>>>>>> Signed-off-by: Don Slutz <dslutz@xxxxxxxxxxx>
>>>>>> So let me get this straight.
>>>>>>
>>>>>> VMWare allows ring3 to access the magic port regardless of whether the
>>>>>> guest OS has enabled access to that IO port or not.
>>>>>>
>>>>>> In order to emulate this, we need to:
>>>>>> * Trap to Xen on #GPs rather than just letting the hardware handle it
>>>>>> * Emulate all instructions which cause a #GP, just to see if they might
>>>>>> be an IO instruction accessing the magic port.
>>>>>> * If it is an IO instruction, and it's accessing the magic port, then we
>>>>>> skip the ioport access checks (which will cause the instruction to
>>>>>> execute as though it had been given access).
>>>>>> * Under all other circumstances (we hope) the emulator in Xen will do
>>>>>> exactly what the hardware just did, and deliver a #GP to the guest.
>>>>>>
>>>>>> In an attempt to make this more safe, emulation ops that write (such as
>>>>>> write and cmpxchg) are replaced with stubs which always return an error.
>>>>>>
>>>>>> Is that about right?
>>>>>>
>>>>>> That sounds completely insane.  It opens up an almost infinite surface
>>>>>> of attack onto the Xen emulator.
>>>>>>
>>>>>> I understand that having the "VMWare compatible" is a nice tick-box to
>>>>>> have, but seriously, I cannot imagine that having unprivileged
>>>>>> user-space tools know the real clock frequency without having to involve
>>>>>> the OS is anywhere close to worth the risk involved.
>>>>>
>>>>> The attack surface sadly is not enlarged in the slightest by this change.
>>>>>
>>>>> We already trap and emulate all #UD exceptions in an attempt to support
>>>>> migration of VMs between Intel and AMD hardware.  See XSA-105.  (There
>>>>> is a good argument to be made for not trapping #UD, but that doesn't
>>>>> completely close the hole)
>>>>
>>>> So at the moment, an attacker on Intel can force the emulation of any
>>>> AMD-only instruction (and vice versa), is that right?
>>>>
>>>> This would allow an attacker to force the emulation of every #GP
>>>> condition of every instruction we emulate.
>>>>
>>>> Those two sets may be within an order of magnitude of each other, but
>>>> they will only overlap a little bit.  So my guess is that enabling this
>>>> would double the surface of attack (give or take).
>>>>
>>>> I'd be a lot happier with this patch if we could make it so that on a
>>>> #GP the only instruction that could get emulated would be an IO 
>>>> instruction.
>>>>
>>>
>>> You mean like I said in:
>>>
>>>
>>> Message-ID: <54C67D8302000078000598E4@xxxxxxxxxxxxxxxxxxxx>
>>
>> Yes, pretty much exactly.
>>
>> I didn't notice that particular part of the discussion, but I did go
>> back and skim the comments that people had made on previous revisions,
>> and I certainly noticed that both Jan and Andy reviewed this patch, and
>> that neither one objected to the general idea.  So my "That sounds
>> insane" was as much directed at them as at you.
>>
>> (As an aside, I think my description does a better job of alerting a
>> reviewer to what's going on in this patch -- you might consider stealing
>> part of it if you end up re-submitting this one.)
>>
> 
> I would be happy to "steal" the description part.  I normally give
> credit to the author in the "what has changed".  I could also add to the
> commit message:
> 
> 
> George Dunlap summarized this change as:
> 
> VMWare allows ring3 to access the magic port regardless of whether the
> guest OS has enabled access to that IO port or not.
> 
> In order to emulate this, we need to:
> * Trap to Xen on #GPs rather than just letting the hardware handle it
> * Emulate all instructions which cause a #GP, just to see if they might
>   be an IO instruction accessing the magic port.
> * If it is an IO instruction, and it's accessing the magic port, then we
>   skip the ioport access checks (which will cause the instruction to
>   execute as though it had been given access).
> * Under all other circumstances (we hope) the emulator in Xen will do
>   exactly what the hardware just did, and deliver a #GP to the guest.
> 
> In an attempt to make this more safe, emulation ops that write (such as
> write and cmpxchg) are replaced with stubs which always return an error.

I would take the "(we hope)" out, since that's really part of the second
half (me doubting whether such a patch is wise).  Not a good idea to
doubt whether a patch is a good idea in the commit message. :-)

If you feel like credit is necessary, I'd just put at the bottom
somewhere in parentheses something like this:

(h/t to George Dunlap for the patch summary.)

 -George

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